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