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

[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 you aren't taking advantage of
that, you're just calling the VFS so the VFS can do its own tracking.

> E.g. iomap will mark the full folio range dirty even if the EOF is inside
> the folio, but btrfs will only mark the range inside EOF as dirty.
> 
> 
> Another thing is, even if we follow iomap to mark the full folio dirty, it's
> still not the end of the story.
> 
> We have other supporting mechanisms required to tracking the dirty range.
> E.g. EXTENT_DELALLOC flags inside extent-io-tree, indicating we have already
> reserved space for the dirty range.
> 
> Only with EXTENT_DELALLOC flag set, we will do the real delayed allocation,
> allocating the on-disk extents etc.
> 
> So even if we always mark the full folio dirty, the writeback path will not
> handle them correctly either.
> 
> 
> Finally, even without large folios, the reproducer can already cause
> problems on btrfs. E.g. on x86_64, with mapping_set_folio_order_range()
> disabled.
> 
> The symptom there is that, ordered extent accounting underflows, which may
> also contribute to the stall observed.
> 
> I'll keep digging for this bug.
> 
> Thanks,
> Qu
> 
>