Re: [PATCH v8 22/23] mm: Enable PTE markers by default

From: Johannes Weiner <hannes@cmpxchg.org>
To: Peter Xu <peterx@redhat.com>
Cc: linux-kernel@vger.kernel.org, linux-mm@kvack.org,
	Mike Kravetz <mike.kravetz@oracle.com>,
	Nadav Amit <nadav.amit@gmail.com>,
	Matthew Wilcox <willy@infradead.org>,
	Mike Rapoport <rppt@linux.vnet.ibm.com>,
	David Hildenbrand <david@redhat.com>,
	Hugh Dickins <hughd@google.com>,
	Jerome Glisse <jglisse@redhat.com>,
	"Kirill A . Shutemov" <kirill@shutemov.name>,
	Andrea Arcangeli <aarcange@redhat.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Axel Rasmussen <axelrasmussen@google.com>,
	Alistair Popple <apopple@nvidia.com>
Subject: Re: [PATCH v8 22/23] mm: Enable PTE markers by default
Date: Tue, 19 Apr 2022 17:24:28 -0400	[thread overview]
Message-ID: <Yl8ojDwxsD/wqWM4@cmpxchg.org> (raw)
In-Reply-To: <Yl8bYKOJGW2py7Q0@xz-m1.local>

On Tue, Apr 19, 2022 at 04:28:16PM -0400, Peter Xu wrote:
> On Tue, Apr 19, 2022 at 04:14:11PM -0400, Johannes Weiner wrote:
> > On Tue, Apr 19, 2022 at 03:59:21PM -0400, Peter Xu wrote:
> > > @@ -910,16 +910,16 @@ config ANON_VMA_NAME
> > Btw, this doesn't do much without userfaultfd being enabled in
> > general, right?
> 
> So far yes, but I'm thinking there can be potential other users of
> PTE_MARKERS from mm world.  The most close discussion is on the swap read
> failures and this patch proposed by Miaohe:
> 
> https://lore.kernel.org/lkml/20220416030549.60559-1-linmiaohe@huawei.com/
>
> So I hope we can still keep them around here under mm/ if possible, and
> from the gut feeling it really should..

Agreed, mm/ seems a good fit for PTE_MARKER.

If it's invisible and gets selected as needed, it's less of a concern,
IMO. I'm somewhat worried about when and how the user-visible options
show up right now, though...

> > Would it make sense to have it next to 'config USERFAULTFD' as a
> > sub-option?
> 
> Yes another good question. :)
> 
> IIUC CONFIG_USERFAULTFD resides in init/Kconfig because it introduces a new
> syscall.  Same to the rest of the bits for uffd since then, namely:
> 
>   - USERFAULTFD_WP
>   - USERFAULTFD_MINOR
> 
> What I am thinking now is the other way round of your suggestion: whether
> we should move most of them out, at least the _WP and _MINOR configs into
> mm/?  Because IMHO they are really pure mm ideas and they're irrelevant to
> syscalls and init.

I'm thinking the MM submenu would probably be a better fit for all
user-visible userfaultfd options, including the syscall. Like you say,
it's an MM concept.

But if moving the syscall knob out from init isn't popular, IMO it
would be better to add the new WP option to init as well. This ensures
that when somebody selects userfaultfd, they also see the relevant
suboptions and don't have to chase them down across multiple submenus.

Conversely, they should also have the necessary depend clauses so that
suboptions aren't visible without the main feature. E.g. it asked me
for userfaultd options even though I have CONFIG_USERFAULTFD=n.

What do you think?