[PATCH] MAINTAINERS: group all VMA-related files into the VMA section

[PATCH] MAINTAINERS: group all VMA-related files into the VMA section

[Lorenzo Stoakes]

There are a number of means of interacting with VMA operations within mm,
and we have on occasion not been made aware of impactful changes due to
these sitting in different files, most recently in [0].

Correct this by bringing all VMA operations under the same section in
MAINTAINERS. Additionally take the opportunity to combine MEMORY MAPPING
with VMA as there needn't be two entries as they amount to the same thing.

[0]:https://lore.kernel.org/linux-mm/CAG48ez0siYGB8GP5+Szgj2ovBZAkL6Zi4n6GUAjzzjFV9LTkRQ@mail.gmail.com/

Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
---
 MAINTAINERS | 19 +++++++------------
 1 file changed, 7 insertions(+), 12 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 1e930c7a58b1..95db20c26f5f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -15060,18 +15060,6 @@ F:	tools/mm/
 F:	tools/testing/selftests/mm/
 N:	include/linux/page[-_]*

-MEMORY MAPPING
-M:	Andrew Morton <akpm@linux-foundation.org>
-M:	Liam R. Howlett <Liam.Howlett@oracle.com>
-M:	Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
-R:	Vlastimil Babka <vbabka@suse.cz>
-R:	Jann Horn <jannh@google.com>
-L:	linux-mm@kvack.org
-S:	Maintained
-W:	http://www.linux-mm.org
-T:	git git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
-F:	mm/mmap.c
-
 MEMORY TECHNOLOGY DEVICES (MTD)
 M:	Miquel Raynal <miquel.raynal@bootlin.com>
 M:	Richard Weinberger <richard@nod.at>
@@ -25028,6 +25016,13 @@ L:	linux-mm@kvack.org
 S:	Maintained
 W:	https://www.linux-mm.org
 T:	git git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
+F:	mm/madvise.c
+F:	mm/mlock.c
+F:	mm/mmap.c
+F:	mm/mprotect.c
+F:	mm/mremap.c
+F:	mm/mseal.c
+F:	mm/msync.c
 F:	mm/vma.c
 F:	mm/vma.h
 F:	mm/vma_internal.h
--
2.47.1

Re: [PATCH] MAINTAINERS: group all VMA-related files into the VMA section

[Liam R. Howlett]

* Lorenzo Stoakes <lorenzo.stoakes@oracle.com> [241206 14:16]:
> There are a number of means of interacting with VMA operations within mm,
> and we have on occasion not been made aware of impactful changes due to
> these sitting in different files, most recently in [0].
> 
> Correct this by bringing all VMA operations under the same section in
> MAINTAINERS. Additionally take the opportunity to combine MEMORY MAPPING
> with VMA as there needn't be two entries as they amount to the same thing.
> 
> [0]:https://lore.kernel.org/linux-mm/CAG48ez0siYGB8GP5+Szgj2ovBZAkL6Zi4n6GUAjzzjFV9LTkRQ@mail.gmail.com/
> 
> Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>

nommu doesn't really make sense under this header, does it?

We're often (always?) involved with fixing that already.

Acked-by: Liam R. Howlett <Liam.Howlett@Oracle.com>

> ---
>  MAINTAINERS | 19 +++++++------------
>  1 file changed, 7 insertions(+), 12 deletions(-)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 1e930c7a58b1..95db20c26f5f 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -15060,18 +15060,6 @@ F:	tools/mm/
>  F:	tools/testing/selftests/mm/
>  N:	include/linux/page[-_]*
> 
> -MEMORY MAPPING
> -M:	Andrew Morton <akpm@linux-foundation.org>
> -M:	Liam R. Howlett <Liam.Howlett@oracle.com>
> -M:	Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> -R:	Vlastimil Babka <vbabka@suse.cz>
> -R:	Jann Horn <jannh@google.com>
> -L:	linux-mm@kvack.org
> -S:	Maintained
> -W:	http://www.linux-mm.org
> -T:	git git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
> -F:	mm/mmap.c
> -
>  MEMORY TECHNOLOGY DEVICES (MTD)
>  M:	Miquel Raynal <miquel.raynal@bootlin.com>
>  M:	Richard Weinberger <richard@nod.at>
> @@ -25028,6 +25016,13 @@ L:	linux-mm@kvack.org
>  S:	Maintained
>  W:	https://www.linux-mm.org
>  T:	git git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
> +F:	mm/madvise.c
> +F:	mm/mlock.c
> +F:	mm/mmap.c
> +F:	mm/mprotect.c
> +F:	mm/mremap.c
> +F:	mm/mseal.c
> +F:	mm/msync.c
>  F:	mm/vma.c
>  F:	mm/vma.h
>  F:	mm/vma_internal.h
> --
> 2.47.1

Re: [PATCH] MAINTAINERS: group all VMA-related files into the VMA section

[David Hildenbrand]

On 06.12.24 20:16, Lorenzo Stoakes wrote:
> There are a number of means of interacting with VMA operations within mm,
> and we have on occasion not been made aware of impactful changes due to
> these sitting in different files, most recently in [0].
> 
> Correct this by bringing all VMA operations under the same section in
> MAINTAINERS. Additionally take the opportunity to combine MEMORY MAPPING
> with VMA as there needn't be two entries as they amount to the same thing.
> 
> [0]:https://lore.kernel.org/linux-mm/CAG48ez0siYGB8GP5+Szgj2ovBZAkL6Zi4n6GUAjzzjFV9LTkRQ@mail.gmail.com/
> 
> Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> ---
>   MAINTAINERS | 19 +++++++------------
>   1 file changed, 7 insertions(+), 12 deletions(-)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 1e930c7a58b1..95db20c26f5f 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -15060,18 +15060,6 @@ F:	tools/mm/
>   F:	tools/testing/selftests/mm/
>   N:	include/linux/page[-_]*
> 
> -MEMORY MAPPING
> -M:	Andrew Morton <akpm@linux-foundation.org>
> -M:	Liam R. Howlett <Liam.Howlett@oracle.com>
> -M:	Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> -R:	Vlastimil Babka <vbabka@suse.cz>
> -R:	Jann Horn <jannh@google.com>
> -L:	linux-mm@kvack.org
> -S:	Maintained
> -W:	http://www.linux-mm.org
> -T:	git git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
> -F:	mm/mmap.c
> -
>   MEMORY TECHNOLOGY DEVICES (MTD)
>   M:	Miquel Raynal <miquel.raynal@bootlin.com>
>   M:	Richard Weinberger <richard@nod.at>
> @@ -25028,6 +25016,13 @@ L:	linux-mm@kvack.org
>   S:	Maintained
>   W:	https://www.linux-mm.org
>   T:	git git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
> +F:	mm/madvise.c
> +F:	mm/mlock.c
> +F:	mm/mmap.c
> +F:	mm/mprotect.c
> +F:	mm/mremap.c
> +F:	mm/mseal.c
> +F:	mm/msync.c

Not sure about mprotect.c, mlock.c and madvise.c, though. I'd claim that 
the real "magic" they perform is in page table handling and not 
primarily VMA handling (yes, both do VMA changes, but they are the 
"easy" part ;) ).

They have much more in common with memory.c, which I wouldn't want to 
see in here either. Hm.

-- 
Cheers,

David / dhildenb

Re: [PATCH] MAINTAINERS: group all VMA-related files into the VMA section

[Lorenzo Stoakes]

On Mon, Dec 09, 2024 at 10:16:21AM +0100, David Hildenbrand wrote:
> On 06.12.24 20:16, Lorenzo Stoakes wrote:
> > There are a number of means of interacting with VMA operations within mm,
> > and we have on occasion not been made aware of impactful changes due to
> > these sitting in different files, most recently in [0].
> >
> > Correct this by bringing all VMA operations under the same section in
> > MAINTAINERS. Additionally take the opportunity to combine MEMORY MAPPING
> > with VMA as there needn't be two entries as they amount to the same thing.
> >
> > [0]:https://lore.kernel.org/linux-mm/CAG48ez0siYGB8GP5+Szgj2ovBZAkL6Zi4n6GUAjzzjFV9LTkRQ@mail.gmail.com/
> >
> > Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> > ---
> >   MAINTAINERS | 19 +++++++------------
> >   1 file changed, 7 insertions(+), 12 deletions(-)
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 1e930c7a58b1..95db20c26f5f 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -15060,18 +15060,6 @@ F:	tools/mm/
> >   F:	tools/testing/selftests/mm/
> >   N:	include/linux/page[-_]*
> >
> > -MEMORY MAPPING
> > -M:	Andrew Morton <akpm@linux-foundation.org>
> > -M:	Liam R. Howlett <Liam.Howlett@oracle.com>
> > -M:	Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> > -R:	Vlastimil Babka <vbabka@suse.cz>
> > -R:	Jann Horn <jannh@google.com>
> > -L:	linux-mm@kvack.org
> > -S:	Maintained
> > -W:	http://www.linux-mm.org
> > -T:	git git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
> > -F:	mm/mmap.c
> > -
> >   MEMORY TECHNOLOGY DEVICES (MTD)
> >   M:	Miquel Raynal <miquel.raynal@bootlin.com>
> >   M:	Richard Weinberger <richard@nod.at>
> > @@ -25028,6 +25016,13 @@ L:	linux-mm@kvack.org
> >   S:	Maintained
> >   W:	https://www.linux-mm.org
> >   T:	git git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
> > +F:	mm/madvise.c
> > +F:	mm/mlock.c
> > +F:	mm/mmap.c
> > +F:	mm/mprotect.c
> > +F:	mm/mremap.c
> > +F:	mm/mseal.c
> > +F:	mm/msync.c
>
> Not sure about mprotect.c, mlock.c and madvise.c, though. I'd claim that the
> real "magic" they perform is in page table handling and not primarily VMA
> handling (yes, both do VMA changes, but they are the "easy" part ;) ).

And large parts of the VMA logic interface with page tables, see - the entire
6.12 cycle - around recent changes in mmap() MAP_FIXED - which... the VMA
maintainers fixed :)

And then there were the issues around VMA and mm locking relating to page
table work which... oh right yeah we had to fix again... :>)

I mean you can make this argument about probably all of these files (mremap
has -tons- of page table-specific stuff), and then we get back to not being
notified about key changes that interface with memory mapping/VMA which we
end up having to deal with anyway.

A lot of the reason we have 'magic' in these files anyway is because we
don't have a decent generic page table handler. Not sure I'd actually use
the word 'magic' for that though.

I am planning to make significant changes to mprotect/mlock soon, which
have some terribly duplicated horrible handling logic, and both are key
considerations in VMA logic as a whole.

Anyway, as far as I'm concerned page table manipulation after the point of
faulting is completely within the purvue of VMA manipulation and a side
product of it.

However, can concede mm/madvise.c if you feel strongly about that as that's
a bit blurry, but of course contains a whole bunch of VMA and... page table
manipulation :) I mean it still to me seems very pertinent.

>
> They have much more in common with memory.c, which I wouldn't want to see in
> here either. Hm.

No, memory.c is really dedicated to fault handling. This is really
different from manipulating page tables in specific cases in my opinion.

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

In case you are concerned about how much code this actually spans, it's
~6ksloc out of ~127ksloc, with roughly half of that in mm/vma.c...

Re: [PATCH] MAINTAINERS: group all VMA-related files into the VMA section

[David Hildenbrand]

On 09.12.24 11:06, Lorenzo Stoakes wrote:
> On Mon, Dec 09, 2024 at 10:16:21AM +0100, David Hildenbrand wrote:
>> On 06.12.24 20:16, Lorenzo Stoakes wrote:
>>> There are a number of means of interacting with VMA operations within mm,
>>> and we have on occasion not been made aware of impactful changes due to
>>> these sitting in different files, most recently in [0].
>>>
>>> Correct this by bringing all VMA operations under the same section in
>>> MAINTAINERS. Additionally take the opportunity to combine MEMORY MAPPING
>>> with VMA as there needn't be two entries as they amount to the same thing.
>>>
>>> [0]:https://lore.kernel.org/linux-mm/CAG48ez0siYGB8GP5+Szgj2ovBZAkL6Zi4n6GUAjzzjFV9LTkRQ@mail.gmail.com/
>>>
>>> Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
>>> ---
>>>    MAINTAINERS | 19 +++++++------------
>>>    1 file changed, 7 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>> index 1e930c7a58b1..95db20c26f5f 100644
>>> --- a/MAINTAINERS
>>> +++ b/MAINTAINERS
>>> @@ -15060,18 +15060,6 @@ F:	tools/mm/
>>>    F:	tools/testing/selftests/mm/
>>>    N:	include/linux/page[-_]*
>>>
>>> -MEMORY MAPPING
>>> -M:	Andrew Morton <akpm@linux-foundation.org>
>>> -M:	Liam R. Howlett <Liam.Howlett@oracle.com>
>>> -M:	Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
>>> -R:	Vlastimil Babka <vbabka@suse.cz>
>>> -R:	Jann Horn <jannh@google.com>
>>> -L:	linux-mm@kvack.org
>>> -S:	Maintained
>>> -W:	http://www.linux-mm.org
>>> -T:	git git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
>>> -F:	mm/mmap.c
>>> -
>>>    MEMORY TECHNOLOGY DEVICES (MTD)
>>>    M:	Miquel Raynal <miquel.raynal@bootlin.com>
>>>    M:	Richard Weinberger <richard@nod.at>
>>> @@ -25028,6 +25016,13 @@ L:	linux-mm@kvack.org
>>>    S:	Maintained
>>>    W:	https://www.linux-mm.org
>>>    T:	git git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
>>> +F:	mm/madvise.c
>>> +F:	mm/mlock.c
>>> +F:	mm/mmap.c
>>> +F:	mm/mprotect.c
>>> +F:	mm/mremap.c
>>> +F:	mm/mseal.c
>>> +F:	mm/msync.c
>>
>> Not sure about mprotect.c, mlock.c and madvise.c, though. I'd claim that the
>> real "magic" they perform is in page table handling and not primarily VMA
>> handling (yes, both do VMA changes, but they are the "easy" part ;) ).
> 
> And large parts of the VMA logic interface with page tables, see - the entire
> 6.12 cycle - around recent changes in mmap() MAP_FIXED - which... the VMA
> maintainers fixed :)
> 
> And then there were the issues around VMA and mm locking relating to page
> table work which... oh right yeah we had to fix again... :>)
> 
> I mean you can make this argument about probably all of these files (mremap
> has -tons- of page table-specific stuff), and then we get back to not being
> notified about key changes that interface with memory mapping/VMA which we
> end up having to deal with anyway.
> 
> A lot of the reason we have 'magic' in these files anyway is because we
> don't have a decent generic page table handler. Not sure I'd actually use
> the word 'magic' for that though.
> 
> I am planning to make significant changes to mprotect/mlock soon, which
> have some terribly duplicated horrible handling logic, and both are key
> considerations in VMA logic as a whole.
> 
> Anyway, as far as I'm concerned page table manipulation after the point of
> faulting is completely within the purvue of VMA manipulation and a side
> product of it.
> 
> However, can concede mm/madvise.c if you feel strongly about that as that's
> a bit blurry, but of course contains a whole bunch of VMA and... page table
> manipulation :) I mean it still to me seems very pertinent.
> 

And then we have mprotect.c being heavily used by uffd-wp and NUMA 
hinting, which don't perform any VMA modification.

That's why I don't think the change proposed here is really the right step.

 >>>> They have much more in common with memory.c, which I wouldn't want 
to see in
>> here either. Hm.
> 
> No, memory.c is really dedicated to fault handling. This is really
> different from manipulating page tables in specific cases in my opinion.

And fork and such stuff. And if you look into huge_memory.c, we actually 
moved all of the THP logic for mprotect()/madvise()/... in there.

Not sure if something similar should have been done for memory.c, or if 
the THP stuff should actually also have gone into the respective files.

To me it sounds wrong to have VMA maintainers maintain a lot of the code 
in these files code because these files somehow modify VMAs, sorry.

-- 
Cheers,

David / dhildenb

Re: [PATCH] MAINTAINERS: group all VMA-related files into the VMA section

[Lorenzo Stoakes]

On Mon, Dec 09, 2024 at 03:09:26PM +0100, David Hildenbrand wrote:
> On 09.12.24 11:06, Lorenzo Stoakes wrote:
> > On Mon, Dec 09, 2024 at 10:16:21AM +0100, David Hildenbrand wrote:
> > > On 06.12.24 20:16, Lorenzo Stoakes wrote:
> > > > There are a number of means of interacting with VMA operations within mm,
> > > > and we have on occasion not been made aware of impactful changes due to
> > > > these sitting in different files, most recently in [0].
> > > >
> > > > Correct this by bringing all VMA operations under the same section in
> > > > MAINTAINERS. Additionally take the opportunity to combine MEMORY MAPPING
> > > > with VMA as there needn't be two entries as they amount to the same thing.
> > > >
> > > > [0]:https://lore.kernel.org/linux-mm/CAG48ez0siYGB8GP5+Szgj2ovBZAkL6Zi4n6GUAjzzjFV9LTkRQ@mail.gmail.com/
> > > >
> > > > Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> > > > ---
> > > >    MAINTAINERS | 19 +++++++------------
> > > >    1 file changed, 7 insertions(+), 12 deletions(-)
> > > >
> > > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > > index 1e930c7a58b1..95db20c26f5f 100644
> > > > --- a/MAINTAINERS
> > > > +++ b/MAINTAINERS
> > > > @@ -15060,18 +15060,6 @@ F:	tools/mm/
> > > >    F:	tools/testing/selftests/mm/
> > > >    N:	include/linux/page[-_]*
> > > >
> > > > -MEMORY MAPPING
> > > > -M:	Andrew Morton <akpm@linux-foundation.org>
> > > > -M:	Liam R. Howlett <Liam.Howlett@oracle.com>
> > > > -M:	Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> > > > -R:	Vlastimil Babka <vbabka@suse.cz>
> > > > -R:	Jann Horn <jannh@google.com>
> > > > -L:	linux-mm@kvack.org
> > > > -S:	Maintained
> > > > -W:	http://www.linux-mm.org
> > > > -T:	git git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
> > > > -F:	mm/mmap.c
> > > > -
> > > >    MEMORY TECHNOLOGY DEVICES (MTD)
> > > >    M:	Miquel Raynal <miquel.raynal@bootlin.com>
> > > >    M:	Richard Weinberger <richard@nod.at>
> > > > @@ -25028,6 +25016,13 @@ L:	linux-mm@kvack.org
> > > >    S:	Maintained
> > > >    W:	https://www.linux-mm.org
> > > >    T:	git git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
> > > > +F:	mm/madvise.c
> > > > +F:	mm/mlock.c
> > > > +F:	mm/mmap.c
> > > > +F:	mm/mprotect.c
> > > > +F:	mm/mremap.c
> > > > +F:	mm/mseal.c
> > > > +F:	mm/msync.c
> > >
> > > Not sure about mprotect.c, mlock.c and madvise.c, though. I'd claim that the
> > > real "magic" they perform is in page table handling and not primarily VMA
> > > handling (yes, both do VMA changes, but they are the "easy" part ;) ).
> >
> > And large parts of the VMA logic interface with page tables, see - the entire
> > 6.12 cycle - around recent changes in mmap() MAP_FIXED - which... the VMA
> > maintainers fixed :)
> >
> > And then there were the issues around VMA and mm locking relating to page
> > table work which... oh right yeah we had to fix again... :>)
> >
> > I mean you can make this argument about probably all of these files (mremap
> > has -tons- of page table-specific stuff), and then we get back to not being
> > notified about key changes that interface with memory mapping/VMA which we
> > end up having to deal with anyway.
> >
> > A lot of the reason we have 'magic' in these files anyway is because we
> > don't have a decent generic page table handler. Not sure I'd actually use
> > the word 'magic' for that though.
> >
> > I am planning to make significant changes to mprotect/mlock soon, which
> > have some terribly duplicated horrible handling logic, and both are key
> > considerations in VMA logic as a whole.
> >
> > Anyway, as far as I'm concerned page table manipulation after the point of
> > faulting is completely within the purvue of VMA manipulation and a side
> > product of it.
> >
> > However, can concede mm/madvise.c if you feel strongly about that as that's
> > a bit blurry, but of course contains a whole bunch of VMA and... page table
> > manipulation :) I mean it still to me seems very pertinent.
> >
>
> And then we have mprotect.c being heavily used by uffd-wp and NUMA hinting,
> which don't perform any VMA modification.
>
> That's why I don't think the change proposed here is really the right step.
>
> >>>> They have much more in common with memory.c, which I wouldn't want to
> see in
> > > here either. Hm.
> >
> > No, memory.c is really dedicated to fault handling. This is really
> > different from manipulating page tables in specific cases in my opinion.
>
> And fork and such stuff. And if you look into huge_memory.c, we actually
> moved all of the THP logic for mprotect()/madvise()/... in there.
>
> Not sure if something similar should have been done for memory.c, or if the
> THP stuff should actually also have gone into the respective files.
>
> To me it sounds wrong to have VMA maintainers maintain a lot of the code in
> these files code because these files somehow modify VMAs, sorry.

This isn't what I said, I said that de facto we (that is the MEMORY MAPPING
maintainers as well as VMA) were dealing with a great many issues around
page tables and page table manipulation which are rather inseparable from
one another.

I even went to the lengths of writing a detailed set of documentation on
locking behaviour in and around page table manipulation and solved
security-sensitive issues in relation to page table teardown over the 6.12
rc cycle.

To me, the idea that mprotect() and mlock(), operations that are explicitly
about manipulating VMAs (_and of course consqeuent page table
manipulation_), are somehow separate is really bizarre to me, but I respect
your opinion even if I disagree.

But unfortunately your arguments apply equally as well to mremap.c (more
than half of which is dedicated to page table manipulation), so I will have
to drop the whole patch then.

If issues arise there in future, I guess others will have to deal with them
if we don't notice them (luckily Jann did and pinged this time, hopefully
will in future).

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

To be clear, I made this change in the interests of the community and
contributing. It seems to me that within mm has far too little sharing of
the maintainership burden and I only wanted to help with that and make
explicit what I work on day-to-day.

I am glad you at least don't object to my doing so with respect to at least
some parts of the VMA logic.

Re: [PATCH] MAINTAINERS: group all VMA-related files into the VMA section

[David Hildenbrand]

On 09.12.24 15:28, Lorenzo Stoakes wrote:
> On Mon, Dec 09, 2024 at 03:09:26PM +0100, David Hildenbrand wrote:
>> On 09.12.24 11:06, Lorenzo Stoakes wrote:
>>> On Mon, Dec 09, 2024 at 10:16:21AM +0100, David Hildenbrand wrote:
>>>> On 06.12.24 20:16, Lorenzo Stoakes wrote:
>>>>> There are a number of means of interacting with VMA operations within mm,
>>>>> and we have on occasion not been made aware of impactful changes due to
>>>>> these sitting in different files, most recently in [0].
>>>>>
>>>>> Correct this by bringing all VMA operations under the same section in
>>>>> MAINTAINERS. Additionally take the opportunity to combine MEMORY MAPPING
>>>>> with VMA as there needn't be two entries as they amount to the same thing.
>>>>>
>>>>> [0]:https://lore.kernel.org/linux-mm/CAG48ez0siYGB8GP5+Szgj2ovBZAkL6Zi4n6GUAjzzjFV9LTkRQ@mail.gmail.com/
>>>>>
>>>>> Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
>>>>> ---
>>>>>     MAINTAINERS | 19 +++++++------------
>>>>>     1 file changed, 7 insertions(+), 12 deletions(-)
>>>>>
>>>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>>>> index 1e930c7a58b1..95db20c26f5f 100644
>>>>> --- a/MAINTAINERS
>>>>> +++ b/MAINTAINERS
>>>>> @@ -15060,18 +15060,6 @@ F:	tools/mm/
>>>>>     F:	tools/testing/selftests/mm/
>>>>>     N:	include/linux/page[-_]*
>>>>>
>>>>> -MEMORY MAPPING
>>>>> -M:	Andrew Morton <akpm@linux-foundation.org>
>>>>> -M:	Liam R. Howlett <Liam.Howlett@oracle.com>
>>>>> -M:	Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
>>>>> -R:	Vlastimil Babka <vbabka@suse.cz>
>>>>> -R:	Jann Horn <jannh@google.com>
>>>>> -L:	linux-mm@kvack.org
>>>>> -S:	Maintained
>>>>> -W:	http://www.linux-mm.org
>>>>> -T:	git git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
>>>>> -F:	mm/mmap.c
>>>>> -
>>>>>     MEMORY TECHNOLOGY DEVICES (MTD)
>>>>>     M:	Miquel Raynal <miquel.raynal@bootlin.com>
>>>>>     M:	Richard Weinberger <richard@nod.at>
>>>>> @@ -25028,6 +25016,13 @@ L:	linux-mm@kvack.org
>>>>>     S:	Maintained
>>>>>     W:	https://www.linux-mm.org
>>>>>     T:	git git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
>>>>> +F:	mm/madvise.c
>>>>> +F:	mm/mlock.c
>>>>> +F:	mm/mmap.c
>>>>> +F:	mm/mprotect.c
>>>>> +F:	mm/mremap.c
>>>>> +F:	mm/mseal.c
>>>>> +F:	mm/msync.c
>>>>
>>>> Not sure about mprotect.c, mlock.c and madvise.c, though. I'd claim that the
>>>> real "magic" they perform is in page table handling and not primarily VMA
>>>> handling (yes, both do VMA changes, but they are the "easy" part ;) ).
>>>
>>> And large parts of the VMA logic interface with page tables, see - the entire
>>> 6.12 cycle - around recent changes in mmap() MAP_FIXED - which... the VMA
>>> maintainers fixed :)
>>>
>>> And then there were the issues around VMA and mm locking relating to page
>>> table work which... oh right yeah we had to fix again... :>)
>>>
>>> I mean you can make this argument about probably all of these files (mremap
>>> has -tons- of page table-specific stuff), and then we get back to not being
>>> notified about key changes that interface with memory mapping/VMA which we
>>> end up having to deal with anyway.
>>>
>>> A lot of the reason we have 'magic' in these files anyway is because we
>>> don't have a decent generic page table handler. Not sure I'd actually use
>>> the word 'magic' for that though.
>>>
>>> I am planning to make significant changes to mprotect/mlock soon, which
>>> have some terribly duplicated horrible handling logic, and both are key
>>> considerations in VMA logic as a whole.
>>>
>>> Anyway, as far as I'm concerned page table manipulation after the point of
>>> faulting is completely within the purvue of VMA manipulation and a side
>>> product of it.
>>>
>>> However, can concede mm/madvise.c if you feel strongly about that as that's
>>> a bit blurry, but of course contains a whole bunch of VMA and... page table
>>> manipulation :) I mean it still to me seems very pertinent.
>>>
>>
>> And then we have mprotect.c being heavily used by uffd-wp and NUMA hinting,
>> which don't perform any VMA modification.
>>
>> That's why I don't think the change proposed here is really the right step.
>>
>>>>>> They have much more in common with memory.c, which I wouldn't want to
>> see in
>>>> here either. Hm.
>>>
>>> No, memory.c is really dedicated to fault handling. This is really
>>> different from manipulating page tables in specific cases in my opinion.
>>
>> And fork and such stuff. And if you look into huge_memory.c, we actually
>> moved all of the THP logic for mprotect()/madvise()/... in there.
>>
>> Not sure if something similar should have been done for memory.c, or if the
>> THP stuff should actually also have gone into the respective files.
>>
>> To me it sounds wrong to have VMA maintainers maintain a lot of the code in
>> these files code because these files somehow modify VMAs, sorry.
> 
> This isn't what I said, I said that de facto we (that is the MEMORY MAPPING
> maintainers as well as VMA) were dealing with a great many issues around
> page tables and page table manipulation which are rather inseparable from
> one another.
> 
> I even went to the lengths of writing a detailed set of documentation on
> locking behaviour in and around page table manipulation and solved
> security-sensitive issues in relation to page table teardown over the 6.12
> rc cycle.
> 
> To me, the idea that mprotect() and mlock(), operations that are explicitly
> about manipulating VMAs (_and of course consqeuent page table
> manipulation_), are somehow separate is really bizarre to me, but I respect
> your opinion even if I disagree.
 > > But unfortunately your arguments apply equally as well to mremap.c 
(more
> than half of which is dedicated to page table manipulation), so I will have
> to drop the whole patch then.
> 
> If issues arise there in future, I guess others will have to deal with them
> if we don't notice them (luckily Jann did and pinged this time, hopefully
> will in future).

Again, the main point I have is that basic VMA handling (the now very 
nice vma.c! ) that wouldn't even require page table modifications (the 
very nice testing framework you added) is different to users of that 
functionality.

And I agree that many VMA modification users imply page table 
modifications and more.

> 
>>
>> --
>> Cheers,
>>
>> David / dhildenb
>>
> 
> To be clear, I made this change in the interests of the community and
> contributing. It seems to me that within mm has far too little sharing of
> the maintainership burden and I only wanted to help with that and make
> explicit what I work on day-to-day.

And I appreciate that. But putting everything that touches VMAs under 
VMA is wrong to me.

> 
> I am glad you at least don't object to my doing so with respect to at least
> some parts of the VMA logic.

I really enjoy how well you separated the core VMA logic from everything 
else.

-- 
Cheers,

David / dhildenb

Re: [PATCH] MAINTAINERS: group all VMA-related files into the VMA section

[Lorenzo Stoakes]

David -

The key aim here is to avoid having something get merged that everyone misses
that sets off a landmine... So.

Let me propose a few things, and if none work for you, perhaps you can suggest
one?

1. We also maintain mm/mmap.c under 'MEMORY MAPPING' (see patch below). Perhaps
   we could place these files here, and add you too as a maintainer, if Andrew
   were open to it? That way you can keep an eye on it? Perhaps others who have
   relevant knowledge?

2. We add a new PAGE TABLE section, with us and yourself + others as maintainers
   which reference the files which explicitly manipulate page tables as the core
   'magic'?

3. We do the same as 2, but add yourself + any others you suggest as maintainers
   and us memory mapping/vma guys as reviewers, if you feel that this makes
   more sense given our relevant areas of expertise?

Or whatever you might propose? Or would you prefer things remain as they are?

Key aim here is so we can explcitly avoid anything that breaks or blows up
memory mapping stuff?

Thanks, Lorenzo


On Mon, Dec 09, 2024 at 03:56:55PM +0100, David Hildenbrand wrote:
> On 09.12.24 15:28, Lorenzo Stoakes wrote:
> > On Mon, Dec 09, 2024 at 03:09:26PM +0100, David Hildenbrand wrote:
> > > On 09.12.24 11:06, Lorenzo Stoakes wrote:
> > > > On Mon, Dec 09, 2024 at 10:16:21AM +0100, David Hildenbrand wrote:
> > > > > On 06.12.24 20:16, Lorenzo Stoakes wrote:
> > > > > > There are a number of means of interacting with VMA operations within mm,
> > > > > > and we have on occasion not been made aware of impactful changes due to
> > > > > > these sitting in different files, most recently in [0].
> > > > > >
> > > > > > Correct this by bringing all VMA operations under the same section in
> > > > > > MAINTAINERS. Additionally take the opportunity to combine MEMORY MAPPING
> > > > > > with VMA as there needn't be two entries as they amount to the same thing.
> > > > > >
> > > > > > [0]:https://lore.kernel.org/linux-mm/CAG48ez0siYGB8GP5+Szgj2ovBZAkL6Zi4n6GUAjzzjFV9LTkRQ@mail.gmail.com/
> > > > > >
> > > > > > Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> > > > > > ---
> > > > > >     MAINTAINERS | 19 +++++++------------
> > > > > >     1 file changed, 7 insertions(+), 12 deletions(-)
> > > > > >
> > > > > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > > > > index 1e930c7a58b1..95db20c26f5f 100644
> > > > > > --- a/MAINTAINERS
> > > > > > +++ b/MAINTAINERS
> > > > > > @@ -15060,18 +15060,6 @@ F:	tools/mm/
> > > > > >     F:	tools/testing/selftests/mm/
> > > > > >     N:	include/linux/page[-_]*
> > > > > >
> > > > > > -MEMORY MAPPING
> > > > > > -M:	Andrew Morton <akpm@linux-foundation.org>
> > > > > > -M:	Liam R. Howlett <Liam.Howlett@oracle.com>
> > > > > > -M:	Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> > > > > > -R:	Vlastimil Babka <vbabka@suse.cz>
> > > > > > -R:	Jann Horn <jannh@google.com>
> > > > > > -L:	linux-mm@kvack.org
> > > > > > -S:	Maintained
> > > > > > -W:	http://www.linux-mm.org
> > > > > > -T:	git git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
> > > > > > -F:	mm/mmap.c
> > > > > > -
> > > > > >     MEMORY TECHNOLOGY DEVICES (MTD)
> > > > > >     M:	Miquel Raynal <miquel.raynal@bootlin.com>
> > > > > >     M:	Richard Weinberger <richard@nod.at>
> > > > > > @@ -25028,6 +25016,13 @@ L:	linux-mm@kvack.org
> > > > > >     S:	Maintained
> > > > > >     W:	https://www.linux-mm.org
> > > > > >     T:	git git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
> > > > > > +F:	mm/madvise.c
> > > > > > +F:	mm/mlock.c
> > > > > > +F:	mm/mmap.c
> > > > > > +F:	mm/mprotect.c
> > > > > > +F:	mm/mremap.c
> > > > > > +F:	mm/mseal.c
> > > > > > +F:	mm/msync.c
> > > > >
> > > > > Not sure about mprotect.c, mlock.c and madvise.c, though. I'd claim that the
> > > > > real "magic" they perform is in page table handling and not primarily VMA
> > > > > handling (yes, both do VMA changes, but they are the "easy" part ;) ).
> > > >
> > > > And large parts of the VMA logic interface with page tables, see - the entire
> > > > 6.12 cycle - around recent changes in mmap() MAP_FIXED - which... the VMA
> > > > maintainers fixed :)
> > > >
> > > > And then there were the issues around VMA and mm locking relating to page
> > > > table work which... oh right yeah we had to fix again... :>)
> > > >
> > > > I mean you can make this argument about probably all of these files (mremap
> > > > has -tons- of page table-specific stuff), and then we get back to not being
> > > > notified about key changes that interface with memory mapping/VMA which we
> > > > end up having to deal with anyway.
> > > >
> > > > A lot of the reason we have 'magic' in these files anyway is because we
> > > > don't have a decent generic page table handler. Not sure I'd actually use
> > > > the word 'magic' for that though.
> > > >
> > > > I am planning to make significant changes to mprotect/mlock soon, which
> > > > have some terribly duplicated horrible handling logic, and both are key
> > > > considerations in VMA logic as a whole.
> > > >
> > > > Anyway, as far as I'm concerned page table manipulation after the point of
> > > > faulting is completely within the purvue of VMA manipulation and a side
> > > > product of it.
> > > >
> > > > However, can concede mm/madvise.c if you feel strongly about that as that's
> > > > a bit blurry, but of course contains a whole bunch of VMA and... page table
> > > > manipulation :) I mean it still to me seems very pertinent.
> > > >
> > >
> > > And then we have mprotect.c being heavily used by uffd-wp and NUMA hinting,
> > > which don't perform any VMA modification.
> > >
> > > That's why I don't think the change proposed here is really the right step.
> > >
> > > > > > > They have much more in common with memory.c, which I wouldn't want to
> > > see in
> > > > > here either. Hm.
> > > >
> > > > No, memory.c is really dedicated to fault handling. This is really
> > > > different from manipulating page tables in specific cases in my opinion.
> > >
> > > And fork and such stuff. And if you look into huge_memory.c, we actually
> > > moved all of the THP logic for mprotect()/madvise()/... in there.
> > >
> > > Not sure if something similar should have been done for memory.c, or if the
> > > THP stuff should actually also have gone into the respective files.
> > >
> > > To me it sounds wrong to have VMA maintainers maintain a lot of the code in
> > > these files code because these files somehow modify VMAs, sorry.
> >
> > This isn't what I said, I said that de facto we (that is the MEMORY MAPPING
> > maintainers as well as VMA) were dealing with a great many issues around
> > page tables and page table manipulation which are rather inseparable from
> > one another.
> >
> > I even went to the lengths of writing a detailed set of documentation on
> > locking behaviour in and around page table manipulation and solved
> > security-sensitive issues in relation to page table teardown over the 6.12
> > rc cycle.
> >
> > To me, the idea that mprotect() and mlock(), operations that are explicitly
> > about manipulating VMAs (_and of course consqeuent page table
> > manipulation_), are somehow separate is really bizarre to me, but I respect
> > your opinion even if I disagree.
> > > But unfortunately your arguments apply equally as well to mremap.c (more
> > than half of which is dedicated to page table manipulation), so I will have
> > to drop the whole patch then.
> >
> > If issues arise there in future, I guess others will have to deal with them
> > if we don't notice them (luckily Jann did and pinged this time, hopefully
> > will in future).
>
> Again, the main point I have is that basic VMA handling (the now very nice
> vma.c! ) that wouldn't even require page table modifications (the very nice
> testing framework you added) is different to users of that functionality.
>
> And I agree that many VMA modification users imply page table modifications
> and more.

Yes.

>
> >
> > >
> > > --
> > > Cheers,
> > >
> > > David / dhildenb
> > >
> >
> > To be clear, I made this change in the interests of the community and
> > contributing. It seems to me that within mm has far too little sharing of
> > the maintainership burden and I only wanted to help with that and make
> > explicit what I work on day-to-day.
>
> And I appreciate that. But putting everything that touches VMAs under VMA is
> wrong to me.

Well perhaps the issue here is the section then? See the bit at the top.

>
> >
> > I am glad you at least don't object to my doing so with respect to at least
> > some parts of the VMA logic.
>
> I really enjoy how well you separated the core VMA logic from everything
> else.
>

Thanks! Why I admire your work too... :)

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

Re: [PATCH] MAINTAINERS: group all VMA-related files into the VMA section

[Vlastimil Babka]

On 12/9/24 10:16, David Hildenbrand wrote:
> On 06.12.24 20:16, Lorenzo Stoakes wrote:
>> There are a number of means of interacting with VMA operations within mm,
>> and we have on occasion not been made aware of impactful changes due to
>> these sitting in different files, most recently in [0].
>> 
>> Correct this by bringing all VMA operations under the same section in
>> MAINTAINERS. Additionally take the opportunity to combine MEMORY MAPPING
>> with VMA as there needn't be two entries as they amount to the same thing.
>> 
>> [0]:https://lore.kernel.org/linux-mm/CAG48ez0siYGB8GP5+Szgj2ovBZAkL6Zi4n6GUAjzzjFV9LTkRQ@mail.gmail.com/
>> 
>> Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
>> ---
>>   MAINTAINERS | 19 +++++++------------
>>   1 file changed, 7 insertions(+), 12 deletions(-)
>> 
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 1e930c7a58b1..95db20c26f5f 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -15060,18 +15060,6 @@ F:	tools/mm/
>>   F:	tools/testing/selftests/mm/
>>   N:	include/linux/page[-_]*
>> 
>> -MEMORY MAPPING
>> -M:	Andrew Morton <akpm@linux-foundation.org>
>> -M:	Liam R. Howlett <Liam.Howlett@oracle.com>
>> -M:	Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
>> -R:	Vlastimil Babka <vbabka@suse.cz>
>> -R:	Jann Horn <jannh@google.com>
>> -L:	linux-mm@kvack.org
>> -S:	Maintained
>> -W:	http://www.linux-mm.org
>> -T:	git git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
>> -F:	mm/mmap.c
>> -
>>   MEMORY TECHNOLOGY DEVICES (MTD)
>>   M:	Miquel Raynal <miquel.raynal@bootlin.com>
>>   M:	Richard Weinberger <richard@nod.at>
>> @@ -25028,6 +25016,13 @@ L:	linux-mm@kvack.org
>>   S:	Maintained
>>   W:	https://www.linux-mm.org
>>   T:	git git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
>> +F:	mm/madvise.c
>> +F:	mm/mlock.c
>> +F:	mm/mmap.c
>> +F:	mm/mprotect.c
>> +F:	mm/mremap.c
>> +F:	mm/mseal.c
>> +F:	mm/msync.c
> 
> Not sure about mprotect.c, mlock.c and madvise.c, though. I'd claim that 
> the real "magic" they perform is in page table handling and not 
> primarily VMA handling (yes, both do VMA changes, but they are the 
> "easy" part ;) ).

I'd think that moving vma files into MEMORY MAPPING (and not the other way)
would result in a better overal name, that would be a better fit for the
newly added files too?

> They have much more in common with memory.c, which I wouldn't want to 
> see in here either. Hm.
> 

Re: [PATCH] MAINTAINERS: group all VMA-related files into the VMA section

[David Hildenbrand]

On 09.12.24 14:25, Vlastimil Babka wrote:
> On 12/9/24 10:16, David Hildenbrand wrote:
>> On 06.12.24 20:16, Lorenzo Stoakes wrote:
>>> There are a number of means of interacting with VMA operations within mm,
>>> and we have on occasion not been made aware of impactful changes due to
>>> these sitting in different files, most recently in [0].
>>>
>>> Correct this by bringing all VMA operations under the same section in
>>> MAINTAINERS. Additionally take the opportunity to combine MEMORY MAPPING
>>> with VMA as there needn't be two entries as they amount to the same thing.
>>>
>>> [0]:https://lore.kernel.org/linux-mm/CAG48ez0siYGB8GP5+Szgj2ovBZAkL6Zi4n6GUAjzzjFV9LTkRQ@mail.gmail.com/
>>>
>>> Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
>>> ---
>>>    MAINTAINERS | 19 +++++++------------
>>>    1 file changed, 7 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>> index 1e930c7a58b1..95db20c26f5f 100644
>>> --- a/MAINTAINERS
>>> +++ b/MAINTAINERS
>>> @@ -15060,18 +15060,6 @@ F:	tools/mm/
>>>    F:	tools/testing/selftests/mm/
>>>    N:	include/linux/page[-_]*
>>>
>>> -MEMORY MAPPING
>>> -M:	Andrew Morton <akpm@linux-foundation.org>
>>> -M:	Liam R. Howlett <Liam.Howlett@oracle.com>
>>> -M:	Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
>>> -R:	Vlastimil Babka <vbabka@suse.cz>
>>> -R:	Jann Horn <jannh@google.com>
>>> -L:	linux-mm@kvack.org
>>> -S:	Maintained
>>> -W:	http://www.linux-mm.org
>>> -T:	git git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
>>> -F:	mm/mmap.c
>>> -
>>>    MEMORY TECHNOLOGY DEVICES (MTD)
>>>    M:	Miquel Raynal <miquel.raynal@bootlin.com>
>>>    M:	Richard Weinberger <richard@nod.at>
>>> @@ -25028,6 +25016,13 @@ L:	linux-mm@kvack.org
>>>    S:	Maintained
>>>    W:	https://www.linux-mm.org
>>>    T:	git git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
>>> +F:	mm/madvise.c
>>> +F:	mm/mlock.c
>>> +F:	mm/mmap.c
>>> +F:	mm/mprotect.c
>>> +F:	mm/mremap.c
>>> +F:	mm/mseal.c
>>> +F:	mm/msync.c
>>
>> Not sure about mprotect.c, mlock.c and madvise.c, though. I'd claim that
>> the real "magic" they perform is in page table handling and not
>> primarily VMA handling (yes, both do VMA changes, but they are the
>> "easy" part ;) ).
> 
> I'd think that moving vma files into MEMORY MAPPING (and not the other way)
> would result in a better overal name, that would be a better fit for the
> newly added files too?

Maybe. I think vma.c should likely have a different set of maintainers 
than madvise.c and mprotect.c. (again, the magic is in page table 
modifications)

-- 
Cheers,

David / dhildenb

Re: [PATCH] MAINTAINERS: group all VMA-related files into the VMA section

[Lorenzo Stoakes]

On Mon, Dec 09, 2024 at 03:00:08PM +0100, David Hildenbrand wrote:
> On 09.12.24 14:25, Vlastimil Babka wrote:
> > On 12/9/24 10:16, David Hildenbrand wrote:
> > > On 06.12.24 20:16, Lorenzo Stoakes wrote:
> > > > There are a number of means of interacting with VMA operations within mm,
> > > > and we have on occasion not been made aware of impactful changes due to
> > > > these sitting in different files, most recently in [0].
> > > >
> > > > Correct this by bringing all VMA operations under the same section in
> > > > MAINTAINERS. Additionally take the opportunity to combine MEMORY MAPPING
> > > > with VMA as there needn't be two entries as they amount to the same thing.
> > > >
> > > > [0]:https://lore.kernel.org/linux-mm/CAG48ez0siYGB8GP5+Szgj2ovBZAkL6Zi4n6GUAjzzjFV9LTkRQ@mail.gmail.com/
> > > >
> > > > Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> > > > ---
> > > >    MAINTAINERS | 19 +++++++------------
> > > >    1 file changed, 7 insertions(+), 12 deletions(-)
> > > >
> > > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > > index 1e930c7a58b1..95db20c26f5f 100644
> > > > --- a/MAINTAINERS
> > > > +++ b/MAINTAINERS
> > > > @@ -15060,18 +15060,6 @@ F:	tools/mm/
> > > >    F:	tools/testing/selftests/mm/
> > > >    N:	include/linux/page[-_]*
> > > >
> > > > -MEMORY MAPPING
> > > > -M:	Andrew Morton <akpm@linux-foundation.org>
> > > > -M:	Liam R. Howlett <Liam.Howlett@oracle.com>
> > > > -M:	Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> > > > -R:	Vlastimil Babka <vbabka@suse.cz>
> > > > -R:	Jann Horn <jannh@google.com>
> > > > -L:	linux-mm@kvack.org
> > > > -S:	Maintained
> > > > -W:	http://www.linux-mm.org
> > > > -T:	git git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
> > > > -F:	mm/mmap.c
> > > > -
> > > >    MEMORY TECHNOLOGY DEVICES (MTD)
> > > >    M:	Miquel Raynal <miquel.raynal@bootlin.com>
> > > >    M:	Richard Weinberger <richard@nod.at>
> > > > @@ -25028,6 +25016,13 @@ L:	linux-mm@kvack.org
> > > >    S:	Maintained
> > > >    W:	https://www.linux-mm.org
> > > >    T:	git git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
> > > > +F:	mm/madvise.c
> > > > +F:	mm/mlock.c
> > > > +F:	mm/mmap.c
> > > > +F:	mm/mprotect.c
> > > > +F:	mm/mremap.c
> > > > +F:	mm/mseal.c
> > > > +F:	mm/msync.c
> > >
> > > Not sure about mprotect.c, mlock.c and madvise.c, though. I'd claim that
> > > the real "magic" they perform is in page table handling and not
> > > primarily VMA handling (yes, both do VMA changes, but they are the
> > > "easy" part ;) ).
> >
> > I'd think that moving vma files into MEMORY MAPPING (and not the other way)
> > would result in a better overal name, that would be a better fit for the
> > newly added files too?
>
> Maybe. I think vma.c should likely have a different set of maintainers than
> madvise.c and mprotect.c. (again, the magic is in page table modifications)

The bulk of the logic in mremap.c is related to page tables so by this
logic then, that is out too, right?

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

Re: [PATCH] MAINTAINERS: group all VMA-related files into the VMA section

[David Hildenbrand]

On 09.12.24 15:11, Lorenzo Stoakes wrote:
> On Mon, Dec 09, 2024 at 03:00:08PM +0100, David Hildenbrand wrote:
>> On 09.12.24 14:25, Vlastimil Babka wrote:
>>> On 12/9/24 10:16, David Hildenbrand wrote:
>>>> On 06.12.24 20:16, Lorenzo Stoakes wrote:
>>>>> There are a number of means of interacting with VMA operations within mm,
>>>>> and we have on occasion not been made aware of impactful changes due to
>>>>> these sitting in different files, most recently in [0].
>>>>>
>>>>> Correct this by bringing all VMA operations under the same section in
>>>>> MAINTAINERS. Additionally take the opportunity to combine MEMORY MAPPING
>>>>> with VMA as there needn't be two entries as they amount to the same thing.
>>>>>
>>>>> [0]:https://lore.kernel.org/linux-mm/CAG48ez0siYGB8GP5+Szgj2ovBZAkL6Zi4n6GUAjzzjFV9LTkRQ@mail.gmail.com/
>>>>>
>>>>> Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
>>>>> ---
>>>>>     MAINTAINERS | 19 +++++++------------
>>>>>     1 file changed, 7 insertions(+), 12 deletions(-)
>>>>>
>>>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>>>> index 1e930c7a58b1..95db20c26f5f 100644
>>>>> --- a/MAINTAINERS
>>>>> +++ b/MAINTAINERS
>>>>> @@ -15060,18 +15060,6 @@ F:	tools/mm/
>>>>>     F:	tools/testing/selftests/mm/
>>>>>     N:	include/linux/page[-_]*
>>>>>
>>>>> -MEMORY MAPPING
>>>>> -M:	Andrew Morton <akpm@linux-foundation.org>
>>>>> -M:	Liam R. Howlett <Liam.Howlett@oracle.com>
>>>>> -M:	Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
>>>>> -R:	Vlastimil Babka <vbabka@suse.cz>
>>>>> -R:	Jann Horn <jannh@google.com>
>>>>> -L:	linux-mm@kvack.org
>>>>> -S:	Maintained
>>>>> -W:	http://www.linux-mm.org
>>>>> -T:	git git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
>>>>> -F:	mm/mmap.c
>>>>> -
>>>>>     MEMORY TECHNOLOGY DEVICES (MTD)
>>>>>     M:	Miquel Raynal <miquel.raynal@bootlin.com>
>>>>>     M:	Richard Weinberger <richard@nod.at>
>>>>> @@ -25028,6 +25016,13 @@ L:	linux-mm@kvack.org
>>>>>     S:	Maintained
>>>>>     W:	https://www.linux-mm.org
>>>>>     T:	git git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
>>>>> +F:	mm/madvise.c
>>>>> +F:	mm/mlock.c
>>>>> +F:	mm/mmap.c
>>>>> +F:	mm/mprotect.c
>>>>> +F:	mm/mremap.c
>>>>> +F:	mm/mseal.c
>>>>> +F:	mm/msync.c
>>>>
>>>> Not sure about mprotect.c, mlock.c and madvise.c, though. I'd claim that
>>>> the real "magic" they perform is in page table handling and not
>>>> primarily VMA handling (yes, both do VMA changes, but they are the
>>>> "easy" part ;) ).
>>>
>>> I'd think that moving vma files into MEMORY MAPPING (and not the other way)
>>> would result in a better overal name, that would be a better fit for the
>>> newly added files too?
>>
>> Maybe. I think vma.c should likely have a different set of maintainers than
>> madvise.c and mprotect.c. (again, the magic is in page table modifications)
> 
> The bulk of the logic in mremap.c is related to page tables so by this
> logic then, that is out too, right?

IMHO, yes.

-- 
Cheers,

David / dhildenb

Re: [PATCH] MAINTAINERS: group all VMA-related files into the VMA section

[Jann Horn]

On Mon, Dec 9, 2024 at 3:11 PM Lorenzo Stoakes
<lorenzo.stoakes@oracle.com> wrote:
> On Mon, Dec 09, 2024 at 03:00:08PM +0100, David Hildenbrand wrote:
> > On 09.12.24 14:25, Vlastimil Babka wrote:
> > > On 12/9/24 10:16, David Hildenbrand wrote:
> > > > On 06.12.24 20:16, Lorenzo Stoakes wrote:
> > > > > There are a number of means of interacting with VMA operations within mm,
> > > > > and we have on occasion not been made aware of impactful changes due to
> > > > > these sitting in different files, most recently in [0].
> > > > >
> > > > > Correct this by bringing all VMA operations under the same section in
> > > > > MAINTAINERS. Additionally take the opportunity to combine MEMORY MAPPING
> > > > > with VMA as there needn't be two entries as they amount to the same thing.
> > > > >
> > > > > [0]:https://lore.kernel.org/linux-mm/CAG48ez0siYGB8GP5+Szgj2ovBZAkL6Zi4n6GUAjzzjFV9LTkRQ@mail.gmail.com/
> > > > >
> > > > > Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> > > > > ---
> > > > >    MAINTAINERS | 19 +++++++------------
> > > > >    1 file changed, 7 insertions(+), 12 deletions(-)
> > > > >
> > > > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > > > index 1e930c7a58b1..95db20c26f5f 100644
> > > > > --- a/MAINTAINERS
> > > > > +++ b/MAINTAINERS
> > > > > @@ -15060,18 +15060,6 @@ F:     tools/mm/
> > > > >    F:   tools/testing/selftests/mm/
> > > > >    N:   include/linux/page[-_]*
> > > > >
> > > > > -MEMORY MAPPING
> > > > > -M:     Andrew Morton <akpm@linux-foundation.org>
> > > > > -M:     Liam R. Howlett <Liam.Howlett@oracle.com>
> > > > > -M:     Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> > > > > -R:     Vlastimil Babka <vbabka@suse.cz>
> > > > > -R:     Jann Horn <jannh@google.com>
> > > > > -L:     linux-mm@kvack.org
> > > > > -S:     Maintained
> > > > > -W:     http://www.linux-mm.org
> > > > > -T:     git git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
> > > > > -F:     mm/mmap.c
> > > > > -
> > > > >    MEMORY TECHNOLOGY DEVICES (MTD)
> > > > >    M:   Miquel Raynal <miquel.raynal@bootlin.com>
> > > > >    M:   Richard Weinberger <richard@nod.at>
> > > > > @@ -25028,6 +25016,13 @@ L:     linux-mm@kvack.org
> > > > >    S:   Maintained
> > > > >    W:   https://www.linux-mm.org
> > > > >    T:   git git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
> > > > > +F:     mm/madvise.c
> > > > > +F:     mm/mlock.c
> > > > > +F:     mm/mmap.c
> > > > > +F:     mm/mprotect.c
> > > > > +F:     mm/mremap.c
> > > > > +F:     mm/mseal.c
> > > > > +F:     mm/msync.c
> > > >
> > > > Not sure about mprotect.c, mlock.c and madvise.c, though. I'd claim that
> > > > the real "magic" they perform is in page table handling and not
> > > > primarily VMA handling (yes, both do VMA changes, but they are the
> > > > "easy" part ;) ).
> > >
> > > I'd think that moving vma files into MEMORY MAPPING (and not the other way)
> > > would result in a better overal name, that would be a better fit for the
> > > newly added files too?
> >
> > Maybe. I think vma.c should likely have a different set of maintainers than
> > madvise.c and mprotect.c. (again, the magic is in page table modifications)
>
> The bulk of the logic in mremap.c is related to page tables so by this
> logic then, that is out too, right?

FWIW, I think technically you can have multiple entries in MAINTAINERS
that cover the same file, maybe that would make sense for files that
belong to multiple parts of the kernel? Or maybe I'm making things too
complicated and it'd be simpler to have some kind of more generic
"core MM for userspace mappings" entry or such.

Re: [PATCH] MAINTAINERS: group all VMA-related files into the VMA section

[David Hildenbrand]

On 09.12.24 15:38, Jann Horn wrote:
> On Mon, Dec 9, 2024 at 3:11 PM Lorenzo Stoakes
> <lorenzo.stoakes@oracle.com> wrote:
>> On Mon, Dec 09, 2024 at 03:00:08PM +0100, David Hildenbrand wrote:
>>> On 09.12.24 14:25, Vlastimil Babka wrote:
>>>> On 12/9/24 10:16, David Hildenbrand wrote:
>>>>> On 06.12.24 20:16, Lorenzo Stoakes wrote:
>>>>>> There are a number of means of interacting with VMA operations within mm,
>>>>>> and we have on occasion not been made aware of impactful changes due to
>>>>>> these sitting in different files, most recently in [0].
>>>>>>
>>>>>> Correct this by bringing all VMA operations under the same section in
>>>>>> MAINTAINERS. Additionally take the opportunity to combine MEMORY MAPPING
>>>>>> with VMA as there needn't be two entries as they amount to the same thing.
>>>>>>
>>>>>> [0]:https://lore.kernel.org/linux-mm/CAG48ez0siYGB8GP5+Szgj2ovBZAkL6Zi4n6GUAjzzjFV9LTkRQ@mail.gmail.com/
>>>>>>
>>>>>> Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
>>>>>> ---
>>>>>>     MAINTAINERS | 19 +++++++------------
>>>>>>     1 file changed, 7 insertions(+), 12 deletions(-)
>>>>>>
>>>>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>>>>> index 1e930c7a58b1..95db20c26f5f 100644
>>>>>> --- a/MAINTAINERS
>>>>>> +++ b/MAINTAINERS
>>>>>> @@ -15060,18 +15060,6 @@ F:     tools/mm/
>>>>>>     F:   tools/testing/selftests/mm/
>>>>>>     N:   include/linux/page[-_]*
>>>>>>
>>>>>> -MEMORY MAPPING
>>>>>> -M:     Andrew Morton <akpm@linux-foundation.org>
>>>>>> -M:     Liam R. Howlett <Liam.Howlett@oracle.com>
>>>>>> -M:     Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
>>>>>> -R:     Vlastimil Babka <vbabka@suse.cz>
>>>>>> -R:     Jann Horn <jannh@google.com>
>>>>>> -L:     linux-mm@kvack.org
>>>>>> -S:     Maintained
>>>>>> -W:     http://www.linux-mm.org
>>>>>> -T:     git git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
>>>>>> -F:     mm/mmap.c
>>>>>> -
>>>>>>     MEMORY TECHNOLOGY DEVICES (MTD)
>>>>>>     M:   Miquel Raynal <miquel.raynal@bootlin.com>
>>>>>>     M:   Richard Weinberger <richard@nod.at>
>>>>>> @@ -25028,6 +25016,13 @@ L:     linux-mm@kvack.org
>>>>>>     S:   Maintained
>>>>>>     W:   https://www.linux-mm.org
>>>>>>     T:   git git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
>>>>>> +F:     mm/madvise.c
>>>>>> +F:     mm/mlock.c
>>>>>> +F:     mm/mmap.c
>>>>>> +F:     mm/mprotect.c
>>>>>> +F:     mm/mremap.c
>>>>>> +F:     mm/mseal.c
>>>>>> +F:     mm/msync.c
>>>>>
>>>>> Not sure about mprotect.c, mlock.c and madvise.c, though. I'd claim that
>>>>> the real "magic" they perform is in page table handling and not
>>>>> primarily VMA handling (yes, both do VMA changes, but they are the
>>>>> "easy" part ;) ).
>>>>
>>>> I'd think that moving vma files into MEMORY MAPPING (and not the other way)
>>>> would result in a better overal name, that would be a better fit for the
>>>> newly added files too?
>>>
>>> Maybe. I think vma.c should likely have a different set of maintainers than
>>> madvise.c and mprotect.c. (again, the magic is in page table modifications)
>>
>> The bulk of the logic in mremap.c is related to page tables so by this
>> logic then, that is out too, right?
> 
> FWIW, I think technically you can have multiple entries in MAINTAINERS
> that cover the same file, maybe that would make sense for files that
> belong to multiple parts of the kernel?

I was asking myself the same question. But then ...

> Or maybe I'm making things too
> complicated and it'd be simpler to have some kind of more generic
> "core MM for userspace mappings" entry or such.

... this sound cleaner than having files as part of every section with 
which they partially overlap.

-- 
Cheers,

David / dhildenb

Re: [PATCH] MAINTAINERS: group all VMA-related files into the VMA section

[Lorenzo Stoakes]

On Mon, Dec 09, 2024 at 03:38:28PM +0100, Jann Horn wrote:
> On Mon, Dec 9, 2024 at 3:11 PM Lorenzo Stoakes
> <lorenzo.stoakes@oracle.com> wrote:
> > On Mon, Dec 09, 2024 at 03:00:08PM +0100, David Hildenbrand wrote:
> > > On 09.12.24 14:25, Vlastimil Babka wrote:
> > > > On 12/9/24 10:16, David Hildenbrand wrote:
> > > > > On 06.12.24 20:16, Lorenzo Stoakes wrote:
> > > > > > There are a number of means of interacting with VMA operations within mm,
> > > > > > and we have on occasion not been made aware of impactful changes due to
> > > > > > these sitting in different files, most recently in [0].
> > > > > >
> > > > > > Correct this by bringing all VMA operations under the same section in
> > > > > > MAINTAINERS. Additionally take the opportunity to combine MEMORY MAPPING
> > > > > > with VMA as there needn't be two entries as they amount to the same thing.
> > > > > >
> > > > > > [0]:https://lore.kernel.org/linux-mm/CAG48ez0siYGB8GP5+Szgj2ovBZAkL6Zi4n6GUAjzzjFV9LTkRQ@mail.gmail.com/
> > > > > >
> > > > > > Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> > > > > > ---
> > > > > >    MAINTAINERS | 19 +++++++------------
> > > > > >    1 file changed, 7 insertions(+), 12 deletions(-)
> > > > > >
> > > > > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > > > > index 1e930c7a58b1..95db20c26f5f 100644
> > > > > > --- a/MAINTAINERS
> > > > > > +++ b/MAINTAINERS
> > > > > > @@ -15060,18 +15060,6 @@ F:     tools/mm/
> > > > > >    F:   tools/testing/selftests/mm/
> > > > > >    N:   include/linux/page[-_]*
> > > > > >
> > > > > > -MEMORY MAPPING
> > > > > > -M:     Andrew Morton <akpm@linux-foundation.org>
> > > > > > -M:     Liam R. Howlett <Liam.Howlett@oracle.com>
> > > > > > -M:     Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> > > > > > -R:     Vlastimil Babka <vbabka@suse.cz>
> > > > > > -R:     Jann Horn <jannh@google.com>
> > > > > > -L:     linux-mm@kvack.org
> > > > > > -S:     Maintained
> > > > > > -W:     http://www.linux-mm.org
> > > > > > -T:     git git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
> > > > > > -F:     mm/mmap.c
> > > > > > -
> > > > > >    MEMORY TECHNOLOGY DEVICES (MTD)
> > > > > >    M:   Miquel Raynal <miquel.raynal@bootlin.com>
> > > > > >    M:   Richard Weinberger <richard@nod.at>
> > > > > > @@ -25028,6 +25016,13 @@ L:     linux-mm@kvack.org
> > > > > >    S:   Maintained
> > > > > >    W:   https://www.linux-mm.org
> > > > > >    T:   git git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
> > > > > > +F:     mm/madvise.c
> > > > > > +F:     mm/mlock.c
> > > > > > +F:     mm/mmap.c
> > > > > > +F:     mm/mprotect.c
> > > > > > +F:     mm/mremap.c
> > > > > > +F:     mm/mseal.c
> > > > > > +F:     mm/msync.c
> > > > >
> > > > > Not sure about mprotect.c, mlock.c and madvise.c, though. I'd claim that
> > > > > the real "magic" they perform is in page table handling and not
> > > > > primarily VMA handling (yes, both do VMA changes, but they are the
> > > > > "easy" part ;) ).
> > > >
> > > > I'd think that moving vma files into MEMORY MAPPING (and not the other way)
> > > > would result in a better overal name, that would be a better fit for the
> > > > newly added files too?
> > >
> > > Maybe. I think vma.c should likely have a different set of maintainers than
> > > madvise.c and mprotect.c. (again, the magic is in page table modifications)
> >
> > The bulk of the logic in mremap.c is related to page tables so by this
> > logic then, that is out too, right?
>
> FWIW, I think technically you can have multiple entries in MAINTAINERS
> that cover the same file, maybe that would make sense for files that
> belong to multiple parts of the kernel? Or maybe I'm making things too
> complicated and it'd be simpler to have some kind of more generic
> "core MM for userspace mappings" entry or such.

I think it's faintly ludicrous to separate things on the basis of whether
they explicitly manipulate one part of the kernel or another, and it'd be
an odd thing to be trusted with one 'portion' of a file based on some fuzzy
sense of which bits are 'magic' and therefore out of bounds and which are
presumably not...

I don't think it makes sense to separate the 'VMA' bits from these files
other than perhaps refactoring things a bit (badly needed actually).

The page table manipulation very sorely needs improvement and
de-duplication, I am somewhat taken aback that it is thought that I might
not be able to do so given I had already paid serious consideration to
doing work in this area based on guard page work (not sure if that work
would now be welcome?)

To me I politely disagree with the assessment made here, but if a senior
member of the kernel objects of course I'll withdraw it.

But yeah I don't think that's workable. We will just have to hope that we
notice mremap changes that might be problematic going forward, I might
therefore update my lei settings accordingly...

Re: [PATCH] MAINTAINERS: group all VMA-related files into the VMA section

[David Hildenbrand]

On 09.12.24 15:45, Lorenzo Stoakes wrote:
> On Mon, Dec 09, 2024 at 03:38:28PM +0100, Jann Horn wrote:
>> On Mon, Dec 9, 2024 at 3:11 PM Lorenzo Stoakes
>> <lorenzo.stoakes@oracle.com> wrote:
>>> On Mon, Dec 09, 2024 at 03:00:08PM +0100, David Hildenbrand wrote:
>>>> On 09.12.24 14:25, Vlastimil Babka wrote:
>>>>> On 12/9/24 10:16, David Hildenbrand wrote:
>>>>>> On 06.12.24 20:16, Lorenzo Stoakes wrote:
>>>>>>> There are a number of means of interacting with VMA operations within mm,
>>>>>>> and we have on occasion not been made aware of impactful changes due to
>>>>>>> these sitting in different files, most recently in [0].
>>>>>>>
>>>>>>> Correct this by bringing all VMA operations under the same section in
>>>>>>> MAINTAINERS. Additionally take the opportunity to combine MEMORY MAPPING
>>>>>>> with VMA as there needn't be two entries as they amount to the same thing.
>>>>>>>
>>>>>>> [0]:https://lore.kernel.org/linux-mm/CAG48ez0siYGB8GP5+Szgj2ovBZAkL6Zi4n6GUAjzzjFV9LTkRQ@mail.gmail.com/
>>>>>>>
>>>>>>> Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
>>>>>>> ---
>>>>>>>     MAINTAINERS | 19 +++++++------------
>>>>>>>     1 file changed, 7 insertions(+), 12 deletions(-)
>>>>>>>
>>>>>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>>>>>> index 1e930c7a58b1..95db20c26f5f 100644
>>>>>>> --- a/MAINTAINERS
>>>>>>> +++ b/MAINTAINERS
>>>>>>> @@ -15060,18 +15060,6 @@ F:     tools/mm/
>>>>>>>     F:   tools/testing/selftests/mm/
>>>>>>>     N:   include/linux/page[-_]*
>>>>>>>
>>>>>>> -MEMORY MAPPING
>>>>>>> -M:     Andrew Morton <akpm@linux-foundation.org>
>>>>>>> -M:     Liam R. Howlett <Liam.Howlett@oracle.com>
>>>>>>> -M:     Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
>>>>>>> -R:     Vlastimil Babka <vbabka@suse.cz>
>>>>>>> -R:     Jann Horn <jannh@google.com>
>>>>>>> -L:     linux-mm@kvack.org
>>>>>>> -S:     Maintained
>>>>>>> -W:     http://www.linux-mm.org
>>>>>>> -T:     git git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
>>>>>>> -F:     mm/mmap.c
>>>>>>> -
>>>>>>>     MEMORY TECHNOLOGY DEVICES (MTD)
>>>>>>>     M:   Miquel Raynal <miquel.raynal@bootlin.com>
>>>>>>>     M:   Richard Weinberger <richard@nod.at>
>>>>>>> @@ -25028,6 +25016,13 @@ L:     linux-mm@kvack.org
>>>>>>>     S:   Maintained
>>>>>>>     W:   https://www.linux-mm.org
>>>>>>>     T:   git git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
>>>>>>> +F:     mm/madvise.c
>>>>>>> +F:     mm/mlock.c
>>>>>>> +F:     mm/mmap.c
>>>>>>> +F:     mm/mprotect.c
>>>>>>> +F:     mm/mremap.c
>>>>>>> +F:     mm/mseal.c
>>>>>>> +F:     mm/msync.c
>>>>>>
>>>>>> Not sure about mprotect.c, mlock.c and madvise.c, though. I'd claim that
>>>>>> the real "magic" they perform is in page table handling and not
>>>>>> primarily VMA handling (yes, both do VMA changes, but they are the
>>>>>> "easy" part ;) ).
>>>>>
>>>>> I'd think that moving vma files into MEMORY MAPPING (and not the other way)
>>>>> would result in a better overal name, that would be a better fit for the
>>>>> newly added files too?
>>>>
>>>> Maybe. I think vma.c should likely have a different set of maintainers than
>>>> madvise.c and mprotect.c. (again, the magic is in page table modifications)
>>>
>>> The bulk of the logic in mremap.c is related to page tables so by this
>>> logic then, that is out too, right?
>>
>> FWIW, I think technically you can have multiple entries in MAINTAINERS
>> that cover the same file, maybe that would make sense for files that
>> belong to multiple parts of the kernel? Or maybe I'm making things too
>> complicated and it'd be simpler to have some kind of more generic
>> "core MM for userspace mappings" entry or such.
> 
> I think it's faintly ludicrous to separate things on the basis of whether
> they explicitly manipulate one part of the kernel or another, and it'd be
> an odd thing to be trusted with one 'portion' of a file based on some fuzzy
> sense of which bits are 'magic' and therefore out of bounds and which are
> presumably not...
> 
> I don't think it makes sense to separate the 'VMA' bits from these files
> other than perhaps refactoring things a bit (badly needed actually).
> 
> The page table manipulation very sorely needs improvement and
> de-duplication, I am somewhat taken aback that it is thought that I might
> not be able to do so given I had already paid serious consideration to
> doing work in this area based on guard page work (not sure if that work
> would now be welcome?)
> 
> To me I politely disagree with the assessment made here, but if a senior
> member of the kernel objects of course I'll withdraw it.
> 
> But yeah I don't think that's workable. We will just have to hope that we
> notice mremap changes that might be problematic going forward, I might
> therefore update my lei settings accordingly...
> 

I have the feeling you take this personally; please don't.

Maybe my other mail with "VMA users" vs. "basic VMA functionality" makes 
it clearer what I mean.

For example, mm/userfaultfd.c also performs VMA modifications. 
kernel/fork.c does a bunch of that. I neither think these should go 
under VMA nor that it should be split up.

Maybe there is a better way to split up things or rework the code; 
likely we'd want this code that works on VMAs to have a clean interface 
with the core vma logic in vma.c, if the current way of handling it is a 
problem for you.

-- 
Cheers,

David / dhildenb

Re: [PATCH] MAINTAINERS: group all VMA-related files into the VMA section

[Lorenzo Stoakes]

On Mon, Dec 09, 2024 at 04:03:59PM +0100, David Hildenbrand wrote:
> On 09.12.24 15:45, Lorenzo Stoakes wrote:
> > On Mon, Dec 09, 2024 at 03:38:28PM +0100, Jann Horn wrote:
> > > On Mon, Dec 9, 2024 at 3:11 PM Lorenzo Stoakes
> > > <lorenzo.stoakes@oracle.com> wrote:
> > > > On Mon, Dec 09, 2024 at 03:00:08PM +0100, David Hildenbrand wrote:
> > > > > On 09.12.24 14:25, Vlastimil Babka wrote:
> > > > > > On 12/9/24 10:16, David Hildenbrand wrote:
> > > > > > > On 06.12.24 20:16, Lorenzo Stoakes wrote:
> > > > > > > > There are a number of means of interacting with VMA operations within mm,
> > > > > > > > and we have on occasion not been made aware of impactful changes due to
> > > > > > > > these sitting in different files, most recently in [0].
> > > > > > > >
> > > > > > > > Correct this by bringing all VMA operations under the same section in
> > > > > > > > MAINTAINERS. Additionally take the opportunity to combine MEMORY MAPPING
> > > > > > > > with VMA as there needn't be two entries as they amount to the same thing.
> > > > > > > >
> > > > > > > > [0]:https://lore.kernel.org/linux-mm/CAG48ez0siYGB8GP5+Szgj2ovBZAkL6Zi4n6GUAjzzjFV9LTkRQ@mail.gmail.com/
> > > > > > > >
> > > > > > > > Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> > > > > > > > ---
> > > > > > > >     MAINTAINERS | 19 +++++++------------
> > > > > > > >     1 file changed, 7 insertions(+), 12 deletions(-)
> > > > > > > >
> > > > > > > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > > > > > > index 1e930c7a58b1..95db20c26f5f 100644
> > > > > > > > --- a/MAINTAINERS
> > > > > > > > +++ b/MAINTAINERS
> > > > > > > > @@ -15060,18 +15060,6 @@ F:     tools/mm/
> > > > > > > >     F:   tools/testing/selftests/mm/
> > > > > > > >     N:   include/linux/page[-_]*
> > > > > > > >
> > > > > > > > -MEMORY MAPPING
> > > > > > > > -M:     Andrew Morton <akpm@linux-foundation.org>
> > > > > > > > -M:     Liam R. Howlett <Liam.Howlett@oracle.com>
> > > > > > > > -M:     Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> > > > > > > > -R:     Vlastimil Babka <vbabka@suse.cz>
> > > > > > > > -R:     Jann Horn <jannh@google.com>
> > > > > > > > -L:     linux-mm@kvack.org
> > > > > > > > -S:     Maintained
> > > > > > > > -W:     http://www.linux-mm.org
> > > > > > > > -T:     git git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
> > > > > > > > -F:     mm/mmap.c
> > > > > > > > -
> > > > > > > >     MEMORY TECHNOLOGY DEVICES (MTD)
> > > > > > > >     M:   Miquel Raynal <miquel.raynal@bootlin.com>
> > > > > > > >     M:   Richard Weinberger <richard@nod.at>
> > > > > > > > @@ -25028,6 +25016,13 @@ L:     linux-mm@kvack.org
> > > > > > > >     S:   Maintained
> > > > > > > >     W:   https://www.linux-mm.org
> > > > > > > >     T:   git git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
> > > > > > > > +F:     mm/madvise.c
> > > > > > > > +F:     mm/mlock.c
> > > > > > > > +F:     mm/mmap.c
> > > > > > > > +F:     mm/mprotect.c
> > > > > > > > +F:     mm/mremap.c
> > > > > > > > +F:     mm/mseal.c
> > > > > > > > +F:     mm/msync.c
> > > > > > >
> > > > > > > Not sure about mprotect.c, mlock.c and madvise.c, though. I'd claim that
> > > > > > > the real "magic" they perform is in page table handling and not
> > > > > > > primarily VMA handling (yes, both do VMA changes, but they are the
> > > > > > > "easy" part ;) ).
> > > > > >
> > > > > > I'd think that moving vma files into MEMORY MAPPING (and not the other way)
> > > > > > would result in a better overal name, that would be a better fit for the
> > > > > > newly added files too?
> > > > >
> > > > > Maybe. I think vma.c should likely have a different set of maintainers than
> > > > > madvise.c and mprotect.c. (again, the magic is in page table modifications)
> > > >
> > > > The bulk of the logic in mremap.c is related to page tables so by this
> > > > logic then, that is out too, right?
> > >
> > > FWIW, I think technically you can have multiple entries in MAINTAINERS
> > > that cover the same file, maybe that would make sense for files that
> > > belong to multiple parts of the kernel? Or maybe I'm making things too
> > > complicated and it'd be simpler to have some kind of more generic
> > > "core MM for userspace mappings" entry or such.
> >
> > I think it's faintly ludicrous to separate things on the basis of whether
> > they explicitly manipulate one part of the kernel or another, and it'd be
> > an odd thing to be trusted with one 'portion' of a file based on some fuzzy
> > sense of which bits are 'magic' and therefore out of bounds and which are
> > presumably not...
> >
> > I don't think it makes sense to separate the 'VMA' bits from these files
> > other than perhaps refactoring things a bit (badly needed actually).
> >
> > The page table manipulation very sorely needs improvement and
> > de-duplication, I am somewhat taken aback that it is thought that I might
> > not be able to do so given I had already paid serious consideration to
> > doing work in this area based on guard page work (not sure if that work
> > would now be welcome?)
> >
> > To me I politely disagree with the assessment made here, but if a senior
> > member of the kernel objects of course I'll withdraw it.
> >
> > But yeah I don't think that's workable. We will just have to hope that we
> > notice mremap changes that might be problematic going forward, I might
> > therefore update my lei settings accordingly...
> >
>
> I have the feeling you take this personally; please don't.

Sure sorry it's text being a bad medium and this sort of thing _inevitably_
being a fraught subject :)

I have a great deal of respect for you so my imagining that you might think
I couldn't do things in this area was slightly shocking, but I suspect this
is, in fact, on reflection, not at all what you meant.

So sorry, I don't intend for this to be anything other than a functional
converastion to determine how best for us to manage workload and avoid
issues in future.

If you see my other mail with suggested ways forward, hopefully one of
those will help ensure appropriate separation and distribution of
workload/responsibility (am of course also open to whatever you might
suggest!)

>
> Maybe my other mail with "VMA users" vs. "basic VMA functionality" makes it
> clearer what I mean.
>
> For example, mm/userfaultfd.c also performs VMA modifications. kernel/fork.c
> does a bunch of that. I neither think these should go under VMA nor that it
> should be split up.

Yeah and I _hate_ that. I mean talk to me about fs/userfaultfd.c, but maybe
only after a few pints :) Or bits of mm that live in fs, but vma-related
and not...

But these are areas to fix.

>
> Maybe there is a better way to split up things or rework the code; likely
> we'd want this code that works on VMAs to have a clean interface with the
> core vma logic in vma.c, if the current way of handling it is a problem for
> you.

I think we probably need a compromise for the time being, and as I was
saying to Jann I don't think it makes sense really to separate the VMA and
page table bits from these files _except_ when we finally have a shared
page table interface that we've spoken about before and perhaps we will
collaborate on in future :)

The key point is so we avoid stepping on landmines when we discover
something was merged in file X which we weren't cc'd on that breaks core
feature Y we have been working on in the VMA part of the code.

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

Cheers!

Re: [PATCH] MAINTAINERS: group all VMA-related files into the VMA section

[David Hildenbrand]

>>> To me I politely disagree with the assessment made here, but if a senior
>>> member of the kernel objects of course I'll withdraw it.
>>>
>>> But yeah I don't think that's workable. We will just have to hope that we
>>> notice mremap changes that might be problematic going forward, I might
>>> therefore update my lei settings accordingly...
>>>
>>
>> I have the feeling you take this personally; please don't.
> 

Hi Lorenzo,

> Sure sorry it's text being a bad medium and this sort of thing _inevitably_
> being a fraught subject :)
> 
> I have a great deal of respect for you so my imagining that you might think
> I couldn't do things in this area was slightly shocking, but I suspect this
> is, in fact, on reflection, not at all what you meant.

Not at all. The key problem here is that we have MM systemcalls, that 
will involve multiple complex things. And the VMA handling is only one 
of these things IMHO.

For example, I suck at VMA handling and wouldn't ever put my name on the 
VMA section (vma.c). In contrast, I might consider myself a bit familiar 
with mprotect() and madvise() handling besides the struct vm_struct 
modifications.

And that was my point: the majority of the complex code in these files 
is not the VMA handling (as in vma.c). Whereby in vma.c and mmap.c it 
absolutely is.

VMA maintainers reviewing (and people expecting them to ack/nack!) 
madvise() changes like MADV_COLD that really do nothing relevant with 
VMAs is not particularly helpful.

I am also not a big fan of having too many maintainers listed, because a 
file ends up falling into multiple sections -- how should a submitter in 
the end know which ack actually counts, and which are required etc.

Maybe we can find a better way to structure things (either in the 
MAINTAINER file or code-wise).

Or maybe I am just wrong.

(many of our files are currently structured around syscalls, and then as 
explained, there is mm/huge_memory.c where we dumped some other parts)

> 
> So sorry, I don't intend for this to be anything other than a functional
> converastion to determine how best for us to manage workload and avoid
> issues in future.

Sorry for misleading you.

> 
> If you see my other mail with suggested ways forward, hopefully one of
> those will help ensure appropriate separation and distribution of
> workload/responsibility (am of course also open to whatever you might
> suggest!)
> 

Yes, I'll try taking a look tomorrow.

>>
>> Maybe my other mail with "VMA users" vs. "basic VMA functionality" makes it
>> clearer what I mean.
>>
>> For example, mm/userfaultfd.c also performs VMA modifications. kernel/fork.c
>> does a bunch of that. I neither think these should go under VMA nor that it
>> should be split up.
> 
> Yeah and I _hate_ that. I mean talk to me about fs/userfaultfd.c, but maybe
> only after a few pints :) Or bits of mm that live in fs, but vma-related
> and not...
> 

:)

> But these are areas to fix.
> 
>>
>> Maybe there is a better way to split up things or rework the code; likely
>> we'd want this code that works on VMAs to have a clean interface with the
>> core vma logic in vma.c, if the current way of handling it is a problem for
>> you.
> 
> I think we probably need a compromise for the time being, and as I was
> saying to Jann I don't think it makes sense really to separate the VMA and
> page table bits from these files _except_ when we finally have a shared
> page table interface that we've spoken about before and perhaps we will
> collaborate on in future :)

In fork.c we split out the page table bits (into mm/memory.c), but left 
the VMA bits in there ... :)

> 
> The key point is so we avoid stepping on landmines when we discover
> something was merged in file X which we weren't cc'd on that breaks core
> feature Y we have been working on in the VMA part of the code.

In my experience, tracking files is not particularly helpful. People 
will still not CC you, or do nasty stuff in files you are not tracking.

What I do is (well, besides screening most of linux-mm), is have a list 
of magic names/keywords, and tag the mails.

This way, when someone uses the magic "COW" keyword, for example, or 
calls mapcount functions, I get them put into a separate folder where I 
can just briefly sanity check them.

-- 
Cheers,

David / dhildenb

Re: [PATCH] MAINTAINERS: group all VMA-related files into the VMA section

[Lorenzo Stoakes]

David,

To avoid an infinite back-and-forth...

I think you're stuck on the idea that we are sat in a VMA 'vacuum', perhaps
because I put so much effort into separating out the VMA logic, for the
purpose of being able to userland test it, it's almost given the wrong
impression (I should perhaps have put less effort into this? :)

But we have for quite some time now de facto been expected to maintain all
aspects of mapping, remapping, etc. INCLUDING page table considerations.

We are more or less de facto maintaining all that (modulo madvise.c - I
grant you this is the odd one out).

So you can imagine it's a bit frustrating, when that's the case, to be told
in effect - no this isn't for you - right?

For instance, as I said before, we have spent large parts of the 6.12 cycle
dealing with practical concerns specifically around page table
manipulation.

Maintainership is more than setting up lei rules, obviously. One can set
lei rules for anything. It means that our say has more impact, and it also
means that we are (co-)responsible for the listed files, and that's acked
rather than disregarded.

So, again, I politely disagree with your assessment re: page tables.

I think their manipulation under circumstances where you
map/remap/mprotect/mlock is -inseparable- from memory mapping/VMA
logic. Otherwise, what's the point right?

My suggestion is that:

a. we put everything in MEMORY MAPPING
b. We drop mm/madvise.c from the list

I'm more than happy to hear your suggestions however. But whatever your
view won't change the de facto situation, it only makes it more difficult
for us to do what is already expected of us...

But I'd also like - given your strong push back - to remind you - this is
not a coup :) we are simply co-maintainers with Andrew, we don't send a PR
to Linus, the <2.5% of mm code this change represents would not be our sole
domain...

Thanks, Lorenzo

Re: [PATCH] MAINTAINERS: group all VMA-related files into the VMA section

[David Hildenbrand]

On 10.12.24 10:51, Lorenzo Stoakes wrote:
> David,

Hi Lorenzo,

sorry for the late reply.

> 
> To avoid an infinite back-and-forth...
> 
> I think you're stuck on the idea that we are sat in a VMA 'vacuum', perhaps
> because I put so much effort into separating out the VMA logic, for the
> purpose of being able to userland test it, it's almost given the wrong
> impression (I should perhaps have put less effort into this? :)

Yes, that nicely summarizes it.

In particular, because the patch description says "group all VMA-related 
files", and I am having a hard time to even identify *what* a 
"VMA-related" file is, really.

For example, why not mempolicy.c->mbind()? Not that I would suggest 
that, because the file is filled with non-vma-related stuff.

See below of what might make sense to me.

> 
> But we have for quite some time now de facto been expected to maintain all
> aspects of mapping, remapping, etc. INCLUDING page table considerations.
 > > We are more or less de facto maintaining all that (modulo madvise.c - I
> grant you this is the odd one out).
> 
> So you can imagine it's a bit frustrating, when that's the case, to be told
> in effect - no this isn't for you - right?

It isn't "VMA" group for me, independent of "who" is currently listed 
there. And maybe we now agree on that, maybe not.

Talking about things that are frustrating: being called a "senior member 
of the kernel". :)

 > > For instance, as I said before, we have spent large parts of the 
6.12 cycle
> dealing with practical concerns specifically around page table
> manipulation.
 > > Maintainership is more than setting up lei rules, obviously. One 
can set
> lei rules for anything. It means that our say has more impact, and it also
> means that we are (co-)responsible for the listed files, and that's acked
> rather than disregarded.
 > > So, again, I politely disagree with your assessment re: page tables.
 > > I think their manipulation under circumstances where you
> map/remap/mprotect/mlock is -inseparable- from memory mapping/VMA
> logic. Otherwise, what's the point right?

We'll likely have to agree to disagree. :) But again, my main point is 
that the VMA group is misleading.

As a side note, I believe the code can be structures accordingly (call 
that separating it). mmap/munmap handling is the prime example right now 
for me.

For example, I really enjoy how:

munmap() (mmap.c) routes to __vm_munmap (vma.c) makes use of abstracted 
page table logic like free_pgtables() and unmap_vmas() (memory.c).

This way it is clear what the rather high-level MM syscall 
implementation is (mmap.c), what the VMA handling is (vma.c) and what 
the abstracted page table handling logic is (memory.c). I don't think 
that page table handling code should be moved to either mmap.c or vma.c.


I would enjoy if we would handle e.g., mprotect.c in a similar way, such 
that the helper like change_protection() -- again, used by completely 
mprotect()-unrelated code outside of mprotect.c -- would not reside in 
mprotect.c. But that code just evolved naturally after we kept reusing 
change_protection() outside of the file.

Regarding mremap logic there once was a discussion about merging it with 
the uffdio_move logic, but not sure if that really makes sense.

> 
> My suggestion is that:
> 
> a. we put everything in MEMORY MAPPING
> b. We drop mm/madvise.c from the list

Sounds better, although I am still fuzzy on what is supposed to be in 
there and what not. See my mbind() example above ..

I see how mmap/munmap/mprotect/mremap fall into the same category of MM 
syscalls that mainly affect the /proc/self/maps output (in contrast to a 
bunch of others). Which make sense to me to group in such a way.

mseal.c and mlock.c likely as well.

msync.c is a simple VMA walker ... not sure if it belongs in there, but 
who am I to tell.

-- 
Cheers,

David / dhildenb

Re: [PATCH] MAINTAINERS: group all VMA-related files into the VMA section

[Lorenzo Stoakes]

On Tue, Dec 10, 2024 at 05:59:08PM +0100, David Hildenbrand wrote:
> On 10.12.24 10:51, Lorenzo Stoakes wrote:
> > David,
>
> Hi Lorenzo,
>
> sorry for the late reply.
>

No worries, we're all busy. I'm off on holiday after the end of this week
also so if I disappear after then this is why :)

> >
> > To avoid an infinite back-and-forth...
> >
> > I think you're stuck on the idea that we are sat in a VMA 'vacuum', perhaps
> > because I put so much effort into separating out the VMA logic, for the
> > purpose of being able to userland test it, it's almost given the wrong
> > impression (I should perhaps have put less effort into this? :)
>
> Yes, that nicely summarizes it.
>
> In particular, because the patch description says "group all VMA-related
> files", and I am having a hard time to even identify *what* a "VMA-related"
> file is, really.

Yeah I definitely misgrouped this.

>
> For example, why not mempolicy.c->mbind()? Not that I would suggest that,
> because the file is filled with non-vma-related stuff.

Right, we have a lot of this 'mixing'.

>
> See below of what might make sense to me.
>
> >
> > But we have for quite some time now de facto been expected to maintain all
> > aspects of mapping, remapping, etc. INCLUDING page table considerations.
> > > We are more or less de facto maintaining all that (modulo madvise.c - I
> > grant you this is the odd one out).
> >
> > So you can imagine it's a bit frustrating, when that's the case, to be told
> > in effect - no this isn't for you - right?
>
> It isn't "VMA" group for me, independent of "who" is currently listed there.
> And maybe we now agree on that, maybe not.

Yes absolutely agreed.

>
> Talking about things that are frustrating: being called a "senior member of
> the kernel". :)

Haha well, I still see myself as a raw "junior" if that helps ;) or maybe
makes it worse? I don't know :P

I am at least certainly senior in the sense of wrinkles...

>
> > > For instance, as I said before, we have spent large parts of the 6.12
> cycle
> > dealing with practical concerns specifically around page table
> > manipulation.
> > > Maintainership is more than setting up lei rules, obviously. One can set
> > lei rules for anything. It means that our say has more impact, and it also
> > means that we are (co-)responsible for the listed files, and that's acked
> > rather than disregarded.
> > > So, again, I politely disagree with your assessment re: page tables.
> > > I think their manipulation under circumstances where you
> > map/remap/mprotect/mlock is -inseparable- from memory mapping/VMA
> > logic. Otherwise, what's the point right?
>
> We'll likely have to agree to disagree. :) But again, my main point is that
> the VMA group is misleading.

Yes and civil disagreement is fine! But yes agreed on structure.

>
> As a side note, I believe the code can be structures accordingly (call that
> separating it). mmap/munmap handling is the prime example right now for me.
>
> For example, I really enjoy how:
>
> munmap() (mmap.c) routes to __vm_munmap (vma.c) makes use of abstracted page
> table logic like free_pgtables() and unmap_vmas() (memory.c).
>
> This way it is clear what the rather high-level MM syscall implementation is
> (mmap.c), what the VMA handling is (vma.c) and what the abstracted page
> table handling logic is (memory.c). I don't think that page table handling
> code should be moved to either mmap.c or vma.c.
>
>
> I would enjoy if we would handle e.g., mprotect.c in a similar way, such
> that the helper like change_protection() -- again, used by completely
> mprotect()-unrelated code outside of mprotect.c -- would not reside in
> mprotect.c. But that code just evolved naturally after we kept reusing
> change_protection() outside of the file.

Yeah, so I think another step forward is to start actually moving some of
the more obviously general stuff like this to other files, I will look at
this in the new year.

>
> Regarding mremap logic there once was a discussion about merging it with the
> uffdio_move logic, but not sure if that really makes sense.
>
> >
> > My suggestion is that:
> >
> > a. we put everything in MEMORY MAPPING
> > b. We drop mm/madvise.c from the list
>
> Sounds better, although I am still fuzzy on what is supposed to be in there
> and what not. See my mbind() example above ..
>
> I see how mmap/munmap/mprotect/mremap fall into the same category of MM
> syscalls that mainly affect the /proc/self/maps output (in contrast to a
> bunch of others). Which make sense to me to group in such a way.
>
> mseal.c and mlock.c likely as well.

Yes this is why I think these belong together, and under 'memory mapping'
makes sense if one sees it from this point of view.

>
> msync.c is a simple VMA walker ... not sure if it belongs in there, but who
> am I to tell.

Will drop, was a tenuous one, just seemed like a tiny vaguely VMA-adjacent
thing but agreed it's a bit out of place.

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

OK so I think we're (probably?) now agreed, I will submit a patch shortly
that:

a. puts everything in MEMORY MAPPING
b. Drops mm/madvise.c, mm/msync.c from the list
c. I commit to moving things out of the various files that truly belongs
   elsewhere

I mean there's stuff that's weirdly used for page table moving in mremap.c
that should probably live in memory.c as well for instance.

Re: [PATCH] MAINTAINERS: group all VMA-related files into the VMA section

[David Hildenbrand]

>>
> 
> OK so I think we're (probably?) now agreed, I will submit a patch shortly
> that:
> 
> a. puts everything in MEMORY MAPPING
> b. Drops mm/madvise.c, mm/msync.c from the list
> c. I commit to moving things out of the various files that truly belongs
>     elsewhere
 > > I mean there's stuff that's weirdly used for page table moving in 
mremap.c
> that should probably live in memory.c as well for instance.

Yes, and hopefully we can clearly frame what MEMORY MAPPING is supposed 
to contain. I tried to tackle it with "/proc/self/maps output", but 
that's probably not the complete story.

For example, maybe mbind() should, for example, at some point be 
separated out into into mbind.c (making use of mempolicy.c 
functionality?) and covered there as well? I really don't know, maybe 
it's not one of the mmap/munmap/mprotect/mremap/mseal/mlock gang after all.

-- 
Cheers,

David / dhildenb

Re: [PATCH] MAINTAINERS: group all VMA-related files into the VMA section

[Lorenzo Stoakes]

On Wed, Dec 11, 2024 at 11:27:39AM +0100, David Hildenbrand wrote:
> > >
> >
> > OK so I think we're (probably?) now agreed, I will submit a patch shortly
> > that:
> >
> > a. puts everything in MEMORY MAPPING
> > b. Drops mm/madvise.c, mm/msync.c from the list
> > c. I commit to moving things out of the various files that truly belongs
> >     elsewhere
> > > I mean there's stuff that's weirdly used for page table moving in
> mremap.c
> > that should probably live in memory.c as well for instance.
>
> Yes, and hopefully we can clearly frame what MEMORY MAPPING is supposed to
> contain. I tried to tackle it with "/proc/self/maps output", but that's
> probably not the complete story.

I think any definition will end up a little fuzzy. For me it's anything
relating to mapping memory and changing its attributes.

I quite like the 'anything that affects /proc/self/maps output' but of
course you can find exceptions to all these rules...

I think we just have to live with a bit of vagueness unfortunately.

But we can definitely do future work on moving things around to make this
clearer. I think there's more benefit to moving stuff around than some
strict adherence to a sense of X belongs in file y etc., because it also
encourages de-duplication and prevents unrelated logic sitting in a file
where it might be neglected.

Anyway one for the new year :)

>
> For example, maybe mbind() should, for example, at some point be separated
> out into into mbind.c (making use of mempolicy.c functionality?) and covered
> there as well? I really don't know, maybe it's not one of the
> mmap/munmap/mprotect/mremap/mseal/mlock gang after all.

I sort of feel it's ok to have that in mempolicy.c with other NUMA related
syscalls, it's still grouped by syscall pretty much there.

We do have weirdness with random things that don't seem to belong sitting
in random files though, vma_migratable() being there is weird for instance.

Anyway, one thing at a time :)

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

Re: [PATCH] MAINTAINERS: group all VMA-related files into the VMA section

[Liam R. Howlett]

* David Hildenbrand <david@redhat.com> [241209 17:02]:
> > > 
> > > Maybe there is a better way to split up things or rework the code; likely
> > > we'd want this code that works on VMAs to have a clean interface with the
> > > core vma logic in vma.c, if the current way of handling it is a problem for
> > > you.
> > 
> > I think we probably need a compromise for the time being, and as I was
> > saying to Jann I don't think it makes sense really to separate the VMA and
> > page table bits from these files _except_ when we finally have a shared
> > page table interface that we've spoken about before and perhaps we will
> > collaborate on in future :)
> 
> In fork.c we split out the page table bits (into mm/memory.c), but left the
> VMA bits in there ... :)
> 
> > 
> > The key point is so we avoid stepping on landmines when we discover
> > something was merged in file X which we weren't cc'd on that breaks core
> > feature Y we have been working on in the VMA part of the code.
> 
> In my experience, tracking files is not particularly helpful. People will
> still not CC you, or do nasty stuff in files you are not tracking.
> 
> What I do is (well, besides screening most of linux-mm), is have a list of
> magic names/keywords, and tag the mails.
> 
> This way, when someone uses the magic "COW" keyword, for example, or calls
> mapcount functions, I get them put into a separate folder where I can just
> briefly sanity check them.

Maybe I can explain where this came from a little bit and see what you
think.

We have found files changed after a release without hitting the mm
branch at all.  Not just once, but a few times.

We have regular push-back and continued conversations for development
after a nack on threads with ideas that need rethinking.

Now we have a user visible undocumented change that is actively being
used that will need to be supported, forever.

It's horrible and we need to do something - anything - to try and change
what is happening.

I'm happy for lei and mailbox rules to help lighten the load, but some
of these things need more weight behind them. I also don't think that a
new tool means we should abandon the existing infrastructure.  Maybe all
the tools together will be more effective.

My hope is that having names against the files will have more weight
when someone decides that they can just take this patch through their
own branch, or mailing list, or whatever.  That is, an explicit
statement to stop doing things behind our backs.

We can also, hopefully, get more reviewers Cc'ed on these areas.

Would a different topic with you, us, and others make sense instead of
under VMA?  I would think MEMORY MAPPING might work for a combined area,
but I have no strong opinion on what we call it; this one already exists
for mmap.c.

Thanks,
Liam

Re: [PATCH] MAINTAINERS: group all VMA-related files into the VMA section

[Lorenzo Stoakes]

Hi Andrew,

Please drop this, David objects on the basis that these files contain page table
manipulation logic.

Thanks.

On Fri, Dec 06, 2024 at 07:16:00PM +0000, Lorenzo Stoakes wrote:
> There are a number of means of interacting with VMA operations within mm,
> and we have on occasion not been made aware of impactful changes due to
> these sitting in different files, most recently in [0].
>
> Correct this by bringing all VMA operations under the same section in
> MAINTAINERS. Additionally take the opportunity to combine MEMORY MAPPING
> with VMA as there needn't be two entries as they amount to the same thing.
>
> [0]:https://lore.kernel.org/linux-mm/CAG48ez0siYGB8GP5+Szgj2ovBZAkL6Zi4n6GUAjzzjFV9LTkRQ@mail.gmail.com/
>
> Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> ---
>  MAINTAINERS | 19 +++++++------------
>  1 file changed, 7 insertions(+), 12 deletions(-)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 1e930c7a58b1..95db20c26f5f 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -15060,18 +15060,6 @@ F:	tools/mm/
>  F:	tools/testing/selftests/mm/
>  N:	include/linux/page[-_]*
>
> -MEMORY MAPPING
> -M:	Andrew Morton <akpm@linux-foundation.org>
> -M:	Liam R. Howlett <Liam.Howlett@oracle.com>
> -M:	Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> -R:	Vlastimil Babka <vbabka@suse.cz>
> -R:	Jann Horn <jannh@google.com>
> -L:	linux-mm@kvack.org
> -S:	Maintained
> -W:	http://www.linux-mm.org
> -T:	git git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
> -F:	mm/mmap.c
> -
>  MEMORY TECHNOLOGY DEVICES (MTD)
>  M:	Miquel Raynal <miquel.raynal@bootlin.com>
>  M:	Richard Weinberger <richard@nod.at>
> @@ -25028,6 +25016,13 @@ L:	linux-mm@kvack.org
>  S:	Maintained
>  W:	https://www.linux-mm.org
>  T:	git git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
> +F:	mm/madvise.c
> +F:	mm/mlock.c
> +F:	mm/mmap.c
> +F:	mm/mprotect.c
> +F:	mm/mremap.c
> +F:	mm/mseal.c
> +F:	mm/msync.c
>  F:	mm/vma.c
>  F:	mm/vma.h
>  F:	mm/vma_internal.h
> --
> 2.47.1