Re: [PATCH/RFC] btrfs: fix folio lock leak in writepage_delalloc() for folios dirtied behind btrfs' back

[Qu Wenruo]

在 2026/7/23 21:30, Matthew Wilcox 写道:
> On Thu, Jul 23, 2026 at 10:12:27AM +0930, Qu Wenruo wrote:
> > 在 2026/7/22 22:27, Matthew Wilcox 写道:
> > > On Wed, Jul 22, 2026 at 11:29:36AM +0200, Christian Borntraeger wrote:
> > > >    *   4. Thread B loops sync_file_range(WRITE|WAIT) on the target file.
> > > >    *      Whenever a full clean cycle (clear_page_dirty_for_io(),
> > > >    *      writeback, bits cleared) completes inside thread A's
> > > >    *      submission->completion window, the completion-time
> > > >    *      set_page_dirty_lock() hits a *clean* folio: filemap_dirty_folio()
> > > >    *      sets only the folio flag and the xarray tag - no btrfs subpage
> > > >    *      dirty bit, no delalloc reservation.  See the 20-year-old comment
> > > >    *      above bio_set_pages_dirty() in block/bio.c describing exactly
> > > >    *      this ("other code (eg, flusher threads) could clean the pages").
> > >
> > > There's your problem.  filemap_dirty_folio() documents that btrfs is
> > > doing it wrongly:
> > >
> > >    * Filesystems which do not use buffer heads should call this function
> > >    * from their dirty_folio address space operation.  It ignores the
> > >    * contents of folio_get_private(), so if the filesystem marks individual
> > >    * blocks as dirty, the filesystem should handle that itself.
> > >
> > > fs/btrfs/inode.c:       .dirty_folio    = filemap_dirty_folio,
> > >
> > > so btrfs should have its own btrfs_dirty_folio() which does whatever
> > > metadata updates it needs to and then call filemap_dirty_folio() to
> > > take care of the page cache business.  See iomap_dirty_folio() as
> > > an example, but many other filesystems also do this.
> >
> > Thanks a lot for the advice.
> >
> > However it looks like the sub-folio dirty block tracking is a little
> > different between iomap and btrfs.
>
> My point is not that "you should do it the exact same way as iomap".
> Rather "the dirty_folio op is the entry point to tell the filesystem
> that a folio is being dirtied".
And since dirty_folio() is not allowed to sleep, we should introduce 
some extra mechanism, e.g. page private 2/checked, to notify the fs that 
the folio is marked dirty without proper preparation.

Then during writeback, detect such folio and do needed preparation for 
it since at writeback we're allowed to sleep.

That sounds feasible, but I haven't seen anyone doing that (including 
the older btrfs cow fixup).

Will explore that path. Thanks a lot again for the dirty_folio() help.

Thanks,
Qu