All of lore.kernel.org
 help / color / mirror / Atom feed
From: John Hubbard <jhubbard@nvidia.com>
To: Rik van Riel <riel@surriel.com>,
	"David Hildenbrand (Arm)" <david@kernel.org>,
	linux-kernel@vger.kernel.org
Cc: kernel-team@meta.com, Andrew Morton <akpm@linux-foundation.org>,
	Jason Gunthorpe <jgg@ziepe.ca>, Peter Xu <peterx@redhat.com>,
	linux-mm@kvack.org
Subject: Re: [RFC PATCH v3 3/8] mm/gup: split follow_page_pte_commit() out of follow_page_pte()
Date: Fri, 21 Aug 2026 15:04:42 -0700	[thread overview]
Message-ID: <fe9f0bb5-73e3-4509-951d-e529e37011fd@nvidia.com> (raw)
In-Reply-To: <ab1661573257d7755d5a48e4f9165ff22febba29.camel@surriel.com>

On 8/21/26 10:38 AM, Rik van Riel wrote:
> On Wed, 2026-08-12 at 15:23 +0200, David Hildenbrand (Arm) wrote:
>> On 8/12/26 15:02, Rik van Riel wrote:
...
> 
> Thinking about it some more, we are marking the 
> folio dirty before (potentially long before) a write
> is actually done.
> 
> Doesn't that give the flushing code time to clean
> the page (by writing out content identical to what
> is already on disk) before the actual dirtying of
> the memory, through e.g. PTRACE_POKE is done?
> 
> Does the dirtying of folios and/or pages need to
> be done when they are actually being written to,
> and not at GUP time?
> 
> It looks like __access_remote_vm() already does
> that dirtying, as do most of the other places
> that use GUP with FOLL_WRITE.
> 
> A number of places use unpin_user_pages_dirty(_lock)
> to get the pages marked dirty at unpin time, after
> the data has been written into the pages.
> 
> However, there seem to be some places in the
> kernel that are not marking pages dirty when
> they write after GUP with FOLL_WRITE.
> 
> I suppose we need to fix those, given how
> sometimes they can pin pages for a long time,
> and do multiple writes over a long period of
> time?
> 
> I'm combing through the tree now to find those
> places.
> 
> This raises the question whether we want to
> call mark_folio_dirty() at all from inside
> GUP.
> 

Hi Rik,

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.

thanks,
-- 
John Hubbard



  reply	other threads:[~2026-08-21 22:04 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-11  2:51 [RFC PATCH v3 0/8] batch lookups in follow_page_mask() Rik van Riel
2026-08-11  2:51 ` [RFC PATCH v3 1/8] mm/gup: break out gup_fill_pages() helper Rik van Riel
2026-08-11  2:51 ` [RFC PATCH v3 2/8] mm/gup: convert follow_page_mask() to return a long Rik van Riel
2026-08-11  2:51 ` [RFC PATCH v3 3/8] mm/gup: split follow_page_pte_commit() out of follow_page_pte() Rik van Riel
2026-08-12 11:50   ` David Hildenbrand (Arm)
2026-08-12 13:02     ` Rik van Riel
2026-08-12 13:23       ` David Hildenbrand (Arm)
2026-08-12 16:19         ` Rik van Riel
2026-08-21 17:38         ` Rik van Riel
2026-08-21 22:04           ` John Hubbard [this message]
2026-08-11  2:51 ` [RFC PATCH v3 4/8] mm/gup: break out follow_one_pte() helper Rik van Riel
2026-08-11  2:51 ` [RFC PATCH v3 5/8] mm/gup: fill the pages array outside the pud/pmd lock Rik van Riel
2026-08-11  2:51 ` [RFC PATCH v3 6/8] mm/gup: return a huge page's full count from follow_page_mask() Rik van Riel
2026-08-11  2:51 ` [RFC PATCH v3 7/8] mm/gup: walk multiple PTEs per follow_page_pte() call Rik van Riel
2026-08-11  2:51 ` [RFC PATCH v3 8/8] mm/gup: batch contiguous same-folio PTEs into one refcount grab Rik van Riel
2026-08-12 11:41 ` [RFC PATCH v3 0/8] batch lookups in follow_page_mask() David Hildenbrand (Arm)

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=fe9f0bb5-73e3-4509-951d-e529e37011fd@nvidia.com \
    --to=jhubbard@nvidia.com \
    --cc=akpm@linux-foundation.org \
    --cc=david@kernel.org \
    --cc=jgg@ziepe.ca \
    --cc=kernel-team@meta.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=peterx@redhat.com \
    --cc=riel@surriel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.