[PATCH] mm: avoid use of BIT() macro for initialising VMA flags

[PATCH] mm: avoid use of BIT() macro for initialising VMA flags

[Lorenzo Stoakes]

Commit 2b6a3f061f11 ("mm: declare VMA flags by bit") significantly changed
how VMA flags are declared, utilising an enum of VMA bit values and
ifdef-fery VM_xxx flag declarations via macro.

As part of this change, it uses INIT_VM_FLAG() to define VM_xxx flags from
the newly introduced VMA bit numbers.

However, use of this macro results in apparently unfortunate macro
expansion and resulted in a performance degradation.This appears to be due
to the (__force int), which is required for the sparse typechecking to
work.

Avoid macro expansion issues by simply using 1UL << bitnum.

Reported-by: kernel test robot <oliver.sang@intel.com>
Closes: https://lore.kernel.org/oe-lkp/202512041634.150c7e4f-lkp@intel.com
Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
---

Andrew - note I've referenced the linux-next commit number above, could you
replace with the upstream commit hash once your PR is taken? Thanks!

 include/linux/mm.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index a2f38fb68840..c4438b30c140 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -395,7 +395,8 @@ enum {
 #undef DECLARE_VMA_BIT
 #undef DECLARE_VMA_BIT_ALIAS

-#define INIT_VM_FLAG(name) BIT((__force int) VMA_ ## name ## _BIT)
+#define INIT_VM_FLAG(name) (1UL << (__force int)(VMA_ ## name ## _BIT))
+
 #define VM_READ		INIT_VM_FLAG(READ)
 #define VM_WRITE	INIT_VM_FLAG(WRITE)
 #define VM_EXEC		INIT_VM_FLAG(EXEC)
--
2.52.0

Re: [PATCH] mm: avoid use of BIT() macro for initialising VMA flags

[Lorenzo Stoakes]

On Fri, Dec 05, 2025 at 05:50:37PM +0000, Lorenzo Stoakes wrote:
> Commit 2b6a3f061f11 ("mm: declare VMA flags by bit") significantly changed
> how VMA flags are declared, utilising an enum of VMA bit values and
> ifdef-fery VM_xxx flag declarations via macro.
>
> As part of this change, it uses INIT_VM_FLAG() to define VM_xxx flags from
> the newly introduced VMA bit numbers.
>
> However, use of this macro results in apparently unfortunate macro
> expansion and resulted in a performance degradation.This appears to be due
> to the (__force int), which is required for the sparse typechecking to
> work.
>
> Avoid macro expansion issues by simply using 1UL << bitnum.
>
> Reported-by: kernel test robot <oliver.sang@intel.com>
> Closes: https://lore.kernel.org/oe-lkp/202512041634.150c7e4f-lkp@intel.com
> Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>

Sorry forget to add Fixes tag... :)

Please update with that also when the PR is upstream apologies :P

> ---
>
> Andrew - note I've referenced the linux-next commit number above, could you
> replace with the upstream commit hash once your PR is taken? Thanks!
>
>  include/linux/mm.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index a2f38fb68840..c4438b30c140 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -395,7 +395,8 @@ enum {
>  #undef DECLARE_VMA_BIT
>  #undef DECLARE_VMA_BIT_ALIAS
>
> -#define INIT_VM_FLAG(name) BIT((__force int) VMA_ ## name ## _BIT)
> +#define INIT_VM_FLAG(name) (1UL << (__force int)(VMA_ ## name ## _BIT))
> +
>  #define VM_READ		INIT_VM_FLAG(READ)
>  #define VM_WRITE	INIT_VM_FLAG(WRITE)
>  #define VM_EXEC		INIT_VM_FLAG(EXEC)
> --
> 2.52.0

Cheers, Lorenzo

Re: [PATCH] mm: avoid use of BIT() macro for initialising VMA flags

[David Laight]

On Fri,  5 Dec 2025 17:50:37 +0000
Lorenzo Stoakes <lorenzo.stoakes@oracle.com> wrote:

> Commit 2b6a3f061f11 ("mm: declare VMA flags by bit") significantly changed
> how VMA flags are declared, utilising an enum of VMA bit values and
> ifdef-fery VM_xxx flag declarations via macro.
> 
> As part of this change, it uses INIT_VM_FLAG() to define VM_xxx flags from
> the newly introduced VMA bit numbers.
> 
> However, use of this macro results in apparently unfortunate macro
> expansion and resulted in a performance degradation.This appears to be due
> to the (__force int), which is required for the sparse typechecking to
> work.

Does sparse complain if you just add 0? As in:
#define INIT_VM_FLAG(name) BIT(VMA_ ## name ## _BIT + 0u)

That should change the type without affecting what BIT() expands to.

	David

> 
> Avoid macro expansion issues by simply using 1UL << bitnum.
> 
> Reported-by: kernel test robot <oliver.sang@intel.com>
> Closes: https://lore.kernel.org/oe-lkp/202512041634.150c7e4f-lkp@intel.com
> Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> ---
> 
> Andrew - note I've referenced the linux-next commit number above, could you
> replace with the upstream commit hash once your PR is taken? Thanks!
> 
>  include/linux/mm.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index a2f38fb68840..c4438b30c140 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -395,7 +395,8 @@ enum {
>  #undef DECLARE_VMA_BIT
>  #undef DECLARE_VMA_BIT_ALIAS
> 
> -#define INIT_VM_FLAG(name) BIT((__force int) VMA_ ## name ## _BIT)
> +#define INIT_VM_FLAG(name) (1UL << (__force int)(VMA_ ## name ## _BIT))
> +
>  #define VM_READ		INIT_VM_FLAG(READ)
>  #define VM_WRITE	INIT_VM_FLAG(WRITE)
>  #define VM_EXEC		INIT_VM_FLAG(EXEC)
> --
> 2.52.0
> 

Re: [PATCH] mm: avoid use of BIT() macro for initialising VMA flags

[Lorenzo Stoakes]

On Fri, Dec 05, 2025 at 06:43:42PM +0000, David Laight wrote:
> On Fri,  5 Dec 2025 17:50:37 +0000
> Lorenzo Stoakes <lorenzo.stoakes@oracle.com> wrote:
>
> > Commit 2b6a3f061f11 ("mm: declare VMA flags by bit") significantly changed
> > how VMA flags are declared, utilising an enum of VMA bit values and
> > ifdef-fery VM_xxx flag declarations via macro.
> >
> > As part of this change, it uses INIT_VM_FLAG() to define VM_xxx flags from
> > the newly introduced VMA bit numbers.
> >
> > However, use of this macro results in apparently unfortunate macro
> > expansion and resulted in a performance degradation.This appears to be due
> > to the (__force int), which is required for the sparse typechecking to
> > work.
>
> Does sparse complain if you just add 0? As in:
> #define INIT_VM_FLAG(name) BIT(VMA_ ## name ## _BIT + 0u)
>
> That should change the type without affecting what BIT() expands to.

Thanks, checked that and unfortunately that doesn't satisfy sparse :)

I don't think it's too crazy to use 1UL << here, just very frustrating (TM)
that this is an issue.

<insert rant about C macros here>

Cheers, Lorenzo

Re: [PATCH] mm: avoid use of BIT() macro for initialising VMA flags

[David Laight]

On Fri, 5 Dec 2025 19:18:56 +0000
Lorenzo Stoakes <lorenzo.stoakes@oracle.com> wrote:

> On Fri, Dec 05, 2025 at 06:43:42PM +0000, David Laight wrote:
> > On Fri,  5 Dec 2025 17:50:37 +0000
> > Lorenzo Stoakes <lorenzo.stoakes@oracle.com> wrote:
> >  
> > > Commit 2b6a3f061f11 ("mm: declare VMA flags by bit") significantly changed
> > > how VMA flags are declared, utilising an enum of VMA bit values and
> > > ifdef-fery VM_xxx flag declarations via macro.
> > >
> > > As part of this change, it uses INIT_VM_FLAG() to define VM_xxx flags from
> > > the newly introduced VMA bit numbers.
> > >
> > > However, use of this macro results in apparently unfortunate macro
> > > expansion and resulted in a performance degradation.This appears to be due
> > > to the (__force int), which is required for the sparse typechecking to
> > > work.  
> >
> > Does sparse complain if you just add 0? As in:
> > #define INIT_VM_FLAG(name) BIT(VMA_ ## name ## _BIT + 0u)
> >
> > That should change the type without affecting what BIT() expands to.  
> 
> Thanks, checked that and unfortunately that doesn't satisfy sparse :)
> 
> I don't think it's too crazy to use 1UL << here, just very frustrating (TM)
> that this is an issue.

I might use some of my copious spare time (ha) to see why BIT() fails.
I bet it is just too complex for its own good.
Personally I'm fine with both explicit (1ul << n) and hex constants.
The latter are definitely most useful if you ever look at hexdumps.

At the moment I'm trying to fix bitfield.h so you don't get compile errors
on lines that are 18KB long.

Found a new version in linux-next - has its own set of new bugs as well
as more of the old ones.

	David

> 
> <insert rant about C macros here>
> 
> Cheers, Lorenzo

Re: [PATCH] mm: avoid use of BIT() macro for initialising VMA flags

[David Laight]

On Fri, 5 Dec 2025 19:18:56 +0000
Lorenzo Stoakes <lorenzo.stoakes@oracle.com> wrote:

> On Fri, Dec 05, 2025 at 06:43:42PM +0000, David Laight wrote:
> > On Fri,  5 Dec 2025 17:50:37 +0000
> > Lorenzo Stoakes <lorenzo.stoakes@oracle.com> wrote:
> >  
> > > Commit 2b6a3f061f11 ("mm: declare VMA flags by bit") significantly changed
> > > how VMA flags are declared, utilising an enum of VMA bit values and
> > > ifdef-fery VM_xxx flag declarations via macro.
> > >
> > > As part of this change, it uses INIT_VM_FLAG() to define VM_xxx flags from
> > > the newly introduced VMA bit numbers.
> > >
> > > However, use of this macro results in apparently unfortunate macro
> > > expansion and resulted in a performance degradation.This appears to be due
> > > to the (__force int), which is required for the sparse typechecking to
> > > work.  
> >
> > Does sparse complain if you just add 0? As in:
> > #define INIT_VM_FLAG(name) BIT(VMA_ ## name ## _BIT + 0u)
> >
> > That should change the type without affecting what BIT() expands to.  
> 
> Thanks, checked that and unfortunately that doesn't satisfy sparse :)

Oh - it is that __bitwise that causes grief.

> I don't think it's too crazy to use 1UL << here, just very frustrating (TM)
> that this is an issue.

Did you try getting DECLARE_VMA_BIT to define both the bit number and the
bit flag and put them both into the anonymous enum?
Or are there other reasons for not doing that?

> 
> <insert rant about C macros here>

Add rant about the compiler thinking anon enums are doing anything other
than defining constants.

	David

> 
> Cheers, Lorenzo

Re: [PATCH] mm: avoid use of BIT() macro for initialising VMA flags

[John Hubbard]

On 12/5/25 9:50 AM, Lorenzo Stoakes wrote:
> Commit 2b6a3f061f11 ("mm: declare VMA flags by bit") significantly changed
> how VMA flags are declared, utilising an enum of VMA bit values and
> ifdef-fery VM_xxx flag declarations via macro.
> 
> As part of this change, it uses INIT_VM_FLAG() to define VM_xxx flags from
> the newly introduced VMA bit numbers.
> 
> However, use of this macro results in apparently unfortunate macro
> expansion and resulted in a performance degradation.This appears to be due
> to the (__force int), which is required for the sparse typechecking to
> work.
> 
> Avoid macro expansion issues by simply using 1UL << bitnum.
> 
> Reported-by: kernel test robot <oliver.sang@intel.com>
> Closes: https://lore.kernel.org/oe-lkp/202512041634.150c7e4f-lkp@intel.com
> Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> ---
> 
> Andrew - note I've referenced the linux-next commit number above, could you
> replace with the upstream commit hash once your PR is taken? Thanks!
> 
>   include/linux/mm.h | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index a2f38fb68840..c4438b30c140 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -395,7 +395,8 @@ enum {
>   #undef DECLARE_VMA_BIT
>   #undef DECLARE_VMA_BIT_ALIAS
> 
> -#define INIT_VM_FLAG(name) BIT((__force int) VMA_ ## name ## _BIT)
> +#define INIT_VM_FLAG(name) (1UL << (__force int)(VMA_ ## name ## _BIT))

OK, so now maybe we don't need all of the rust/bindings/bindings_helper.h
changes? Those were because Rust's bindgen doesn't properly handle
nested macros, as I recall.

> +
>   #define VM_READ		INIT_VM_FLAG(READ)
>   #define VM_WRITE	INIT_VM_FLAG(WRITE)
>   #define VM_EXEC		INIT_VM_FLAG(EXEC)
> --
> 2.52.0
> 

thanks,
-- 
John Hubbard

Re: [PATCH] mm: avoid use of BIT() macro for initialising VMA flags

[Andrew Morton]

On Fri,  5 Dec 2025 17:50:37 +0000 Lorenzo Stoakes <lorenzo.stoakes@oracle.com> wrote:

> Commit 2b6a3f061f11 ("mm: declare VMA flags by bit") significantly changed
>
> ...
>
> Andrew - note I've referenced the linux-next commit number above, could you
> replace with the upstream commit hash once your PR is taken? Thanks!

That's in mm-stable so the hash shouldn't be changing.


I'm not really sure what's the best way to determine this.  I use

hp2:/usr/src/mm> git tag --contains 2b6a3f061f11
mm-everything-2025-11-29-19-43
mm-everything-2025-12-01-19-02
mm-everything-2025-12-03-23-49
mm-everything-2025-12-05-00-55
mm-stable-2025-12-03-21-26

Re: [PATCH] mm: avoid use of BIT() macro for initialising VMA flags

[David Hildenbrand (Red Hat)]

On 12/5/25 21:15, Andrew Morton wrote:
> On Fri,  5 Dec 2025 17:50:37 +0000 Lorenzo Stoakes <lorenzo.stoakes@oracle.com> wrote:
> 
>> Commit 2b6a3f061f11 ("mm: declare VMA flags by bit") significantly changed
>>
>> ...
>>
>> Andrew - note I've referenced the linux-next commit number above, could you
>> replace with the upstream commit hash once your PR is taken? Thanks!
> 
> That's in mm-stable so the hash shouldn't be changing.
> 
> 
> I'm not really sure what's the best way to determine this.  I use
> 
> hp2:/usr/src/mm> git tag --contains 2b6a3f061f11
> mm-everything-2025-11-29-19-43
> mm-everything-2025-12-01-19-02
> mm-everything-2025-12-03-23-49
> mm-everything-2025-12-05-00-55
> mm-stable-2025-12-03-21-26

I asked myself the same question a couple of times.

Maybe this?

  $ git branch -r --contains 2b6a3f061f11
   mm/mm-everything
   mm/mm-new
   mm/mm-stable


-- 
Cheers

David

Re: [PATCH] mm: avoid use of BIT() macro for initialising VMA flags

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 723 bytes --]

Hi Andrew,

On Fri, 5 Dec 2025 12:15:01 -0800 Andrew Morton <akpm@linux-foundation.org> wrote:
>
> I'm not really sure what's the best way to determine this.  I use
> 
> hp2:/usr/src/mm> git tag --contains 2b6a3f061f11  
> mm-everything-2025-11-29-19-43
> mm-everything-2025-12-01-19-02
> mm-everything-2025-12-03-23-49
> mm-everything-2025-12-05-00-55
> mm-stable-2025-12-03-21-26
> 

What does "git branch --contains 2b6a3f061f11" say in your tree?

In my linux-next tree it says (I need the -r to check remote branches):

$ git branch -r --contains 2b6a3f061f11
  mm-stable/mm-stable
  mm-unstable/mm-unstable

but I don't export my remotes to my published tree.

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] mm: avoid use of BIT() macro for initialising VMA flags

[Andrew Morton]

On Sat, 6 Dec 2025 11:40:34 +1100 Stephen Rothwell <sfr@canb.auug.org.au> wrote:

> Hi Andrew,
> 
> On Fri, 5 Dec 2025 12:15:01 -0800 Andrew Morton <akpm@linux-foundation.org> wrote:
> >
> > I'm not really sure what's the best way to determine this.  I use
> > 
> > hp2:/usr/src/mm> git tag --contains 2b6a3f061f11  
> > mm-everything-2025-11-29-19-43
> > mm-everything-2025-12-01-19-02
> > mm-everything-2025-12-03-23-49
> > mm-everything-2025-12-05-00-55
> > mm-stable-2025-12-03-21-26
> > 
> 
> What does "git branch --contains 2b6a3f061f11" say in your tree?

hp2:/usr/src/mm> git branch --contains 2b6a3f061f11
* linus
  mm-everything
  mm-new
  mm-stable
  mm-unstable
hp2:/usr/src/mm> git branch -r --contains 2b6a3f061f11 
  linus/master
  origin/mm-everything
  origin/mm-new
  origin/mm-stable
  origin/mm-unstable

kinda random, but it tells me "that's in mm-stable", which is what counts.

> In my linux-next tree it says (I need the -r to check remote branches):
> 
> $ git branch -r --contains 2b6a3f061f11
>   mm-stable/mm-stable
>   mm-unstable/mm-unstable
> 
> but I don't export my remotes to my published tree.

Re: [PATCH] mm: avoid use of BIT() macro for initialising VMA flags

[Al Viro]

On Fri, Dec 05, 2025 at 05:50:37PM +0000, Lorenzo Stoakes wrote:
> Commit 2b6a3f061f11 ("mm: declare VMA flags by bit") significantly changed
> how VMA flags are declared, utilising an enum of VMA bit values and
> ifdef-fery VM_xxx flag declarations via macro.
> 
> As part of this change, it uses INIT_VM_FLAG() to define VM_xxx flags from
> the newly introduced VMA bit numbers.
> 
> However, use of this macro results in apparently unfortunate macro
> expansion and resulted in a performance degradation.This appears to be due
> to the (__force int), which is required for the sparse typechecking to
> work.

> -#define INIT_VM_FLAG(name) BIT((__force int) VMA_ ## name ## _BIT)
> +#define INIT_VM_FLAG(name) (1UL << (__force int)(VMA_ ## name ## _BIT))

What the hell is __bitwise doing on these enum values?
Could we please get rid of that ridiculous cargo-culting?

Bitwise operations on BIT NUMBERS make no sense whatsoever; why are those
declared __bitwise?

Re: [PATCH] mm: avoid use of BIT() macro for initialising VMA flags

[Al Viro]

On Sat, Dec 06, 2025 at 01:14:35AM +0000, Al Viro wrote:
> On Fri, Dec 05, 2025 at 05:50:37PM +0000, Lorenzo Stoakes wrote:
> > Commit 2b6a3f061f11 ("mm: declare VMA flags by bit") significantly changed
> > how VMA flags are declared, utilising an enum of VMA bit values and
> > ifdef-fery VM_xxx flag declarations via macro.
> > 
> > As part of this change, it uses INIT_VM_FLAG() to define VM_xxx flags from
> > the newly introduced VMA bit numbers.
> > 
> > However, use of this macro results in apparently unfortunate macro
> > expansion and resulted in a performance degradation.This appears to be due
> > to the (__force int), which is required for the sparse typechecking to
> > work.
> 
> > -#define INIT_VM_FLAG(name) BIT((__force int) VMA_ ## name ## _BIT)
> > +#define INIT_VM_FLAG(name) (1UL << (__force int)(VMA_ ## name ## _BIT))
> 
> What the hell is __bitwise doing on these enum values?
> Could we please get rid of that ridiculous cargo-culting?
> 
> Bitwise operations on BIT NUMBERS make no sense whatsoever; why are those
> declared __bitwise?

FWIW, bitwise does make sense for things like (1 << SOME_CONSTANT);
then you get warned about arithmetics and conversions to integer
for those, with bitwise operations explicitly allowed.

VM_... are such; VMA_..._BIT are not.  VM_READ | VM_EXEC is fine;
VM_READ + 14 is nonsense and should be warned about.  That's where
__bitwise would make sense.  On bit numbers it's not - what makes
VMA_BIT_MAYREAD ^ VMA_BIT_SHARED any better than 3 * VMA_BIT_MAYREAD?

Re: [PATCH] mm: avoid use of BIT() macro for initialising VMA flags

[Vlastimil Babka]

On 12/6/25 2:26 AM, Al Viro wrote:
> On Sat, Dec 06, 2025 at 01:14:35AM +0000, Al Viro wrote:
>> On Fri, Dec 05, 2025 at 05:50:37PM +0000, Lorenzo Stoakes wrote:
>>> Commit 2b6a3f061f11 ("mm: declare VMA flags by bit") significantly changed
>>> how VMA flags are declared, utilising an enum of VMA bit values and
>>> ifdef-fery VM_xxx flag declarations via macro.
>>>
>>> As part of this change, it uses INIT_VM_FLAG() to define VM_xxx flags from
>>> the newly introduced VMA bit numbers.
>>>
>>> However, use of this macro results in apparently unfortunate macro
>>> expansion and resulted in a performance degradation.This appears to be due
>>> to the (__force int), which is required for the sparse typechecking to
>>> work.
>>
>>> -#define INIT_VM_FLAG(name) BIT((__force int) VMA_ ## name ## _BIT)
>>> +#define INIT_VM_FLAG(name) (1UL << (__force int)(VMA_ ## name ## _BIT))
>>
>> What the hell is __bitwise doing on these enum values?
>> Could we please get rid of that ridiculous cargo-culting?
>>
>> Bitwise operations on BIT NUMBERS make no sense whatsoever; why are those
>> declared __bitwise?

I was confused by this too at first when reviewing, but instead of the angry
display above, simply asked the author and got answers.

Comment says:

/**     
 * typedef vma_flag_t - specifies an individual VMA flag by bit number.
 *      
 * This value is made type safe by sparse to avoid passing invalid flag values
 * around.
 */     
typedef int __bitwise vma_flag_t;

It's done as documented in Documentation/dev-tools/sparse.rst section
"Using sparse for typechecking".

So yeah the keyword is __bitwise and indeed we don't perform bitwise operations
on the VM_ values, in fact we don't perform any operations without __force
casting them back first, to catch when they are used by mistake.
It's not cargo-culting, IIRC it catched a bug in an early version of the
patch itself.

I wouldn't mind if sparse provided a different keyword than __bitwise
for this use case to make it less misleading. Or even better if we could
make the compiler itself treat vma_flag_t as a "special int" that can't
be implicitly cast to a normal int, so we don't have to rely on sparse
checks to catch those.

 
> FWIW, bitwise does make sense for things like (1 << SOME_CONSTANT);
> then you get warned about arithmetics and conversions to integer
> for those, with bitwise operations explicitly allowed.
> 
> VM_... are such; VMA_..._BIT are not.  VM_READ | VM_EXEC is fine;
> VM_READ + 14 is nonsense and should be warned about.  That's where
> __bitwise would make sense.  On bit numbers it's not - what makes
> VMA_BIT_MAYREAD ^ VMA_BIT_SHARED any better than 3 * VMA_BIT_MAYREAD?