[PATCH] btrfs: fix subpage state mismatch in cow_fixup writeback path

[PATCH] btrfs: fix subpage state mismatch in cow_fixup writeback path

[Werner Kasselman]

writepage_delalloc() marks all dirty sectors as locked via
btrfs_folio_set_lock(), setting bits in the subpage locked bitmap and
incrementing nr_locked.  These are cleaned up by
btrfs_folio_end_lock_bitmap() at the end of extent_writepage().

However, when btrfs_writepage_cow_fixup() returns -EAGAIN inside
extent_writepage_io(), the code calls folio_unlock() directly and
returns 1, causing extent_writepage() to skip the bitmap cleanup:

    ret = btrfs_writepage_cow_fixup(folio);
    if (ret == -EAGAIN) {
        folio_redirty_for_writepage(bio_ctrl->wbc, folio);
        folio_unlock(folio);     // doesn't clear locked bitmap
        return 1;                // caller skips end_lock_bitmap()
    }

This leaves the subpage locked bitmap out of sync with the folio lock
state: the folio is unlocked but its subpage locked bitmap still has
bits set and nr_locked is elevated.  When writeback retries the folio,
btrfs_folio_set_lock() hits the ASSERT at subpage.c:746 because the
bits are still set from the previous attempt.

The cow_fixup path is largely a legacy path -- the GUP dirty-without-
informing-fs issue that triggered it has been fixed on the GUP side,
and experimental builds already catch this case with -EUCLEAN before
reaching the -EAGAIN return.  However the subpage state mismatch is
still a correctness issue for non-experimental builds under error
injection or memory pressure (kzalloc failure in
btrfs_writepage_cow_fixup()).

Fix this by replacing folio_unlock() with btrfs_folio_end_lock_bitmap(),
which properly clears the locked bitmap bits before unlocking.  For
non-subpage or when nr_locked is 0 (e.g. called from
extent_write_locked_range()), btrfs_folio_end_lock_bitmap() falls
through to plain folio_unlock(), so existing behavior is preserved.

Fixes: d034cdb4cc8a ("btrfs: lock subpage ranges in one go for writepage_delalloc()")
CC: stable@vger.kernel.org
Signed-off-by: Werner Kasselman <werner@verivus.com>
---
 fs/btrfs/extent_io.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index 5f97a3d2a8d7..c7b25e415498 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -1746,7 +1746,13 @@ static noinline_for_stack int extent_writepage_io(struct btrfs_inode *inode,
 	if (ret == -EAGAIN) {
 		/* Fixup worker will requeue */
 		folio_redirty_for_writepage(bio_ctrl->wbc, folio);
-		folio_unlock(folio);
+		/*
+		 * For subpage case, writepage_delalloc() may have set locked
+		 * bitmap bits for this folio.  We need to clear them or
+		 * btrfs_folio_set_lock() will ASSERT when writeback retries.
+		 */
+		btrfs_folio_end_lock_bitmap(fs_info, folio,
+					    bio_ctrl->submit_bitmap);
 		return 1;
 	}
 	if (ret < 0) {
-- 
2.43.0

Re: [PATCH] btrfs: fix subpage state mismatch in cow_fixup writeback path

[David Sterba]

On Mon, Mar 16, 2026 at 10:56:56AM +0000, Werner Kasselman wrote:
> writepage_delalloc() marks all dirty sectors as locked via
> btrfs_folio_set_lock(), setting bits in the subpage locked bitmap and
> incrementing nr_locked.  These are cleaned up by
> btrfs_folio_end_lock_bitmap() at the end of extent_writepage().
> 
> However, when btrfs_writepage_cow_fixup() returns -EAGAIN inside
> extent_writepage_io(), the code calls folio_unlock() directly and
> returns 1, causing extent_writepage() to skip the bitmap cleanup:
> 
>     ret = btrfs_writepage_cow_fixup(folio);
>     if (ret == -EAGAIN) {
>         folio_redirty_for_writepage(bio_ctrl->wbc, folio);
>         folio_unlock(folio);     // doesn't clear locked bitmap
>         return 1;                // caller skips end_lock_bitmap()
>     }
> 
> This leaves the subpage locked bitmap out of sync with the folio lock
> state: the folio is unlocked but its subpage locked bitmap still has
> bits set and nr_locked is elevated.  When writeback retries the folio,
> btrfs_folio_set_lock() hits the ASSERT at subpage.c:746 because the
> bits are still set from the previous attempt.
> 
> The cow_fixup path is largely a legacy path -- the GUP dirty-without-
> informing-fs issue that triggered it has been fixed on the GUP side,
> and experimental builds already catch this case with -EUCLEAN before
> reaching the -EAGAIN return.  However the subpage state mismatch is
> still a correctness issue for non-experimental builds under error
> injection or memory pressure (kzalloc failure in
> btrfs_writepage_cow_fixup()).
> 
> Fix this by replacing folio_unlock() with btrfs_folio_end_lock_bitmap(),
> which properly clears the locked bitmap bits before unlocking.  For
> non-subpage or when nr_locked is 0 (e.g. called from
> extent_write_locked_range()), btrfs_folio_end_lock_bitmap() falls
> through to plain folio_unlock(), so existing behavior is preserved.
> 
> Fixes: d034cdb4cc8a ("btrfs: lock subpage ranges in one go for writepage_delalloc()")
> CC: stable@vger.kernel.org
> Signed-off-by: Werner Kasselman <werner@verivus.com>

I'm going through patch backlog, this patch has some relevance. We're
going to remove the fixup worker code in 7.2 completely so it cannot be
applied to the development branch anymore.

The problems are hard to hit or need error injection, I don't know if
it's worth to backport to stable. We've provided a long grace period to
the fixup worker before removal and I'm glad we can delete it and forget
about it. If somebody wants one last fix then I'm OK with that.

Re: [PATCH] btrfs: fix subpage state mismatch in cow_fixup writeback path

[Qu Wenruo]

在 2026/5/25 23:43, David Sterba 写道:
> On Mon, Mar 16, 2026 at 10:56:56AM +0000, Werner Kasselman wrote:
>> writepage_delalloc() marks all dirty sectors as locked via
>> btrfs_folio_set_lock(), setting bits in the subpage locked bitmap and
>> incrementing nr_locked.  These are cleaned up by
>> btrfs_folio_end_lock_bitmap() at the end of extent_writepage().
>>
>> However, when btrfs_writepage_cow_fixup() returns -EAGAIN inside
>> extent_writepage_io(), the code calls folio_unlock() directly and
>> returns 1, causing extent_writepage() to skip the bitmap cleanup:
>>
>>      ret = btrfs_writepage_cow_fixup(folio);
>>      if (ret == -EAGAIN) {
>>          folio_redirty_for_writepage(bio_ctrl->wbc, folio);
>>          folio_unlock(folio);     // doesn't clear locked bitmap
>>          return 1;                // caller skips end_lock_bitmap()
>>      }
>>
>> This leaves the subpage locked bitmap out of sync with the folio lock
>> state: the folio is unlocked but its subpage locked bitmap still has
>> bits set and nr_locked is elevated.  When writeback retries the folio,
>> btrfs_folio_set_lock() hits the ASSERT at subpage.c:746 because the
>> bits are still set from the previous attempt.
>>
>> The cow_fixup path is largely a legacy path -- the GUP dirty-without-
>> informing-fs issue that triggered it has been fixed on the GUP side,
>> and experimental builds already catch this case with -EUCLEAN before
>> reaching the -EAGAIN return.  However the subpage state mismatch is
>> still a correctness issue for non-experimental builds under error
>> injection or memory pressure (kzalloc failure in
>> btrfs_writepage_cow_fixup()).
>>
>> Fix this by replacing folio_unlock() with btrfs_folio_end_lock_bitmap(),
>> which properly clears the locked bitmap bits before unlocking.  For
>> non-subpage or when nr_locked is 0 (e.g. called from
>> extent_write_locked_range()), btrfs_folio_end_lock_bitmap() falls
>> through to plain folio_unlock(), so existing behavior is preserved.
>>
>> Fixes: d034cdb4cc8a ("btrfs: lock subpage ranges in one go for writepage_delalloc()")
>> CC: stable@vger.kernel.org
>> Signed-off-by: Werner Kasselman <werner@verivus.com>
> 
> I'm going through patch backlog, this patch has some relevance. We're
> going to remove the fixup worker code in 7.2 completely so it cannot be
> applied to the development branch anymore.
> 
> The problems are hard to hit or need error injection, I don't know if
> it's worth to backport to stable.

I think the root fix, btrfs: check and set EXTENT_DELALLOC_NEW before 
clearing EXTENT_DELALLOC, is more relevant to backport.

And that fix is already CCed to stable, although only for 6.1+.
Older can be harder to backport.

With that fix backported, the cow fixup path will be a dead code, won't 
make any difference if fix the error path or not.

Thanks,
Qu

> We've provided a long grace period to
> the fixup worker before removal and I'm glad we can delete it and forget
> about it. If somebody wants one last fix then I'm OK with that.
>