Message-ID: <22e9e910-630a-41c9-bf6d-aacf7c5183f5@lucifer.local>
Date: Tue, 20 Jan 2026 16:46:28 +0000
From: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
To: Jason Gunthorpe <jgg@...dia.com>
Cc: Andrew Morton <akpm@...ux-foundation.org>,
        Jarkko Sakkinen <jarkko@...nel.org>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        Thomas Gleixner <tglx@...nel.org>, Ingo Molnar <mingo@...hat.com>,
        Borislav Petkov <bp@...en8.de>, x86@...nel.org,
        "H . Peter Anvin" <hpa@...or.com>, Arnd Bergmann <arnd@...db.de>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Dan Williams <dan.j.williams@...el.com>,
        Vishal Verma <vishal.l.verma@...el.com>,
        Dave Jiang <dave.jiang@...el.com>,
        Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
        Maxime Ripard <mripard@...nel.org>,
        Thomas Zimmermann <tzimmermann@...e.de>,
        David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
        Jani Nikula <jani.nikula@...ux.intel.com>,
        Joonas Lahtinen <joonas.lahtinen@...ux.intel.com>,
        Rodrigo Vivi <rodrigo.vivi@...el.com>,
        Tvrtko Ursulin <tursulin@...ulin.net>,
        Christian Koenig <christian.koenig@....com>,
        Huang Rui <ray.huang@....com>, Matthew Auld <matthew.auld@...el.com>,
        Matthew Brost <matthew.brost@...el.com>,
        Alexander Viro <viro@...iv.linux.org.uk>,
        Christian Brauner <brauner@...nel.org>, Jan Kara <jack@...e.cz>,
        Benjamin LaHaise <bcrl@...ck.org>, Gao Xiang <xiang@...nel.org>,
        Chao Yu <chao@...nel.org>, Yue Hu <zbestahu@...il.com>,
        Jeffle Xu <jefflexu@...ux.alibaba.com>,
        Sandeep Dhavale <dhavale@...gle.com>,
        Hongbo Li <lihongbo22@...wei.com>, Chunhai Guo <guochunhai@...o.com>,
        Theodore Ts'o <tytso@....edu>,
        Andreas Dilger <adilger.kernel@...ger.ca>,
        Muchun Song <muchun.song@...ux.dev>,
        Oscar Salvador <osalvador@...e.de>,
        David Hildenbrand <david@...nel.org>,
        Konstantin Komarov <almaz.alexandrovich@...agon-software.com>,
        Mike Marshall <hubcap@...ibond.com>,
        Martin Brandenburg <martin@...ibond.com>,
        Tony Luck <tony.luck@...el.com>,
        Reinette Chatre <reinette.chatre@...el.com>,
        Dave Martin <Dave.Martin@....com>, James Morse <james.morse@....com>,
        Babu Moger <babu.moger@....com>, Carlos Maiolino <cem@...nel.org>,
        Damien Le Moal <dlemoal@...nel.org>,
        Naohiro Aota <naohiro.aota@....com>,
        Johannes Thumshirn <jth@...nel.org>,
        Matthew Wilcox <willy@...radead.org>,
        "Liam R . Howlett" <Liam.Howlett@...cle.com>,
        Vlastimil Babka <vbabka@...e.cz>, Mike Rapoport <rppt@...nel.org>,
        Suren Baghdasaryan <surenb@...gle.com>, Michal Hocko <mhocko@...e.com>,
        Hugh Dickins <hughd@...gle.com>,
        Baolin Wang <baolin.wang@...ux.alibaba.com>, Zi Yan <ziy@...dia.com>,
        Nico Pache <npache@...hat.com>, Ryan Roberts <ryan.roberts@....com>,
        Dev Jain <dev.jain@....com>, Barry Song <baohua@...nel.org>,
        Lance Yang <lance.yang@...ux.dev>, Jann Horn <jannh@...gle.com>,
        Pedro Falcato <pfalcato@...e.de>, David Howells <dhowells@...hat.com>,
        Paul Moore <paul@...l-moore.com>, James Morris <jmorris@...ei.org>,
        "Serge E . Hallyn" <serge@...lyn.com>,
        Yury Norov <yury.norov@...il.com>,
        Rasmus Villemoes <linux@...musvillemoes.dk>, linux-sgx@...r.kernel.org,
        linux-kernel@...r.kernel.org, nvdimm@...ts.linux.dev,
        linux-cxl@...r.kernel.org, dri-devel@...ts.freedesktop.org,
        intel-gfx@...ts.freedesktop.org, linux-fsdevel@...r.kernel.org,
        linux-aio@...ck.org, linux-erofs@...ts.ozlabs.org,
        linux-ext4@...r.kernel.org, linux-mm@...ck.org, ntfs3@...ts.linux.dev,
        devel@...ts.orangefs.org, linux-xfs@...r.kernel.org,
        keyrings@...r.kernel.org, linux-security-module@...r.kernel.org
Subject: Re: [PATCH RESEND 09/12] mm: make vm_area_desc utilise vma_flags_t
 only

On Tue, Jan 20, 2026 at 11:22:45AM -0400, Jason Gunthorpe wrote:
> On Tue, Jan 20, 2026 at 03:10:54PM +0000, Lorenzo Stoakes wrote:
> > The natural implication of what you're saying is that we can no longer use this
> > from _anywhere_ because - hey - passing this by value is bad so now _everything_
> > has to be re-written as:
>
> No, I'm not saying that, I'm saying this specific case where you are
> making an accessor to reach an unknown value located on the heap

OK it would have been helpful for you to say that! Sometimes reviews feel
like a ratcheting series of 'what do you actually mean?'s... :)

> should be using a pointer as both a matter of style and to simplify
> life for the compiler.

OK fine.

>
> > 	vma_flags_t flags_to_set = mk_vma_flags(<flags>);
> >
> > 	if (vma_flags_test(&flags, &flags_to_set)) { ... }
>
> This is quite a different situation, it is a known const at compile
> time value located on the stack.

Well as a const time thing it'll be optimised to just a value assuming
nothing changes flags_to_set in the mean time. You'd hope.

Note that we have xxx_mask() variants, such that you can do, e.g.:

	vma_flags_t flags1 = mk_vma_flags(...);
	vma_flags_t flags2 = mk_vma_flags(...);

	if (vma_flags_test_mask(flags1, flags2)) {
		...
	}

ASIDE ->
	NOTE: A likely use of this, and one I already added is so we can do
	e.g.:

	#define VMA_REMAP_FLAGS mk_vma_flags(VMA_IO_BIT, VMA_PFNMAP_BIT, \
		VMA_DONTEXPAND_BIT, VMA_DONTDUMP_BIT)

	...

	if (vma_flagss_test_mask(flags, VMA_REMAP_FLAGS)) { ... }

	Which would be effectively a const input anyway.
<- ASIDE

Or in a world where flags1 is a const pointer now:

	if (vma_flags_test_mask(&flags1, flags2)) { ... }

Which makes the form... kinda weird. Then again it's consistent with other
forms which update flags1, ofc we name this separately, e.g. flags, to_test
or flags, to_set so I guess not such a problme.

Now, nobody is _likely_ to do e.g.:

	if (vma_flags_test_mask(&vma1->flags, vma2->flags)) { ... }

In this situation, but they could.

However perhaps having one value pass-by-const-pointer and the other
by-value essentially documents the fact you're being dumb.

And if somebody really needs something like this (not sure why) we could
add something.

But yeah ok, I'll change this. It's more than this case it's also all the
test stuff but shouldn't be a really huge change.

>
> > If it was just changing this one function I'd still object as it makes it differ
> > from _every other test predicate_ using vma_flags_t but maybe to humour you I'd
> > change it, but surely by this argument you're essentially objecting to the whole
> > series?
>
> I only think that if you are taking a heap input that is not of known
> value you should continue to pass by pointer as is generally expected
> in the C style we use.

Ack.

>
> And it isn't saying anything about the overall technique in the
> series, just a minor note about style.

OK good, though Arnd's reply feels more like a comment on the latter,
though only really doing pass-by-value for const values (in nearly all sane
cases) should hopefully mitigate.

>
> > I am not sure about this 'idiomatic kernel style' thing either, it feels rather
> > conjured. Yes you wouldn't ordinarily pass something larger than a register size
> > by-value, but here the intent is for it to be inlined anyway right?
>
> Well, exactly, we don't normally pass things larger than an interger
> by value, that isn't the style, so I don't think it is such a great
> thing to introduce here kind of unnecessarily.
>
> The troubles I recently had were linked to odd things like gcov and
> very old still supported versions of gcc. Also I saw a power compiler
> make a very strange choice to not inline something that evaluated to a
> constant.

Right ok.

>
> Jason

Thanks, Lorenzo

Powered by blists - more mailing lists