Re: [RFC PATCH v3 3/8] mm/gup: split follow_page_pte_commit() out of follow_page_pte()

On 8/22/26 6:20 AM, Rik van Riel wrote:
> On Fri, 2026-08-21 at 15:04 -0700, John Hubbard wrote:
>>
>> You have delved too deep, and now uncovered something that's been
>> a problem for the whole time. :)
>>
>> Specifically, "no, GUP should not be setting folios nor pages
>> dirty", because that generally needs to be done as part of a
>> filesystem
>> coordinated set of steps. If the page is dirty, and the filesystem
>> didn't expect it to be, that leads to problems.
>>
>> This is part of the big, remaining set of required fixes, that
>> launched the creation of pin_user_pages*() and related. Connecting
>> up the filesystems properly is yet to be done. And until then, this
>> is a real defect.
>>
> Not just filesystems.
> 
> I found about two dozen places where drivers
> fail to mark a page dirty after writing to
> it, after obtaining the page from GUP.
> 

Yes, I agree about that problem, but it's not the only one. It turns
out that the only thing worse than failing to mark a page dirty is
marking it dirty without coordinating with the filesystem.

> I'll get out patches for that, unless there's
> some reason I shouldn't.
> 

Well...on a file-backed page, marking it dirty is the *last* step of
a sequence, and a driver cannot run the earlier steps.

ext4_page_mkwrite() into ext4_block_page_mkwrite() runs it in this
order:

	sb_start_pagefault(inode->i_sb);	/* freeze protection */
	file_update_time(vma->vm_file);
	filemap_invalidate_lock_shared(mapping);	/* truncate */
	ext4_journal_start(inode, EXT4_HT_WRITE_PAGE, credits);
	ext4_block_write_begin(handle, folio, 0, len, get_block);
	folio_mark_dirty(folio);
	ext4_journal_stop(handle);
	folio_wait_stable(folio);

A set_page_dirty() at the driver's write site is that
folio_mark_dirty(), but with none of the preparation above it.

ext4 documents this case already, in ext4_journalled_dirty_folio() on
the data=journal path:

  * ... except for the case when someone
  * had the page pinned and dirtied the page through this pin (e.g. by doing
  * direct IO to it). In that case we'd need to attach buffers here to the
  * transaction but we cannot due to lock ordering.

Its workaround is folio_set_checked(), which defers the real work to
ext4_writepages(). If ext4 can't do the preparation from inside the
dirty call, a driver can't either.

So for a file-backed page there's nothing the driver can add. What's
missing is a way for the filesystem to be told before the device
writes, and to revoke the pin when it needs to, which is where the
lease proposals come in. None of that exists today.

And yes, unpin_user_pages_dirty_lock() is in the same awkward mess.

thanks,
-- 
John Hubbard