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@xxxxxxxxxxxxxxx
> Signed-off-by: Shengzhuo Wei <me@xxxxxxxx>
> ---
> 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@xxxxxxxx>