[PATCH 08/10] mm: update fork mm->flags initialisation to use bitmap

Lorenzo Stoakes posted 10 patches 1 month, 3 weeks ago
[PATCH 08/10] mm: update fork mm->flags initialisation to use bitmap
Posted by Lorenzo Stoakes 1 month, 3 weeks ago
We now need to account for flag initialisation on fork. We retain the
existing logic as much as we can, but dub the existing flag mask legacy.

These flags are therefore required to fit in the first 32-bits of the flags
field.

However, further flag propagation upon fork can be implemented in mm_init()
on a per-flag basis.

We ensure we clear the entire bitmap prior to setting it, and use
__mm_flags_get_word() and __mm_flags_set_word() to manipulate these legacy
fields efficiently.

Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
---
 include/linux/mm_types.h | 13 ++++++++++---
 kernel/fork.c            |  7 +++++--
 2 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
index 38b3fa927997..25577ab39094 100644
--- a/include/linux/mm_types.h
+++ b/include/linux/mm_types.h
@@ -1820,16 +1820,23 @@ enum {
 #define MMF_TOPDOWN		31	/* mm searches top down by default */
 #define MMF_TOPDOWN_MASK	_BITUL(MMF_TOPDOWN)
 
-#define MMF_INIT_MASK		(MMF_DUMPABLE_MASK | MMF_DUMP_FILTER_MASK |\
+#define MMF_INIT_LEGACY_MASK	(MMF_DUMPABLE_MASK | MMF_DUMP_FILTER_MASK |\
 				 MMF_DISABLE_THP_MASK | MMF_HAS_MDWE_MASK |\
 				 MMF_VM_MERGE_ANY_MASK | MMF_TOPDOWN_MASK)
 
-static inline unsigned long mmf_init_flags(unsigned long flags)
+/* Legacy flags must fit within 32 bits. */
+static_assert((u64)MMF_INIT_LEGACY_MASK <= (u64)UINT_MAX);
+
+/*
+ * Initialise legacy flags according to masks, propagating selected flags on
+ * fork. Further flag manipulation can be performed by the caller.
+ */
+static inline unsigned long mmf_init_legacy_flags(unsigned long flags)
 {
 	if (flags & (1UL << MMF_HAS_MDWE_NO_INHERIT))
 		flags &= ~((1UL << MMF_HAS_MDWE) |
 			   (1UL << MMF_HAS_MDWE_NO_INHERIT));
-	return flags & MMF_INIT_MASK;
+	return flags & MMF_INIT_LEGACY_MASK;
 }
 
 #endif /* _LINUX_MM_TYPES_H */
diff --git a/kernel/fork.c b/kernel/fork.c
index c4ada32598bd..b311caec6419 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -1056,11 +1056,14 @@ static struct mm_struct *mm_init(struct mm_struct *mm, struct task_struct *p,
 	mm_init_uprobes_state(mm);
 	hugetlb_count_init(mm);
 
+	mm_flags_clear_all(mm);
 	if (current->mm) {
-		mm->flags = mmf_init_flags(current->mm->flags);
+		unsigned long flags = __mm_flags_get_word(current->mm);
+
+		__mm_flags_set_word(mm, mmf_init_legacy_flags(flags));
 		mm->def_flags = current->mm->def_flags & VM_INIT_DEF_MASK;
 	} else {
-		mm->flags = default_dump_filter;
+		__mm_flags_set_word(mm, default_dump_filter);
 		mm->def_flags = 0;
 	}
 
-- 
2.50.1
Re: [PATCH 08/10] mm: update fork mm->flags initialisation to use bitmap
Posted by David Hildenbrand 1 month, 1 week ago
On 12.08.25 17:44, Lorenzo Stoakes wrote:
> We now need to account for flag initialisation on fork. We retain the
> existing logic as much as we can, but dub the existing flag mask legacy.
> 
> These flags are therefore required to fit in the first 32-bits of the flags
> field.
> 
> However, further flag propagation upon fork can be implemented in mm_init()
> on a per-flag basis.
> 
> We ensure we clear the entire bitmap prior to setting it, and use
> __mm_flags_get_word() and __mm_flags_set_word() to manipulate these legacy
> fields efficiently.
> 
> Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> ---
>   include/linux/mm_types.h | 13 ++++++++++---
>   kernel/fork.c            |  7 +++++--
>   2 files changed, 15 insertions(+), 5 deletions(-)
> 
> diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
> index 38b3fa927997..25577ab39094 100644
> --- a/include/linux/mm_types.h
> +++ b/include/linux/mm_types.h
> @@ -1820,16 +1820,23 @@ enum {
>   #define MMF_TOPDOWN		31	/* mm searches top down by default */
>   #define MMF_TOPDOWN_MASK	_BITUL(MMF_TOPDOWN)
>   
> -#define MMF_INIT_MASK		(MMF_DUMPABLE_MASK | MMF_DUMP_FILTER_MASK |\
> +#define MMF_INIT_LEGACY_MASK	(MMF_DUMPABLE_MASK | MMF_DUMP_FILTER_MASK |\
>   				 MMF_DISABLE_THP_MASK | MMF_HAS_MDWE_MASK |\
>   				 MMF_VM_MERGE_ANY_MASK | MMF_TOPDOWN_MASK)
>   
> -static inline unsigned long mmf_init_flags(unsigned long flags)
> +/* Legacy flags must fit within 32 bits. */
> +static_assert((u64)MMF_INIT_LEGACY_MASK <= (u64)UINT_MAX);

Why not use the magic number 32 you are mentioning in the comment? :)

static_assert((u32)MMF_INIT_LEGACY_MASK != MMF_INIT_LEGACY_MASK);

> +
> +/*
> + * Initialise legacy flags according to masks, propagating selected flags on
> + * fork. Further flag manipulation can be performed by the caller.

It's weird not reading "initialize", but I am afraid the kernel is 
already tainted :P

t14s: ~/git/linux nth_page $ git grep "initialise" | wc -l
1778
t14s: ~/git/linux nth_page $ git grep "initialize" | wc -l
22043

Besides the assert simplification

Acked-by: David Hildenbrand <david@redhat.com>

-- 
Cheers

David / dhildenb
Re: [PATCH 08/10] mm: update fork mm->flags initialisation to use bitmap
Posted by Lorenzo Stoakes 1 month, 1 week ago
On Tue, Aug 26, 2025 at 03:12:08PM +0200, David Hildenbrand wrote:
> On 12.08.25 17:44, Lorenzo Stoakes wrote:
> > We now need to account for flag initialisation on fork. We retain the
> > existing logic as much as we can, but dub the existing flag mask legacy.
> >
> > These flags are therefore required to fit in the first 32-bits of the flags
> > field.
> >
> > However, further flag propagation upon fork can be implemented in mm_init()
> > on a per-flag basis.
> >
> > We ensure we clear the entire bitmap prior to setting it, and use
> > __mm_flags_get_word() and __mm_flags_set_word() to manipulate these legacy
> > fields efficiently.
> >
> > Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> > ---
> >   include/linux/mm_types.h | 13 ++++++++++---
> >   kernel/fork.c            |  7 +++++--
> >   2 files changed, 15 insertions(+), 5 deletions(-)
> >
> > diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
> > index 38b3fa927997..25577ab39094 100644
> > --- a/include/linux/mm_types.h
> > +++ b/include/linux/mm_types.h
> > @@ -1820,16 +1820,23 @@ enum {
> >   #define MMF_TOPDOWN		31	/* mm searches top down by default */
> >   #define MMF_TOPDOWN_MASK	_BITUL(MMF_TOPDOWN)
> > -#define MMF_INIT_MASK		(MMF_DUMPABLE_MASK | MMF_DUMP_FILTER_MASK |\
> > +#define MMF_INIT_LEGACY_MASK	(MMF_DUMPABLE_MASK | MMF_DUMP_FILTER_MASK |\
> >   				 MMF_DISABLE_THP_MASK | MMF_HAS_MDWE_MASK |\
> >   				 MMF_VM_MERGE_ANY_MASK | MMF_TOPDOWN_MASK)
> > -static inline unsigned long mmf_init_flags(unsigned long flags)
> > +/* Legacy flags must fit within 32 bits. */
> > +static_assert((u64)MMF_INIT_LEGACY_MASK <= (u64)UINT_MAX);
>
> Why not use the magic number 32 you are mentioning in the comment? :)

Meh I mean UINT_MAX works as a good 'any bit' mask and this will work on
both 32-bit and 64-bit systems.

>
> static_assert((u32)MMF_INIT_LEGACY_MASK != MMF_INIT_LEGACY_MASK);

On 32-bit that'd not work would it?

Kinda the point here is that the issue would get picked up on either.

I think with the comment above it's fine as-is! :)

>
> > +
> > +/*
> > + * Initialise legacy flags according to masks, propagating selected flags on
> > + * fork. Further flag manipulation can be performed by the caller.
>
> It's weird not reading "initialize", but I am afraid the kernel is already
> tainted :P
>
> t14s: ~/git/linux nth_page $ git grep "initialise" | wc -l
> 1778
> t14s: ~/git/linux nth_page $ git grep "initialize" | wc -l
> 22043

British English in the kernel will survive if I have anything to do with it
;)

>
> Besides the assert simplification
>
> Acked-by: David Hildenbrand <david@redhat.com>

Thanks!

>
> --
> Cheers
>
> David / dhildenb
>
Re: [PATCH 08/10] mm: update fork mm->flags initialisation to use bitmap
Posted by David Hildenbrand 1 month, 1 week ago
On 26.08.25 16:21, Lorenzo Stoakes wrote:
> On Tue, Aug 26, 2025 at 03:12:08PM +0200, David Hildenbrand wrote:
>> On 12.08.25 17:44, Lorenzo Stoakes wrote:
>>> We now need to account for flag initialisation on fork. We retain the
>>> existing logic as much as we can, but dub the existing flag mask legacy.
>>>
>>> These flags are therefore required to fit in the first 32-bits of the flags
>>> field.
>>>
>>> However, further flag propagation upon fork can be implemented in mm_init()
>>> on a per-flag basis.
>>>
>>> We ensure we clear the entire bitmap prior to setting it, and use
>>> __mm_flags_get_word() and __mm_flags_set_word() to manipulate these legacy
>>> fields efficiently.
>>>
>>> Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
>>> ---
>>>    include/linux/mm_types.h | 13 ++++++++++---
>>>    kernel/fork.c            |  7 +++++--
>>>    2 files changed, 15 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
>>> index 38b3fa927997..25577ab39094 100644
>>> --- a/include/linux/mm_types.h
>>> +++ b/include/linux/mm_types.h
>>> @@ -1820,16 +1820,23 @@ enum {
>>>    #define MMF_TOPDOWN		31	/* mm searches top down by default */
>>>    #define MMF_TOPDOWN_MASK	_BITUL(MMF_TOPDOWN)
>>> -#define MMF_INIT_MASK		(MMF_DUMPABLE_MASK | MMF_DUMP_FILTER_MASK |\
>>> +#define MMF_INIT_LEGACY_MASK	(MMF_DUMPABLE_MASK | MMF_DUMP_FILTER_MASK |\
>>>    				 MMF_DISABLE_THP_MASK | MMF_HAS_MDWE_MASK |\
>>>    				 MMF_VM_MERGE_ANY_MASK | MMF_TOPDOWN_MASK)
>>> -static inline unsigned long mmf_init_flags(unsigned long flags)
>>> +/* Legacy flags must fit within 32 bits. */
>>> +static_assert((u64)MMF_INIT_LEGACY_MASK <= (u64)UINT_MAX);
>>
>> Why not use the magic number 32 you are mentioning in the comment? :)
> 
> Meh I mean UINT_MAX works as a good 'any bit' mask and this will work on
> both 32-bit and 64-bit systems.
> 
>>
>> static_assert((u32)MMF_INIT_LEGACY_MASK != MMF_INIT_LEGACY_MASK);
> 
> On 32-bit that'd not work would it?

On 32bit, BIT(32) would exceed the shift width of unsigned long -> 
undefined behavior.

The compiler should naturally complain.

-- 
Cheers

David / dhildenb
Re: [PATCH 08/10] mm: update fork mm->flags initialisation to use bitmap
Posted by Lorenzo Stoakes 1 month, 1 week ago
On Tue, Aug 26, 2025 at 04:28:20PM +0200, David Hildenbrand wrote:
> On 26.08.25 16:21, Lorenzo Stoakes wrote:
> > On Tue, Aug 26, 2025 at 03:12:08PM +0200, David Hildenbrand wrote:
> > > On 12.08.25 17:44, Lorenzo Stoakes wrote:
> > > > We now need to account for flag initialisation on fork. We retain the
> > > > existing logic as much as we can, but dub the existing flag mask legacy.
> > > >
> > > > These flags are therefore required to fit in the first 32-bits of the flags
> > > > field.
> > > >
> > > > However, further flag propagation upon fork can be implemented in mm_init()
> > > > on a per-flag basis.
> > > >
> > > > We ensure we clear the entire bitmap prior to setting it, and use
> > > > __mm_flags_get_word() and __mm_flags_set_word() to manipulate these legacy
> > > > fields efficiently.
> > > >
> > > > Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> > > > ---
> > > >    include/linux/mm_types.h | 13 ++++++++++---
> > > >    kernel/fork.c            |  7 +++++--
> > > >    2 files changed, 15 insertions(+), 5 deletions(-)
> > > >
> > > > diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
> > > > index 38b3fa927997..25577ab39094 100644
> > > > --- a/include/linux/mm_types.h
> > > > +++ b/include/linux/mm_types.h
> > > > @@ -1820,16 +1820,23 @@ enum {
> > > >    #define MMF_TOPDOWN		31	/* mm searches top down by default */
> > > >    #define MMF_TOPDOWN_MASK	_BITUL(MMF_TOPDOWN)
> > > > -#define MMF_INIT_MASK		(MMF_DUMPABLE_MASK | MMF_DUMP_FILTER_MASK |\
> > > > +#define MMF_INIT_LEGACY_MASK	(MMF_DUMPABLE_MASK | MMF_DUMP_FILTER_MASK |\
> > > >    				 MMF_DISABLE_THP_MASK | MMF_HAS_MDWE_MASK |\
> > > >    				 MMF_VM_MERGE_ANY_MASK | MMF_TOPDOWN_MASK)
> > > > -static inline unsigned long mmf_init_flags(unsigned long flags)
> > > > +/* Legacy flags must fit within 32 bits. */
> > > > +static_assert((u64)MMF_INIT_LEGACY_MASK <= (u64)UINT_MAX);
> > >
> > > Why not use the magic number 32 you are mentioning in the comment? :)
> >
> > Meh I mean UINT_MAX works as a good 'any bit' mask and this will work on
> > both 32-bit and 64-bit systems.
> >
> > >
> > > static_assert((u32)MMF_INIT_LEGACY_MASK != MMF_INIT_LEGACY_MASK);
> >
> > On 32-bit that'd not work would it?
>
> On 32bit, BIT(32) would exceed the shift width of unsigned long -> undefined
> behavior.
>
> The compiler should naturally complain.

Yeah, I don't love that sorry. Firstly it's a warning, so you may well miss it
(I just tried), and secondly you're making the static assert not have any
meaning except that you expect to trigger a compiler warning, it's a bit
bizarre.

My solution works (unless you can see a reason it shouldn't) and I don't find
this approach any simpler.

>
> --
> Cheers
>
> David / dhildenb
>

Cheers, Lorenzo
Re: [PATCH 08/10] mm: update fork mm->flags initialisation to use bitmap
Posted by David Hildenbrand 1 month, 1 week ago
On 26.08.25 16:32, Lorenzo Stoakes wrote:
> On Tue, Aug 26, 2025 at 04:28:20PM +0200, David Hildenbrand wrote:
>> On 26.08.25 16:21, Lorenzo Stoakes wrote:
>>> On Tue, Aug 26, 2025 at 03:12:08PM +0200, David Hildenbrand wrote:
>>>> On 12.08.25 17:44, Lorenzo Stoakes wrote:
>>>>> We now need to account for flag initialisation on fork. We retain the
>>>>> existing logic as much as we can, but dub the existing flag mask legacy.
>>>>>
>>>>> These flags are therefore required to fit in the first 32-bits of the flags
>>>>> field.
>>>>>
>>>>> However, further flag propagation upon fork can be implemented in mm_init()
>>>>> on a per-flag basis.
>>>>>
>>>>> We ensure we clear the entire bitmap prior to setting it, and use
>>>>> __mm_flags_get_word() and __mm_flags_set_word() to manipulate these legacy
>>>>> fields efficiently.
>>>>>
>>>>> Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
>>>>> ---
>>>>>     include/linux/mm_types.h | 13 ++++++++++---
>>>>>     kernel/fork.c            |  7 +++++--
>>>>>     2 files changed, 15 insertions(+), 5 deletions(-)
>>>>>
>>>>> diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
>>>>> index 38b3fa927997..25577ab39094 100644
>>>>> --- a/include/linux/mm_types.h
>>>>> +++ b/include/linux/mm_types.h
>>>>> @@ -1820,16 +1820,23 @@ enum {
>>>>>     #define MMF_TOPDOWN		31	/* mm searches top down by default */
>>>>>     #define MMF_TOPDOWN_MASK	_BITUL(MMF_TOPDOWN)
>>>>> -#define MMF_INIT_MASK		(MMF_DUMPABLE_MASK | MMF_DUMP_FILTER_MASK |\
>>>>> +#define MMF_INIT_LEGACY_MASK	(MMF_DUMPABLE_MASK | MMF_DUMP_FILTER_MASK |\
>>>>>     				 MMF_DISABLE_THP_MASK | MMF_HAS_MDWE_MASK |\
>>>>>     				 MMF_VM_MERGE_ANY_MASK | MMF_TOPDOWN_MASK)
>>>>> -static inline unsigned long mmf_init_flags(unsigned long flags)
>>>>> +/* Legacy flags must fit within 32 bits. */
>>>>> +static_assert((u64)MMF_INIT_LEGACY_MASK <= (u64)UINT_MAX);
>>>>
>>>> Why not use the magic number 32 you are mentioning in the comment? :)
>>>
>>> Meh I mean UINT_MAX works as a good 'any bit' mask and this will work on
>>> both 32-bit and 64-bit systems.
>>>
>>>>
>>>> static_assert((u32)MMF_INIT_LEGACY_MASK != MMF_INIT_LEGACY_MASK);
>>>
>>> On 32-bit that'd not work would it?
>>
>> On 32bit, BIT(32) would exceed the shift width of unsigned long -> undefined
>> behavior.
>>
>> The compiler should naturally complain.
> 
> Yeah, I don't love that sorry. Firstly it's a warning, so you may well miss it
> (I just tried), 

Upstream bots usually complain at you for warnings :P

> and secondly you're making the static assert not have any
> meaning except that you expect to trigger a compiler warning, it's a bit
> bizarre.

On 64 bit where BIT(32) *makes any sense* it triggers as expected, no?

> 
> My solution works (unless you can see a reason it shouldn't) and I don't find
> this approach any simpler.

Please explain to me like I am a 5 yo how your approach works with 
BIT(32) on 32bit when the behavior on 32bit is undefined. :P

-- 
Cheers

David / dhildenb
Re: [PATCH 08/10] mm: update fork mm->flags initialisation to use bitmap
Posted by Lorenzo Stoakes 1 month, 1 week ago
On Tue, Aug 26, 2025 at 05:24:22PM +0200, David Hildenbrand wrote:
> On 26.08.25 16:32, Lorenzo Stoakes wrote:
> > On Tue, Aug 26, 2025 at 04:28:20PM +0200, David Hildenbrand wrote:
> > > On 26.08.25 16:21, Lorenzo Stoakes wrote:
> > > > On Tue, Aug 26, 2025 at 03:12:08PM +0200, David Hildenbrand wrote:
> > > > > On 12.08.25 17:44, Lorenzo Stoakes wrote:
> > > > > > We now need to account for flag initialisation on fork. We retain the
> > > > > > existing logic as much as we can, but dub the existing flag mask legacy.
> > > > > >
> > > > > > These flags are therefore required to fit in the first 32-bits of the flags
> > > > > > field.
> > > > > >
> > > > > > However, further flag propagation upon fork can be implemented in mm_init()
> > > > > > on a per-flag basis.
> > > > > >
> > > > > > We ensure we clear the entire bitmap prior to setting it, and use
> > > > > > __mm_flags_get_word() and __mm_flags_set_word() to manipulate these legacy
> > > > > > fields efficiently.
> > > > > >
> > > > > > Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> > > > > > ---
> > > > > >     include/linux/mm_types.h | 13 ++++++++++---
> > > > > >     kernel/fork.c            |  7 +++++--
> > > > > >     2 files changed, 15 insertions(+), 5 deletions(-)
> > > > > >
> > > > > > diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
> > > > > > index 38b3fa927997..25577ab39094 100644
> > > > > > --- a/include/linux/mm_types.h
> > > > > > +++ b/include/linux/mm_types.h
> > > > > > @@ -1820,16 +1820,23 @@ enum {
> > > > > >     #define MMF_TOPDOWN		31	/* mm searches top down by default */
> > > > > >     #define MMF_TOPDOWN_MASK	_BITUL(MMF_TOPDOWN)
> > > > > > -#define MMF_INIT_MASK		(MMF_DUMPABLE_MASK | MMF_DUMP_FILTER_MASK |\
> > > > > > +#define MMF_INIT_LEGACY_MASK	(MMF_DUMPABLE_MASK | MMF_DUMP_FILTER_MASK |\
> > > > > >     				 MMF_DISABLE_THP_MASK | MMF_HAS_MDWE_MASK |\
> > > > > >     				 MMF_VM_MERGE_ANY_MASK | MMF_TOPDOWN_MASK)
> > > > > > -static inline unsigned long mmf_init_flags(unsigned long flags)
> > > > > > +/* Legacy flags must fit within 32 bits. */
> > > > > > +static_assert((u64)MMF_INIT_LEGACY_MASK <= (u64)UINT_MAX);
> > > > >
> > > > > Why not use the magic number 32 you are mentioning in the comment? :)
> > > >
> > > > Meh I mean UINT_MAX works as a good 'any bit' mask and this will work on
> > > > both 32-bit and 64-bit systems.
> > > >
> > > > >
> > > > > static_assert((u32)MMF_INIT_LEGACY_MASK != MMF_INIT_LEGACY_MASK);
> > > >
> > > > On 32-bit that'd not work would it?
> > >
> > > On 32bit, BIT(32) would exceed the shift width of unsigned long -> undefined
> > > behavior.
> > >
> > > The compiler should naturally complain.
> >
> > Yeah, I don't love that sorry. Firstly it's a warning, so you may well miss it
> > (I just tried),
>
> Upstream bots usually complain at you for warnings :P

Fine, but it's not a static assert and they can be delayed.

>
> > and secondly you're making the static assert not have any
> > meaning except that you expect to trigger a compiler warning, it's a bit
> > bizarre.
>
> On 64 bit where BIT(32) *makes any sense* it triggers as expected, no?

It's not a static assert.

>
> >
> > My solution works (unless you can see a reason it shouldn't) and I don't find
> > this approach any simpler.
>
> Please explain to me like I am a 5 yo how your approach works with BIT(32)
> on 32bit when the behavior on 32bit is undefined. :P

OK right I see, in both cases BIT(32) is going to cause a warning on 32-bit.

I was wrong in thinking (u64)(1UL << 32) would get fixed up because of the
outer cast I guess.

This was the mistake here, so fine, we could do it this way.

I guess I'll have to respin the series at this point.
Re: [PATCH 08/10] mm: update fork mm->flags initialisation to use bitmap
Posted by David Hildenbrand 1 month, 1 week ago
On 26.08.25 17:39, Lorenzo Stoakes wrote:
> On Tue, Aug 26, 2025 at 05:24:22PM +0200, David Hildenbrand wrote:
>> On 26.08.25 16:32, Lorenzo Stoakes wrote:
>>> On Tue, Aug 26, 2025 at 04:28:20PM +0200, David Hildenbrand wrote:
>>>> On 26.08.25 16:21, Lorenzo Stoakes wrote:
>>>>> On Tue, Aug 26, 2025 at 03:12:08PM +0200, David Hildenbrand wrote:
>>>>>> On 12.08.25 17:44, Lorenzo Stoakes wrote:
>>>>>>> We now need to account for flag initialisation on fork. We retain the
>>>>>>> existing logic as much as we can, but dub the existing flag mask legacy.
>>>>>>>
>>>>>>> These flags are therefore required to fit in the first 32-bits of the flags
>>>>>>> field.
>>>>>>>
>>>>>>> However, further flag propagation upon fork can be implemented in mm_init()
>>>>>>> on a per-flag basis.
>>>>>>>
>>>>>>> We ensure we clear the entire bitmap prior to setting it, and use
>>>>>>> __mm_flags_get_word() and __mm_flags_set_word() to manipulate these legacy
>>>>>>> fields efficiently.
>>>>>>>
>>>>>>> Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
>>>>>>> ---
>>>>>>>      include/linux/mm_types.h | 13 ++++++++++---
>>>>>>>      kernel/fork.c            |  7 +++++--
>>>>>>>      2 files changed, 15 insertions(+), 5 deletions(-)
>>>>>>>
>>>>>>> diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
>>>>>>> index 38b3fa927997..25577ab39094 100644
>>>>>>> --- a/include/linux/mm_types.h
>>>>>>> +++ b/include/linux/mm_types.h
>>>>>>> @@ -1820,16 +1820,23 @@ enum {
>>>>>>>      #define MMF_TOPDOWN		31	/* mm searches top down by default */
>>>>>>>      #define MMF_TOPDOWN_MASK	_BITUL(MMF_TOPDOWN)
>>>>>>> -#define MMF_INIT_MASK		(MMF_DUMPABLE_MASK | MMF_DUMP_FILTER_MASK |\
>>>>>>> +#define MMF_INIT_LEGACY_MASK	(MMF_DUMPABLE_MASK | MMF_DUMP_FILTER_MASK |\
>>>>>>>      				 MMF_DISABLE_THP_MASK | MMF_HAS_MDWE_MASK |\
>>>>>>>      				 MMF_VM_MERGE_ANY_MASK | MMF_TOPDOWN_MASK)
>>>>>>> -static inline unsigned long mmf_init_flags(unsigned long flags)
>>>>>>> +/* Legacy flags must fit within 32 bits. */
>>>>>>> +static_assert((u64)MMF_INIT_LEGACY_MASK <= (u64)UINT_MAX);
>>>>>>
>>>>>> Why not use the magic number 32 you are mentioning in the comment? :)
>>>>>
>>>>> Meh I mean UINT_MAX works as a good 'any bit' mask and this will work on
>>>>> both 32-bit and 64-bit systems.
>>>>>
>>>>>>
>>>>>> static_assert((u32)MMF_INIT_LEGACY_MASK != MMF_INIT_LEGACY_MASK);
>>>>>
>>>>> On 32-bit that'd not work would it?
>>>>
>>>> On 32bit, BIT(32) would exceed the shift width of unsigned long -> undefined
>>>> behavior.
>>>>
>>>> The compiler should naturally complain.
>>>
>>> Yeah, I don't love that sorry. Firstly it's a warning, so you may well miss it
>>> (I just tried),
>>
>> Upstream bots usually complain at you for warnings :P
> 
> Fine, but it's not a static assert and they can be delayed.
> 
>>
>>> and secondly you're making the static assert not have any
>>> meaning except that you expect to trigger a compiler warning, it's a bit
>>> bizarre.
>>
>> On 64 bit where BIT(32) *makes any sense* it triggers as expected, no?
> 
> It's not a static assert.
> 
>>
>>>
>>> My solution works (unless you can see a reason it shouldn't) and I don't find
>>> this approach any simpler.
>>
>> Please explain to me like I am a 5 yo how your approach works with BIT(32)
>> on 32bit when the behavior on 32bit is undefined. :P
> 
> OK right I see, in both cases BIT(32) is going to cause a warning on 32-bit.
> 
> I was wrong in thinking (u64)(1UL << 32) would get fixed up because of the
> outer cast I guess.
> 
> This was the mistake here, so fine, we could do it this way.
> 
> I guess I'll have to respin the series at this point.

Let me think again: assuming someone would mess up the BIT() thing and 
convert back to 1 << NR, your variant would catch it on 32bit I guess.

So given I was primarily confused by the "u64" when talking about 32it, 
no need for a full resend of this series just to clean this up.

-- 
Cheers

David / dhildenb
Re: [PATCH 08/10] mm: update fork mm->flags initialisation to use bitmap
Posted by Lorenzo Stoakes 1 month, 1 week ago
On Tue, Aug 26, 2025 at 05:53:36PM +0200, David Hildenbrand wrote:
> > OK right I see, in both cases BIT(32) is going to cause a warning on 32-bit.
> >
> > I was wrong in thinking (u64)(1UL << 32) would get fixed up because of the
> > outer cast I guess.
> >
> > This was the mistake here, so fine, we could do it this way.
> >
> > I guess I'll have to respin the series at this point.
>
> Let me think again: assuming someone would mess up the BIT() thing and
> convert back to 1 << NR, your variant would catch it on 32bit I guess.

Yeah and it should also catch the BIT() case albeit via warning :P

>
> So given I was primarily confused by the "u64" when talking about 32it, no
> need for a full resend of this series just to clean this up.

Yeah this isn't _terrible_ I don't think, although I was labouring under a
misapprehension (understandable - hopefully :) in the way the casting would
work, but of course you're right the ub would happen _prior_ to the u64 cast.

Anyway I think net we're getting the same result, and the comment makes clear
the intent, so I think yeah sort of fine as-is.

But I take your point absolutely :)

Cheers, Lorenzo
Re: [PATCH 08/10] mm: update fork mm->flags initialisation to use bitmap
Posted by Mike Rapoport 1 month, 3 weeks ago
On Tue, Aug 12, 2025 at 04:44:17PM +0100, Lorenzo Stoakes wrote:
> We now need to account for flag initialisation on fork. We retain the
> existing logic as much as we can, but dub the existing flag mask legacy.
> 
> These flags are therefore required to fit in the first 32-bits of the flags
> field.
> 
> However, further flag propagation upon fork can be implemented in mm_init()
> on a per-flag basis.
> 
> We ensure we clear the entire bitmap prior to setting it, and use
> __mm_flags_get_word() and __mm_flags_set_word() to manipulate these legacy
> fields efficiently.
> 
> Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>

Reviewed-by: Mike Rapoport (Microsoft) <rppt@kernel.org>

> ---
>  include/linux/mm_types.h | 13 ++++++++++---
>  kernel/fork.c            |  7 +++++--
>  2 files changed, 15 insertions(+), 5 deletions(-)
> 
> diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
> index 38b3fa927997..25577ab39094 100644
> --- a/include/linux/mm_types.h
> +++ b/include/linux/mm_types.h
> @@ -1820,16 +1820,23 @@ enum {
>  #define MMF_TOPDOWN		31	/* mm searches top down by default */
>  #define MMF_TOPDOWN_MASK	_BITUL(MMF_TOPDOWN)
>  
> -#define MMF_INIT_MASK		(MMF_DUMPABLE_MASK | MMF_DUMP_FILTER_MASK |\
> +#define MMF_INIT_LEGACY_MASK	(MMF_DUMPABLE_MASK | MMF_DUMP_FILTER_MASK |\
>  				 MMF_DISABLE_THP_MASK | MMF_HAS_MDWE_MASK |\
>  				 MMF_VM_MERGE_ANY_MASK | MMF_TOPDOWN_MASK)
>  
> -static inline unsigned long mmf_init_flags(unsigned long flags)
> +/* Legacy flags must fit within 32 bits. */
> +static_assert((u64)MMF_INIT_LEGACY_MASK <= (u64)UINT_MAX);
> +
> +/*
> + * Initialise legacy flags according to masks, propagating selected flags on
> + * fork. Further flag manipulation can be performed by the caller.
> + */
> +static inline unsigned long mmf_init_legacy_flags(unsigned long flags)
>  {
>  	if (flags & (1UL << MMF_HAS_MDWE_NO_INHERIT))
>  		flags &= ~((1UL << MMF_HAS_MDWE) |
>  			   (1UL << MMF_HAS_MDWE_NO_INHERIT));
> -	return flags & MMF_INIT_MASK;
> +	return flags & MMF_INIT_LEGACY_MASK;
>  }
>  
>  #endif /* _LINUX_MM_TYPES_H */
> diff --git a/kernel/fork.c b/kernel/fork.c
> index c4ada32598bd..b311caec6419 100644
> --- a/kernel/fork.c
> +++ b/kernel/fork.c
> @@ -1056,11 +1056,14 @@ static struct mm_struct *mm_init(struct mm_struct *mm, struct task_struct *p,
>  	mm_init_uprobes_state(mm);
>  	hugetlb_count_init(mm);
>  
> +	mm_flags_clear_all(mm);
>  	if (current->mm) {
> -		mm->flags = mmf_init_flags(current->mm->flags);
> +		unsigned long flags = __mm_flags_get_word(current->mm);
> +
> +		__mm_flags_set_word(mm, mmf_init_legacy_flags(flags));
>  		mm->def_flags = current->mm->def_flags & VM_INIT_DEF_MASK;
>  	} else {
> -		mm->flags = default_dump_filter;
> +		__mm_flags_set_word(mm, default_dump_filter);
>  		mm->def_flags = 0;
>  	}
>  
> -- 
> 2.50.1
> 

-- 
Sincerely yours,
Mike.
Re: [PATCH 08/10] mm: update fork mm->flags initialisation to use bitmap
Posted by Liam R. Howlett 1 month, 3 weeks ago
* Lorenzo Stoakes <lorenzo.stoakes@oracle.com> [250812 11:47]:
> We now need to account for flag initialisation on fork. We retain the
> existing logic as much as we can, but dub the existing flag mask legacy.
> 
> These flags are therefore required to fit in the first 32-bits of the flags
> field.
> 
> However, further flag propagation upon fork can be implemented in mm_init()
> on a per-flag basis.
> 
> We ensure we clear the entire bitmap prior to setting it, and use
> __mm_flags_get_word() and __mm_flags_set_word() to manipulate these legacy
> fields efficiently.
> 
> Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>

Reviewed-by: Liam R. Howlett <Liam.Howlett@oracle.com>

> ---
>  include/linux/mm_types.h | 13 ++++++++++---
>  kernel/fork.c            |  7 +++++--
>  2 files changed, 15 insertions(+), 5 deletions(-)
> 
> diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
> index 38b3fa927997..25577ab39094 100644
> --- a/include/linux/mm_types.h
> +++ b/include/linux/mm_types.h
> @@ -1820,16 +1820,23 @@ enum {
>  #define MMF_TOPDOWN		31	/* mm searches top down by default */
>  #define MMF_TOPDOWN_MASK	_BITUL(MMF_TOPDOWN)
>  
> -#define MMF_INIT_MASK		(MMF_DUMPABLE_MASK | MMF_DUMP_FILTER_MASK |\
> +#define MMF_INIT_LEGACY_MASK	(MMF_DUMPABLE_MASK | MMF_DUMP_FILTER_MASK |\
>  				 MMF_DISABLE_THP_MASK | MMF_HAS_MDWE_MASK |\
>  				 MMF_VM_MERGE_ANY_MASK | MMF_TOPDOWN_MASK)
>  
> -static inline unsigned long mmf_init_flags(unsigned long flags)
> +/* Legacy flags must fit within 32 bits. */
> +static_assert((u64)MMF_INIT_LEGACY_MASK <= (u64)UINT_MAX);
> +
> +/*
> + * Initialise legacy flags according to masks, propagating selected flags on
> + * fork. Further flag manipulation can be performed by the caller.
> + */
> +static inline unsigned long mmf_init_legacy_flags(unsigned long flags)
>  {
>  	if (flags & (1UL << MMF_HAS_MDWE_NO_INHERIT))
>  		flags &= ~((1UL << MMF_HAS_MDWE) |
>  			   (1UL << MMF_HAS_MDWE_NO_INHERIT));
> -	return flags & MMF_INIT_MASK;
> +	return flags & MMF_INIT_LEGACY_MASK;
>  }
>  
>  #endif /* _LINUX_MM_TYPES_H */
> diff --git a/kernel/fork.c b/kernel/fork.c
> index c4ada32598bd..b311caec6419 100644
> --- a/kernel/fork.c
> +++ b/kernel/fork.c
> @@ -1056,11 +1056,14 @@ static struct mm_struct *mm_init(struct mm_struct *mm, struct task_struct *p,
>  	mm_init_uprobes_state(mm);
>  	hugetlb_count_init(mm);
>  
> +	mm_flags_clear_all(mm);
>  	if (current->mm) {
> -		mm->flags = mmf_init_flags(current->mm->flags);
> +		unsigned long flags = __mm_flags_get_word(current->mm);
> +
> +		__mm_flags_set_word(mm, mmf_init_legacy_flags(flags));
>  		mm->def_flags = current->mm->def_flags & VM_INIT_DEF_MASK;
>  	} else {
> -		mm->flags = default_dump_filter;
> +		__mm_flags_set_word(mm, default_dump_filter);
>  		mm->def_flags = 0;
>  	}
>  
> -- 
> 2.50.1
>