Re: Removing ->dirty_folio

[Matthew Wilcox <willy@infradead.org>]

On Mon, Aug 24, 2026 at 02:27:27PM -0700, Boris Burkov wrote:
> On Mon, Aug 24, 2026 at 08:08:06PM +0100, Matthew Wilcox wrote:
> > I think it's time to remove folio_mark_dirty(), ->dirty_folio() and so on.
> > 
> > This is not how filesystems want to be informed of folio dirtying.
> > It was fine for ext2, but anything that's journalled or COW has work
> > to do before the folio is made dirty, and it's hard to do that work
> > under the page table spinlock (not all callers hold that lock, but the
> > filesystem has to be able to handle the cases where it is.
> > 
> > Filesystems want the page_mkwrite() entry point to be how they find out
> > about a folio being dirtied -- and that works great!  Except that we
> > can writeback the folio for a number of reasons.  If it's been dirtied
> > due to a shared writable mmap, that's fine; we map the folio read-only
> > and any subsequent writes will re-enter the page_mkwrite path.
> 
> Can you elaborate on this part a bit more? I can't tell if you are
> proposing a change or saying the existing behavior is fine if we drop
> ->dirty_folio(). I am also confused about exactly what sort of folio
> dirtying you are referring to. Sorry if I am being obtuse.

Sorry for not being clearer.  This is what happens today.  We first
tell the FS that we're going to write to the page through
page_mkwrite(), then we transfer the dirty bit from the PTE to
the folio through ->dirty_folio() ... but it's already there, thanks
to the call to page_mkwrite()!

> When I was recently adding ->dirty_folio() to btrfs, one of the main
> cases was the call to folio_mark_dirty() that came via
> __iomap_dio_bio_end_io() calling bio_check_pages_dirty() which schedules
> bio_dirty_fn(). (i.e., completion of a dio read into a shared mmap)
> 
> Is that the case you are referring to here, or are you referring to
> someone just modifying a byte they faulted in from a shared mmap? The
> latter I would expect to have called page_mkwrite in the fault and done
> fs-specific work, so I assume it's the former that you are referring to?

Right.  The former, I _believe_ already calls page_mkwrite() today, and so
the call to dirty_folio() is redundant.  Except if writeback came in
while the I/O was in progress.  So if we preserve the dirty bit for
pinned pages in the writeback code, the caall to dirty_folio() will
always be redundant and can be removed.

> Either way, I do believe that for the dio read endio case pinning is not
> involved and btrfs relies on the ->dirty_folio() call, so I think
> something would need to be done about that case too.

I traced that code path down into iov_iter_extract_user_pages() which
calls pin_user_pages_fast(), so I do think the pages you're talking
about are pinned and would be handled through page_mkwrite() already.

iomap_dio_rw
__iomap_dio_rw
iomap_dio_iter
iomap_dio_bio_iter
iomap_dio_bio_iter_one
bio_iov_iter_get_pages
iov_iter_extract_bvecs
iov_iter_extract_pages
iov_iter_extract_user_pages

> I believe you saw this patch since it was your idea for us to use
> ->dirty_folio(), but just for reference for anyone else who didn't see
> it, the btrfs patch adding ->dirty_folio():
> https://lore.kernel.org/linux-btrfs/69d0043e0f6a3d17048dfde857127ab0bf331154.1785190866.git.boris@bur.io/

Yes, I want you to be able to revert that patch ;-)
>