Re: Removing ->dirty_folio

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.

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?

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 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/

Thanks,
Boris

> 
> The problem is GUP.  We have no way to force the GUP caller to go
> through page_mkwrite again.  So instead we make the GUP caller call
> folio_mark_dirty_lock() which many just don't, and generally we get away
> with it.  But it's a bug, and a bad interface.
> 
> There's also the problem that GUP users bypass the folio_wait_stable()
> mechanism.  If a page is written to while somebody is creating a
> checksum over that page, the checksum will be corrupted.  If we want
> to fix this, we have to bounce-buffer the page.  There's no way to
> prevent or delay a GUP user from writing to the page.  Enjoy your RAID.
> 
> My proposal is this:
> 
>  - Fileystems take note of folio_maybe_dma_pinned() during writeback.
>    If it's true, do the writeback, but retain/recreate whatever data
>    structures you need in order to write the folio again; behave as if
>    ->page_mkdirty() had been called again for each page in the folio is
>    marked as dirty.
>  - The MM behaves similarly; we do not clear the writeback flag for
>    folio_maybe_dma_pinned().
> 
> This will have the effect of writing pinned folios back every time the
> inode is scheduled for writeback.  But since we have no idea whether
> the folio is actually dirty (because the GUP user won't tell us),
> this is the correct behaviour.
> 
> I'm probably missing some stuff here.  Let me know.