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

[Boris Burkov]

On Fri, Jul 24, 2026 at 08:10:36AM +0930, Qu Wenruo wrote:
> 
> 
> 在 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

Here is my proposal for a candidate fix. It passes the reproducer in
this thread as well as several more intense reproducers (alluded to but
not yet included)

https://lore.kernel.org/linux-btrfs/6758d4f27be0bbdb865cee7dd5adc435c969f4a3.1784960646.git.boris@xxxxxx/T/#u

Thanks,
Boris