Re: [PATCH v1 2/5] userfaultfd: introduce access-likely mode for common operations

[Peter Xu <peterx@redhat.com>]

On Fri, Jun 24, 2022 at 12:03:38AM +0000, Nadav Amit wrote:
> My take is that hints are hints. Following David’s (or was it yours?)
> feedback, I fixed the description to indicate that this is merely a hint and
> removed all references to dirty/access bits. The kernel therefore can ignore
> the hint when it wants to or use it in any other way. I fully agree that
> this gives the kernel the ability to change the behavior as needed.
> 
> Note that for write-protected 4KB zero-page (where we share the zero-page)
> we always set the access-bit, regardless of the hint, because it makes
> sense: the zero-page is not swappable and therefore the access-bit is set.

The zero-page example makes sense, and yeah that makes the hugetlb behavior
making more sense too.

> 
> I think that the lesser user-facing documentation there is on how the
> feature is *exactly* used by the kernel - is better from an API point of
> view.
> 
> So I see no reason to fail or be forced not to set a page as young, just
> because a hint was *not* provided. This would even be a regression in the
> behavior. The hint is actually always respected right now, it is just that
> even if you do not provide the hint, the access/dirty is set.
> 
> The only consistency I think worth thinking about is with the dirty-bit, and
> I can add it if you want. Note that the access-bit (in x86) might be set
> speculatively in contrast to the dirty-bit is only set atomically with a
> real access. That’s the reason I think it may make sense not to set the
> dirty without a hint.

Sorry to ask if this is (another) naive question: any link/help to explain
the speculative behavior on access bit?  Is it part of speculative
execution (which, iiuc, would it be reverted if the speculation failed)?

> 
> Is that acceptable? Access-bit always set, dirty-bit according to hint?

I'm still trying to digest what you said above, sorry.

Aren't both access and dirty bits need an atomic op to be set anyway?  Then
from perf pov should we simply keep setting them both too like what you did
with this version? because it seems that'll always avoid an extra pgtable
update access?

-- 
Peter Xu