[PATCH] ntfs: wait for bitmap folio writeback before modifying it

[PATCH] ntfs: wait for bitmap folio writeback before modifying it

[Shengzhuo Wei]

The bitmap update paths lock their page cache folios before modifying
them, but the folio lock does not wait for an earlier writeback to
finish.  Writeback can release the lock while its BIO still references
the folio memory, allowing bitmap changes to race with that I/O and
expose partially updated contents to it.

Wait for existing writeback after locking each folio and before
modifying it in ntfs_attr_set(), both bitmap setter loops, the cluster
allocator, and the two MFT bitmap allocation paths.  This follows the
lock-then-wait sequence already used for attribute and raw MFT updates.

Cc: stable@vger.kernel.org
Signed-off-by: Shengzhuo Wei <me@cherr.cc>
---
The change was checked against the existing attribute/MFT waits and
the iomap completion path.  Existing base and patched NTFS object
builds with W=1 and CONFIG_WERROR=y complete without warnings.  No
runtime reproduction or filesystem stress test was performed.
---
 fs/ntfs/attrib.c   | 2 ++
 fs/ntfs/bitmap.c   | 4 ++++
 fs/ntfs/lcnalloc.c | 2 ++
 fs/ntfs/mft.c      | 4 ++++
 4 files changed, 12 insertions(+)

diff --git a/fs/ntfs/attrib.c b/fs/ntfs/attrib.c
index fce7144d7bef..b460d2223314 100644
--- a/fs/ntfs/attrib.c
+++ b/fs/ntfs/attrib.c
@@ -2332,6 +2332,8 @@ int ntfs_attr_set(struct ntfs_inode *ni, s64 ofs, s64 cnt, const u8 val)
 		attr_len = min_t(size_t, (size_t)cnt, folio_size(folio) - offset);
 
 		folio_lock(folio);
+		if (folio_test_writeback(folio))
+			folio_wait_writeback(folio);
 		addr = kmap_local_folio(folio, offset);
 		memset(addr, val, attr_len);
 		kunmap_local(addr);
diff --git a/fs/ntfs/bitmap.c b/fs/ntfs/bitmap.c
index af86fb1c199b..057b315789ec 100644
--- a/fs/ntfs/bitmap.c
+++ b/fs/ntfs/bitmap.c
@@ -213,6 +213,8 @@ int __ntfs_bitmap_set_bits_in_run(struct inode *vi, const s64 start_bit,
 	}
 
 	folio_lock(folio);
+	if (folio_test_writeback(folio))
+		folio_wait_writeback(folio);
 	kaddr = kmap_local_folio(folio, 0);
 
 	/* Set @pos to the position of the byte containing @start_bit. */
@@ -277,6 +279,8 @@ int __ntfs_bitmap_set_bits_in_run(struct inode *vi, const s64 start_bit,
 		}
 
 		folio_lock(folio);
+		if (folio_test_writeback(folio))
+			folio_wait_writeback(folio);
 		kaddr = kmap_local_folio(folio, 0);
 		/*
 		 * Depending on @value, modify all remaining whole bytes in the
diff --git a/fs/ntfs/lcnalloc.c b/fs/ntfs/lcnalloc.c
index 3ebcac4c2864..aac8627a9283 100644
--- a/fs/ntfs/lcnalloc.c
+++ b/fs/ntfs/lcnalloc.c
@@ -348,6 +348,8 @@ struct runlist_element *ntfs_cluster_alloc(struct ntfs_volume *vol, const s64 st
 		}
 
 		folio_lock(folio);
+		if (folio_test_writeback(folio))
+			folio_wait_writeback(folio);
 		buf = kmap_local_folio(folio, 0) + pg_off;
 		ntfs_debug("Before inner while loop: buf_size %i, lcn 0x%llx, bmp_pos 0x%llx, need_writeback %i.",
 				buf_size, lcn, bmp_pos, need_writeback);
diff --git a/fs/ntfs/mft.c b/fs/ntfs/mft.c
index 89288b794348..b8aa8b0891dc 100644
--- a/fs/ntfs/mft.c
+++ b/fs/ntfs/mft.c
@@ -1206,6 +1206,8 @@ static s64 mft_bitmap_alloc_free_rec(struct ntfs_volume *vol,
 				return PTR_ERR(folio);
 			}
 			folio_lock(folio);
+			if (folio_test_writeback(folio))
+				folio_wait_writeback(folio);
 			buf = (u8 *)kmap_local_folio(folio, 0) + folio_ofs;
 			bit = data_pos & 7;
 			data_pos &= ~7ull;
@@ -1402,6 +1404,8 @@ static int ntfs_mft_bitmap_extend_allocation_nolock(struct ntfs_volume *vol)
 
 	down_write(&vol->lcnbmp_lock);
 	folio_lock(folio);
+	if (folio_test_writeback(folio))
+		folio_wait_writeback(folio);
 	b = (u8 *)kmap_local_folio(folio, 0) + (ll & ~PAGE_MASK);
 	tb = 1 << (lcn & 7ull);
 	/*

---
base-commit: d135ab453a7400a34930bdda1ef4bdc8acfac80e
change-id: 20261029-b4-ntfs-bitmap-writeback-wait-d4561fa54c97

Best regards,
--  
Shengzhuo Wei <me@cherr.cc>

Re: [PATCH] ntfs: wait for bitmap folio writeback before modifying it

[liubaolin]

在 2026/10/29 04:09, Shengzhuo Wei 写道:
> The bitmap update paths lock their page cache folios before modifying
> them, but the folio lock does not wait for an earlier writeback to
> finish.  Writeback can release the lock while its BIO still references
> the folio memory, allowing bitmap changes to race with that I/O and
> expose partially updated contents to it.
> 
> Wait for existing writeback after locking each folio and before
> modifying it in ntfs_attr_set(), both bitmap setter loops, the cluster
> allocator, and the two MFT bitmap allocation paths.  This follows the
> lock-then-wait sequence already used for attribute and raw MFT updates.
> 
> Cc: stable@vger.kernel.org
> Signed-off-by: Shengzhuo Wei <me@cherr.cc>
> ---
> The change was checked against the existing attribute/MFT waits and
> the iomap completion path.  Existing base and patched NTFS object
> builds with W=1 and CONFIG_WERROR=y complete without warnings.  No
> runtime reproduction or filesystem stress test was performed.
> ---
>   fs/ntfs/attrib.c   | 2 ++
>   fs/ntfs/bitmap.c   | 4 ++++
>   fs/ntfs/lcnalloc.c | 2 ++
>   fs/ntfs/mft.c      | 4 ++++
>   4 files changed, 12 insertions(+)
> 
> diff --git a/fs/ntfs/attrib.c b/fs/ntfs/attrib.c
> index fce7144d7bef..b460d2223314 100644
> --- a/fs/ntfs/attrib.c
> +++ b/fs/ntfs/attrib.c
> @@ -2332,6 +2332,8 @@ int ntfs_attr_set(struct ntfs_inode *ni, s64 ofs, s64 cnt, const u8 val)
>   		attr_len = min_t(size_t, (size_t)cnt, folio_size(folio) - offset);
>   
>   		folio_lock(folio);
> +		if (folio_test_writeback(folio))
> +			folio_wait_writeback(folio);
>   		addr = kmap_local_folio(folio, offset);
>   		memset(addr, val, attr_len);
>   		kunmap_local(addr);
> diff --git a/fs/ntfs/bitmap.c b/fs/ntfs/bitmap.c
> index af86fb1c199b..057b315789ec 100644
> --- a/fs/ntfs/bitmap.c
> +++ b/fs/ntfs/bitmap.c
> @@ -213,6 +213,8 @@ int __ntfs_bitmap_set_bits_in_run(struct inode *vi, const s64 start_bit,
>   	}
>   
>   	folio_lock(folio);
> +	if (folio_test_writeback(folio))
> +		folio_wait_writeback(folio);
>   	kaddr = kmap_local_folio(folio, 0);
>   
>   	/* Set @pos to the position of the byte containing @start_bit. */
> @@ -277,6 +279,8 @@ int __ntfs_bitmap_set_bits_in_run(struct inode *vi, const s64 start_bit,
>   		}
>   
>   		folio_lock(folio);
> +		if (folio_test_writeback(folio))
> +			folio_wait_writeback(folio);
>   		kaddr = kmap_local_folio(folio, 0);
>   		/*
>   		 * Depending on @value, modify all remaining whole bytes in the
> diff --git a/fs/ntfs/lcnalloc.c b/fs/ntfs/lcnalloc.c
> index 3ebcac4c2864..aac8627a9283 100644
> --- a/fs/ntfs/lcnalloc.c
> +++ b/fs/ntfs/lcnalloc.c
> @@ -348,6 +348,8 @@ struct runlist_element *ntfs_cluster_alloc(struct ntfs_volume *vol, const s64 st
>   		}
>   
>   		folio_lock(folio);
> +		if (folio_test_writeback(folio))
> +			folio_wait_writeback(folio);
>   		buf = kmap_local_folio(folio, 0) + pg_off;
>   		ntfs_debug("Before inner while loop: buf_size %i, lcn 0x%llx, bmp_pos 0x%llx, need_writeback %i.",
>   				buf_size, lcn, bmp_pos, need_writeback);
> diff --git a/fs/ntfs/mft.c b/fs/ntfs/mft.c
> index 89288b794348..b8aa8b0891dc 100644
> --- a/fs/ntfs/mft.c
> +++ b/fs/ntfs/mft.c
> @@ -1206,6 +1206,8 @@ static s64 mft_bitmap_alloc_free_rec(struct ntfs_volume *vol,
>   				return PTR_ERR(folio);
>   			}
>   			folio_lock(folio);
> +			if (folio_test_writeback(folio))
> +				folio_wait_writeback(folio);

Hi Shengzhuo,
    Could this deadlock during MFT bitmap 
expansion？ntfs_mft_bitmap_extend_allocation_nolock() can recurse into MFT
record allocation while holding mftbmp_ni->runlist.lock.

   With L being that lock, could we get the following interleaving?
A and B are $MFT::$BITMAP folios; B is outside the cached iomap.
Time flows downwards.

   Writeback                        Bitmap expansion
   ---------                        ----------------
   lock A
   set PG_writeback(A)
   map A (take/release L)
   add A to wpc->wb_ctx
   unlock A
   [BIO still unsubmitted]
                                    take L
                                    extend bitmap mapping pairs
                                    need another extent record
                                    ntfs_mft_record_alloc()
                                      mft_bitmap_alloc_free_rec()
                                        lock A
                                        folio_wait_writeback(A)
                                    [wait for A's I/O, holding L]

   lock B
   set PG_writeback(B)
   ntfs_writeback_range(B)
     __ntfs_write_iomap_begin()
       down_write(L)
       [blocked on L]

   Cannot reach iomap_add_to_ioend()
   to submit A's pending BIO.        Cannot release L until
                                    A's writeback completes.

    ntfs_writeback_range() obtains the next mapping before 
iomap_add_to_ioend() can submit the pending BIO.
    Would we need to submit that BIO before taking the mapping locks?

Thanks,
Baolin.


>   			buf = (u8 *)kmap_local_folio(folio, 0) + folio_ofs;
>   			bit = data_pos & 7;
>   			data_pos &= ~7ull;
> @@ -1402,6 +1404,8 @@ static int ntfs_mft_bitmap_extend_allocation_nolock(struct ntfs_volume *vol)
>   
>   	down_write(&vol->lcnbmp_lock);
>   	folio_lock(folio);
> +	if (folio_test_writeback(folio))
> +		folio_wait_writeback(folio);
>   	b = (u8 *)kmap_local_folio(folio, 0) + (ll & ~PAGE_MASK);
>   	tb = 1 << (lcn & 7ull);
>   	/*
> 
> ---
> base-commit: d135ab453a7400a34930bdda1ef4bdc8acfac80e
> change-id: 20261029-b4-ntfs-bitmap-writeback-wait-d4561fa54c97
> 
> Best regards,
> --
> Shengzhuo Wei <me@cherr.cc>

Re: [PATCH] ntfs: wait for bitmap folio writeback before modifying it

[Shengzhuo Wei]

On 2026-10-29 10:07, liubaolin wrote:
 
> Hi Shengzhuo,
>    Could this deadlock during MFT bitmap
> expansion？ntfs_mft_bitmap_extend_allocation_nolock() can recurse into MFT
> record allocation while holding mftbmp_ni->runlist.lock.
> 
>   With L being that lock, could we get the following interleaving?
> A and B are $MFT::$BITMAP folios; B is outside the cached iomap.
> Time flows downwards.
> 
>   Writeback                        Bitmap expansion
>   ---------                        ----------------
>   lock A
>   set PG_writeback(A)
>   map A (take/release L)
>   add A to wpc->wb_ctx
>   unlock A
>   [BIO still unsubmitted]
>                                    take L
>                                    extend bitmap mapping pairs
>                                    need another extent record
>                                    ntfs_mft_record_alloc()
>                                      mft_bitmap_alloc_free_rec()
>                                        lock A
>                                        folio_wait_writeback(A)
>                                    [wait for A's I/O, holding L]
> 
>   lock B
>   set PG_writeback(B)
>   ntfs_writeback_range(B)
>     __ntfs_write_iomap_begin()
>       down_write(L)
>       [blocked on L]
> 
>   Cannot reach iomap_add_to_ioend()
>   to submit A's pending BIO.        Cannot release L until
>                                    A's writeback completes.
> 
>    ntfs_writeback_range() obtains the next mapping before
> iomap_add_to_ioend() can submit the pending BIO.
>    Would we need to submit that BIO before taking the mapping locks?
> 
> Thanks,
> Baolin.
> 

Hi Baolin,

Thanks for the analysis. I checked the paths and agree with you.

Following your suggestion, I prepared a change to submit the pending
ioend before obtaining a new mapping in ntfs_writeback_range(), clearing
wpc->wb_ctx even on error to avoid resubmission. I plan to make this a
prerequisite for v2, retaining all six waits.

One concern is that submitting at mapping boundaries may reduce BIO
aggregation, including for ordinary file writeback. The change has
passed object builds, but has not been runtime- or performance-tested.

Does this approach look reasonable to you?

Thanks,
Shengzhuo

Re: [PATCH] ntfs: wait for bitmap folio writeback before modifying it

[liubaolin]

在 2026/10/29 14:17, Shengzhuo Wei 写道:
> On 2026-10-29 10:07, liubaolin wrote:
>   
>> Hi Shengzhuo,
>>     Could this deadlock during MFT bitmap
>> expansion？ntfs_mft_bitmap_extend_allocation_nolock() can recurse into MFT
>> record allocation while holding mftbmp_ni->runlist.lock.
>>
>>    With L being that lock, could we get the following interleaving?
>> A and B are $MFT::$BITMAP folios; B is outside the cached iomap.
>> Time flows downwards.
>>
>>    Writeback                        Bitmap expansion
>>    ---------                        ----------------
>>    lock A
>>    set PG_writeback(A)
>>    map A (take/release L)
>>    add A to wpc->wb_ctx
>>    unlock A
>>    [BIO still unsubmitted]
>>                                     take L
>>                                     extend bitmap mapping pairs
>>                                     need another extent record
>>                                     ntfs_mft_record_alloc()
>>                                       mft_bitmap_alloc_free_rec()
>>                                         lock A
>>                                         folio_wait_writeback(A)
>>                                     [wait for A's I/O, holding L]
>>
>>    lock B
>>    set PG_writeback(B)
>>    ntfs_writeback_range(B)
>>      __ntfs_write_iomap_begin()
>>        down_write(L)
>>        [blocked on L]
>>
>>    Cannot reach iomap_add_to_ioend()
>>    to submit A's pending BIO.        Cannot release L until
>>                                     A's writeback completes.
>>
>>     ntfs_writeback_range() obtains the next mapping before
>> iomap_add_to_ioend() can submit the pending BIO.
>>     Would we need to submit that BIO before taking the mapping locks?
>>
>> Thanks,
>> Baolin.
>>
> 
> Hi Baolin,
> 
> Thanks for the analysis. I checked the paths and agree with you.
> 
> Following your suggestion, I prepared a change to submit the pending
> ioend before obtaining a new mapping in ntfs_writeback_range(), clearing
> wpc->wb_ctx even on error to avoid resubmission. I plan to make this a
> prerequisite for v2, retaining all six waits.
> 
> One concern is that submitting at mapping boundaries may reduce BIO
> aggregation, including for ordinary file writeback. The change has
> passed object builds, but has not been runtime- or performance-tested.
> 
> Does this approach look reasonable to you?
> 
> Thanks,
> Shengzhuo

Hi Shengzhuo,

    That sounds reasonable to me.
    Clearing wpc->wb_ctx on error also makes sense, since the submit 
helper calls bio_endio() on that path.
    You could run some tests to check the performance impact.A 
before/after comparison of throughput and BIO sizes would help, 
especially where adjacent mappings are physically contiguous. Testing 
bitmap expansion alongside writeback would also help check the locking 
change.

Thanks,
Baolin