MAINTAINERS: update MEMORY MAPPING section

Commit Message

Lorenzo Stoakes Dec. 11, 2024, 10:53 a.m. UTC

Update the MEMORY MAPPING section to contain VMA logic as it makes no
sense to have these two sections separate.

Additionally, add files which permit changes to the attributes and/or
ranges spanned by memory mappings, in essence anything which might alter
the output of /proc/$pid/[s]maps.

This is necessarily fuzzy, as there is not quite as good separation of
concerns as we would ideally like in the kernel. However each of these
files interacts with the VMA and memory mapping logic in such a way as to
be inseparatable from it, and it is important that they are maintained in
conjunction with it.

Signed-off-by: Lorenzo Stoakes <[e-mail]>
---
 MAINTAINERS | 23 ++++++++---------------
 1 file changed, 8 insertions(+), 15 deletions(-)

Comments

Jeff Xu Dec. 11, 2024, 6:36 p.m. UTC | #1

On Wed, Dec 11, 2024 at 2:53 AM Lorenzo Stoakes
<[e-mail]> wrote:
>
> Update the MEMORY MAPPING section to contain VMA logic as it makes no
> sense to have these two sections separate.
>
> Additionally, add files which permit changes to the attributes and/or
> ranges spanned by memory mappings, in essence anything which might alter
> the output of /proc/$pid/[s]maps.
>
> This is necessarily fuzzy, as there is not quite as good separation of
> concerns as we would ideally like in the kernel. However each of these
> files interacts with the VMA and memory mapping logic in such a way as to
> be inseparatable from it, and it is important that they are maintained in
> conjunction with it.
>
> Signed-off-by: Lorenzo Stoakes <[e-mail]>
> ---
>  MAINTAINERS | 23 ++++++++---------------
>  1 file changed, 8 insertions(+), 15 deletions(-)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 68d825a4c69c..fb91389addd7 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -15071,7 +15071,15 @@ L:     [e-mail]
>  S:     Maintained
>  W:     http://www.linux-mm.org
>  T:     git git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
> +F:     mm/mlock.c
>  F:     mm/mmap.c
> +F:     mm/mprotect.c
> +F:     mm/mremap.c
> +F:     mm/mseal.c
> +F:     mm/vma.c
> +F:     mm/vma.h
> +F:     mm/vma_internal.h
> +F:     tools/testing/vma/
>
Will  madvise be here too ?
I'd like to be added as a reviewer on mm/mseal.c.  Is there any way to
indicate this from this file ?

>  MEMORY TECHNOLOGY DEVICES (MTD)
>  M:     Miquel Raynal <[e-mail]>
> @@ -25019,21 +25027,6 @@ F:     include/uapi/linux/vsockmon.h
>  F:     net/vmw_vsock/
>  F:     tools/testing/vsock/
>
> -VMA
> -M:     Andrew Morton <[e-mail]>
> -M:     Liam R. Howlett <[e-mail]>
> -M:     Lorenzo Stoakes <[e-mail]>
> -R:     Vlastimil Babka <[e-mail]>
> -R:     Jann Horn <[e-mail]>
> -L:     [e-mail]
> -S:     Maintained
> -W:     https://www.linux-mm.org
> -T:     git git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
> -F:     mm/vma.c
> -F:     mm/vma.h
> -F:     mm/vma_internal.h
> -F:     tools/testing/vma/
> -
>  VMALLOC
>  M:     Andrew Morton <[e-mail]>
>  R:     Uladzislau Rezki <[e-mail]>
> --
> 2.47.1
>
>

Lorenzo Stoakes Dec. 11, 2024, 6:57 p.m. UTC | #2

On Wed, Dec 11, 2024 at 10:36:42AM -0800, Jeff Xu wrote:
> On Wed, Dec 11, 2024 at 2:53 AM Lorenzo Stoakes
> <[e-mail]> wrote:
> >
> > Update the MEMORY MAPPING section to contain VMA logic as it makes no
> > sense to have these two sections separate.
> >
> > Additionally, add files which permit changes to the attributes and/or
> > ranges spanned by memory mappings, in essence anything which might alter
> > the output of /proc/$pid/[s]maps.
> >
> > This is necessarily fuzzy, as there is not quite as good separation of
> > concerns as we would ideally like in the kernel. However each of these
> > files interacts with the VMA and memory mapping logic in such a way as to
> > be inseparatable from it, and it is important that they are maintained in
> > conjunction with it.
> >
> > Signed-off-by: Lorenzo Stoakes <[e-mail]>
> > ---
> >  MAINTAINERS | 23 ++++++++---------------
> >  1 file changed, 8 insertions(+), 15 deletions(-)
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 68d825a4c69c..fb91389addd7 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -15071,7 +15071,15 @@ L:     [e-mail]
> >  S:     Maintained
> >  W:     http://www.linux-mm.org
> >  T:     git git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
> > +F:     mm/mlock.c
> >  F:     mm/mmap.c
> > +F:     mm/mprotect.c
> > +F:     mm/mremap.c
> > +F:     mm/mseal.c
> > +F:     mm/vma.c
> > +F:     mm/vma.h
> > +F:     mm/vma_internal.h
> > +F:     tools/testing/vma/
> >
> Will  madvise be here too ?

No. We had a long discussion about this on another version of this patch :)
it's blurry lines but it, in the end, is too much related to things other
than VMA logic.

We probably need better separation of stuff, but that's another thing...

> I'd like to be added as a reviewer on mm/mseal.c.  Is there any way to
> indicate this from this file ?

This is something we can consider in the future, sure.

However at this time you have had really significant issues in engaging
with the community on a regular basis, so I think the community is unlikely
to be open to this until you have improved in this area.

You will, of course, remain cc'd on any mseal changes regardless, so
functionally nothing will differ.

And equally, this change doesn't alter my or Liam's role, we will apply the
same review regardless.

The purpose of this change is, as the message says, to ensure the integrity
and maintainership of logic relating to memory mapping, and mseal is really
entirely a VMA operation so has to be included as a result.

So it is administrative in nature, ultimately.

>
> >  MEMORY TECHNOLOGY DEVICES (MTD)
> >  M:     Miquel Raynal <[e-mail]>
> > @@ -25019,21 +25027,6 @@ F:     include/uapi/linux/vsockmon.h
> >  F:     net/vmw_vsock/
> >  F:     tools/testing/vsock/
> >
> > -VMA
> > -M:     Andrew Morton <[e-mail]>
> > -M:     Liam R. Howlett <[e-mail]>
> > -M:     Lorenzo Stoakes <[e-mail]>
> > -R:     Vlastimil Babka <[e-mail]>
> > -R:     Jann Horn <[e-mail]>
> > -L:     [e-mail]
> > -S:     Maintained
> > -W:     https://www.linux-mm.org
> > -T:     git git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
> > -F:     mm/vma.c
> > -F:     mm/vma.h
> > -F:     mm/vma_internal.h
> > -F:     tools/testing/vma/
> > -
> >  VMALLOC
> >  M:     Andrew Morton <[e-mail]>
> >  R:     Uladzislau Rezki <[e-mail]>
> > --
> > 2.47.1
> >
> >

Vlastimil Babka Dec. 12, 2024, 8:09 a.m. UTC | #3

On 12/11/24 11:53, Lorenzo Stoakes wrote:
> Update the MEMORY MAPPING section to contain VMA logic as it makes no
> sense to have these two sections separate.
> 
> Additionally, add files which permit changes to the attributes and/or
> ranges spanned by memory mappings, in essence anything which might alter
> the output of /proc/$pid/[s]maps.
> 
> This is necessarily fuzzy, as there is not quite as good separation of
> concerns as we would ideally like in the kernel. However each of these
> files interacts with the VMA and memory mapping logic in such a way as to
> be inseparatable from it, and it is important that they are maintained in
> conjunction with it.
> 
> Signed-off-by: Lorenzo Stoakes <[e-mail]>

Yeah having MEMORY MAPPING and VMA separate but with same people wasn't
necessary. MEMORY MAPPING is IMHO the correct superset and the newly added
files there make sense to me.

Acked-by: Vlastimil Babka <[e-mail]>

> ---
>  MAINTAINERS | 23 ++++++++---------------
>  1 file changed, 8 insertions(+), 15 deletions(-)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 68d825a4c69c..fb91389addd7 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -15071,7 +15071,15 @@ L:	[e-mail]
>  S:	Maintained
>  W:	http://www.linux-mm.org
>  T:	git git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
> +F:	mm/mlock.c
>  F:	mm/mmap.c
> +F:	mm/mprotect.c
> +F:	mm/mremap.c
> +F:	mm/mseal.c
> +F:	mm/vma.c
> +F:	mm/vma.h
> +F:	mm/vma_internal.h
> +F:	tools/testing/vma/
>  
>  MEMORY TECHNOLOGY DEVICES (MTD)
>  M:	Miquel Raynal <[e-mail]>
> @@ -25019,21 +25027,6 @@ F:	include/uapi/linux/vsockmon.h
>  F:	net/vmw_vsock/
>  F:	tools/testing/vsock/
>  
> -VMA
> -M:	Andrew Morton <[e-mail]>
> -M:	Liam R. Howlett <[e-mail]>
> -M:	Lorenzo Stoakes <[e-mail]>
> -R:	Vlastimil Babka <[e-mail]>
> -R:	Jann Horn <[e-mail]>
> -L:	[e-mail]
> -S:	Maintained
> -W:	https://www.linux-mm.org
> -T:	git git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
> -F:	mm/vma.c
> -F:	mm/vma.h
> -F:	mm/vma_internal.h
> -F:	tools/testing/vma/
> -
>  VMALLOC
>  M:	Andrew Morton <[e-mail]>
>  R:	Uladzislau Rezki <[e-mail]>

David Hildenbrand Dec. 12, 2024, 2:40 p.m. UTC | #4

On 11.12.24 11:53, Lorenzo Stoakes wrote:
> Update the MEMORY MAPPING section to contain VMA logic as it makes no
> sense to have these two sections separate.
> 
> Additionally, add files which permit changes to the attributes and/or
> ranges spanned by memory mappings, in essence anything which might alter
> the output of /proc/$pid/[s]maps.
> 
> This is necessarily fuzzy, as there is not quite as good separation of
> concerns as we would ideally like in the kernel. However each of these
> files interacts with the VMA and memory mapping logic in such a way as to
> be inseparatable from it, and it is important that they are maintained in
> conjunction with it.
> 
> Signed-off-by: Lorenzo Stoakes <[e-mail]>
> ---

LGTM, thanks!

Yu Zhao Dec. 13, 2024, 5:50 a.m. UTC | #5

On Wed, Dec 11, 2024 at 11:57 AM Lorenzo Stoakes
<[e-mail]> wrote:
>
> On Wed, Dec 11, 2024 at 10:36:42AM -0800, Jeff Xu wrote:
> > On Wed, Dec 11, 2024 at 2:53 AM Lorenzo Stoakes
> > <[e-mail]> wrote:
> > >
> > > Update the MEMORY MAPPING section to contain VMA logic as it makes no
> > > sense to have these two sections separate.
> > >
> > > Additionally, add files which permit changes to the attributes and/or
> > > ranges spanned by memory mappings, in essence anything which might alter
> > > the output of /proc/$pid/[s]maps.
> > >
> > > This is necessarily fuzzy, as there is not quite as good separation of
> > > concerns as we would ideally like in the kernel. However each of these
> > > files interacts with the VMA and memory mapping logic in such a way as to
> > > be inseparatable from it, and it is important that they are maintained in
> > > conjunction with it.
> > >
> > > Signed-off-by: Lorenzo Stoakes <[e-mail]>
> > > ---
> > >  MAINTAINERS | 23 ++++++++---------------
> > >  1 file changed, 8 insertions(+), 15 deletions(-)
> > >
> > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > index 68d825a4c69c..fb91389addd7 100644
> > > --- a/MAINTAINERS
> > > +++ b/MAINTAINERS
> > > @@ -15071,7 +15071,15 @@ L:     [e-mail]
> > >  S:     Maintained
> > >  W:     http://www.linux-mm.org
> > >  T:     git git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
> > > +F:     mm/mlock.c
> > >  F:     mm/mmap.c
> > > +F:     mm/mprotect.c
> > > +F:     mm/mremap.c
> > > +F:     mm/mseal.c
> > > +F:     mm/vma.c
> > > +F:     mm/vma.h
> > > +F:     mm/vma_internal.h
> > > +F:     tools/testing/vma/
> > >
> > Will  madvise be here too ?
>
> No. We had a long discussion about this on another version of this patch :)
> it's blurry lines but it, in the end, is too much related to things other
> than VMA logic.
>
> We probably need better separation of stuff, but that's another thing...
>
> > I'd like to be added as a reviewer on mm/mseal.c.  Is there any way to
> > indicate this from this file ?
>
> This is something we can consider in the future, sure.

What'd be the downsides of having an additional reviewer? Especially
the one who wrote the code...

> However at this time you have had really significant issues in engaging
> with the community on a regular basis

I'm not aware that this can disqualify anyone from being a reviewer of
a specific file.

> so I think the community is unlikely
> to be open to this until you have improved in this area.

I do not know Jeff personally, but I think the community should make
anyone who wants to contribute feel welcome.

> You will, of course, remain cc'd on any mseal changes regardless, so
> functionally nothing will differ.
>
> And equally, this change doesn't alter my or Liam's role, we will apply the
> same review regardless.
>
> The purpose of this change is, as the message says, to ensure the integrity
> and maintainership of logic relating to memory mapping, and mseal is really
> entirely a VMA operation so has to be included as a result.
>
> So it is administrative in nature, ultimately.

Sorry -- I couldn't make out what you are trying to say here. So I'd
like to ask bluntly: is there any previous disagreement between you
and Jeff to make you reject his request? If so, I think we'd need a
3rd party (probably Andrew) to review his request. If not, I'd urge
you to use his help.

Vlastimil Babka Dec. 13, 2024, 8:25 a.m. UTC | #6

On 12/13/24 06:50, Yu Zhao wrote:
> On Wed, Dec 11, 2024 at 11:57 AM Lorenzo Stoakes
> <[e-mail]> wrote:
>>
>> > I'd like to be added as a reviewer on mm/mseal.c.  Is there any way to
>> > indicate this from this file ?

I don't think the format and tooling supports adding a reviewer for a single
file out of a subsystem. mm/mseal.c would have to be an own subsystem, or
Jeff would be indicated as a reviewer for whole memory mapping.

>> This is something we can consider in the future, sure.
> 
> What'd be the downsides of having an additional reviewer?

General answer to general question: being R: means 1. getting email for
patches touching the files (if people use tooling properly, sigh). This can
be also achieved on the receiver wised by e.g. the lei tool.
2. being perceived as an authority for people sending patches, some of them
not being familiar with the subsystem and the people working on it. This is
why it could be counter productive to be given out to just anyone who asks.

> Especially
> the one who wrote the code...

This comes back to the issue of limiting the R: to mseal.

Yu Zhao Dec. 13, 2024, 9 a.m. UTC | #7

On Fri, Dec 13, 2024 at 1:25 AM Vlastimil Babka <[e-mail]> wrote:
>
> On 12/13/24 06:50, Yu Zhao wrote:
> > On Wed, Dec 11, 2024 at 11:57 AM Lorenzo Stoakes
> > <[e-mail]> wrote:
> >>
> >> > I'd like to be added as a reviewer on mm/mseal.c.  Is there any way to
> >> > indicate this from this file ?
>
> I don't think the format and tooling supports adding a reviewer for a single
> file out of a subsystem. mm/mseal.c would have to be an own subsystem, or
> Jeff would be indicated as a reviewer for whole memory mapping.
>
> >> This is something we can consider in the future, sure.
> >
> > What'd be the downsides of having an additional reviewer?
>
> General answer to general question: being R: means 1. getting email for
> patches touching the files (if people use tooling properly, sigh). This can
> be also achieved on the receiver wised by e.g. the lei tool.
> 2. being perceived as an authority for people sending patches, some of them
> not being familiar with the subsystem and the people working on it.

I think you are saying 1 & 2 above (the meaning of being R) can lead
to the following?

> This is
> why it could be counter productive to be given out to just anyone who asks.

I understand the words, but I still don't see the logical connection.
Also, Jeff is not "anyone", and I'm not sure why he can't be
"perceived as an authority" on mseal.

Anyway, I would encourage more technical contributions rather than
"administrative" barriers.

Vlastimil Babka Dec. 13, 2024, 9:10 a.m. UTC | #8

On 12/13/24 10:00, Yu Zhao wrote:
> On Fri, Dec 13, 2024 at 1:25 AM Vlastimil Babka <[e-mail]> wrote:
>>
>> On 12/13/24 06:50, Yu Zhao wrote:
>> > On Wed, Dec 11, 2024 at 11:57 AM Lorenzo Stoakes
>> > <[e-mail]> wrote:
>> >>
>> >> > I'd like to be added as a reviewer on mm/mseal.c.  Is there any way to
>> >> > indicate this from this file ?
>>
>> I don't think the format and tooling supports adding a reviewer for a single
>> file out of a subsystem. mm/mseal.c would have to be an own subsystem, or
>> Jeff would be indicated as a reviewer for whole memory mapping.
>>
>> >> This is something we can consider in the future, sure.
>> >
>> > What'd be the downsides of having an additional reviewer?
>>
>> General answer to general question: being R: means 1. getting email for
>> patches touching the files (if people use tooling properly, sigh). This can
>> be also achieved on the receiver wised by e.g. the lei tool.
>> 2. being perceived as an authority for people sending patches, some of them
>> not being familiar with the subsystem and the people working on it.
> 
> I think you are saying 1 & 2 above (the meaning of being R) can lead
> to the following?
> 
>> This is
>> why it could be counter productive to be given out to just anyone who asks.
> 
> I understand the words, but I still don't see the logical connection.

I said "General answer to general question". You don't see a problem with
giving R: to anyone who asks, given they're now perceived as an authority by
people not very familiar with a subsystem, sending a patch and getting the
info from MAINTAINERS?

> Also, Jeff is not "anyone", and I'm not sure why he can't be
> "perceived as an authority" on mseal.

I didn't want to say anything about Jeff specifically as I haven't
interacted with him myself much. I know he did mseal but explained how
adding him R: here would not make him reviewer only for mseal.

> Anyway, I would encourage more technical contributions rather than
> "administrative" barriers.

Maybe a technical contribution to get_maintainers.pl to make it possible for
R: for a specific file? :)

Lorenzo Stoakes Dec. 13, 2024, 9:17 a.m. UTC | #9

On Thu, Dec 12, 2024 at 10:50:19PM -0700, Yu Zhao wrote:
> On Wed, Dec 11, 2024 at 11:57 AM Lorenzo Stoakes
> <[e-mail]> wrote:
> >
> > On Wed, Dec 11, 2024 at 10:36:42AM -0800, Jeff Xu wrote:
> > > On Wed, Dec 11, 2024 at 2:53 AM Lorenzo Stoakes
> > > <[e-mail]> wrote:
> > > >
> > > > Update the MEMORY MAPPING section to contain VMA logic as it makes no
> > > > sense to have these two sections separate.
> > > >
> > > > Additionally, add files which permit changes to the attributes and/or
> > > > ranges spanned by memory mappings, in essence anything which might alter
> > > > the output of /proc/$pid/[s]maps.
> > > >
> > > > This is necessarily fuzzy, as there is not quite as good separation of
> > > > concerns as we would ideally like in the kernel. However each of these
> > > > files interacts with the VMA and memory mapping logic in such a way as to
> > > > be inseparatable from it, and it is important that they are maintained in
> > > > conjunction with it.
> > > >
> > > > Signed-off-by: Lorenzo Stoakes <[e-mail]>
> > > > ---
> > > >  MAINTAINERS | 23 ++++++++---------------
> > > >  1 file changed, 8 insertions(+), 15 deletions(-)
> > > >
> > > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > > index 68d825a4c69c..fb91389addd7 100644
> > > > --- a/MAINTAINERS
> > > > +++ b/MAINTAINERS
> > > > @@ -15071,7 +15071,15 @@ L:     [e-mail]
> > > >  S:     Maintained
> > > >  W:     http://www.linux-mm.org
> > > >  T:     git git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
> > > > +F:     mm/mlock.c
> > > >  F:     mm/mmap.c
> > > > +F:     mm/mprotect.c
> > > > +F:     mm/mremap.c
> > > > +F:     mm/mseal.c
> > > > +F:     mm/vma.c
> > > > +F:     mm/vma.h
> > > > +F:     mm/vma_internal.h
> > > > +F:     tools/testing/vma/
> > > >
> > > Will  madvise be here too ?
> >
> > No. We had a long discussion about this on another version of this patch :)
> > it's blurry lines but it, in the end, is too much related to things other
> > than VMA logic.
> >
> > We probably need better separation of stuff, but that's another thing...
> >
> > > I'd like to be added as a reviewer on mm/mseal.c.  Is there any way to
> > > indicate this from this file ?
> >
> > This is something we can consider in the future, sure.
>
> What'd be the downsides of having an additional reviewer? Especially
> the one who wrote the code...
>
> > However at this time you have had really significant issues in engaging
> > with the community on a regular basis
>
> I'm not aware that this can disqualify anyone from being a reviewer of
> a specific file.
>
> > so I think the community is unlikely
> > to be open to this until you have improved in this area.
>
> I do not know Jeff personally, but I think the community should make
> anyone who wants to contribute feel welcome.

This is very unfair.

I have personally spent several hours doing my best to try to provide
advice and review strictly to help Jeff get series into the kernel, perhaps
more than anybody else.

My intent throughout has strictly been to HELP Jeff, to both ensure that
mseal is as good as it can be, and that he can be a productive and
successful member of the community.

This is, and has always been, my only intent and desire here - so things
are actually quite entirely the opposite of what you seem to think they
are.

My point here is solely that this is just an area that he needs to work on
and I'm not enitrely sure it'd be helpful until he has done so, this is
all.

>
> > You will, of course, remain cc'd on any mseal changes regardless, so
> > functionally nothing will differ.
> >
> > And equally, this change doesn't alter my or Liam's role, we will apply the
> > same review regardless.
> >
> > The purpose of this change is, as the message says, to ensure the integrity
> > and maintainership of logic relating to memory mapping, and mseal is really
> > entirely a VMA operation so has to be included as a result.
> >
> > So it is administrative in nature, ultimately.
>
> Sorry -- I couldn't make out what you are trying to say here. So I'd
> like to ask bluntly: is there any previous disagreement between you
> and Jeff to make you reject his request? If so, I think we'd need a
> 3rd party (probably Andrew) to review his request. If not, I'd urge
> you to use his help.

The point is actually a kind one - that I and others will ensure that Jeff
is _always_ involved in any technical discussion pertaining to mseal.c
regardless of the structure of this file.

However as Vlastimil points out - we can't separate out mseal.c here, not
reasonably. And it is so clearly strictly an mmap/vma bit of logic that it
really ought to be included here to ensure that we who maintain the overall
vma work can ensure everything works together - it doesn't make sense to.

Thanks, Lorenzo

Lorenzo Stoakes Dec. 13, 2024, 9:23 a.m. UTC | #10

On Fri, Dec 13, 2024 at 09:17:57AM +0000, Lorenzo Stoakes wrote:
> However as Vlastimil points out - we can't separate out mseal.c here, not
> reasonably. And it is so clearly strictly an mmap/vma bit of logic that it
> really ought to be included here to ensure that we who maintain the overall
> vma work can ensure everything works together - it doesn't make sense to.

^ it doesn't make sense _NOT_ to :P a silly typo because I've not had coffee yet...

Yu Zhao Dec. 13, 2024, 9:36 a.m. UTC | #11

On Fri, Dec 13, 2024 at 2:18 AM Lorenzo Stoakes
<[e-mail]> wrote:
>
> On Thu, Dec 12, 2024 at 10:50:19PM -0700, Yu Zhao wrote:
> > On Wed, Dec 11, 2024 at 11:57 AM Lorenzo Stoakes
> > <[e-mail]> wrote:
> > >
> > > On Wed, Dec 11, 2024 at 10:36:42AM -0800, Jeff Xu wrote:
> > > > On Wed, Dec 11, 2024 at 2:53 AM Lorenzo Stoakes
> > > > <[e-mail]> wrote:
> > > > >
> > > > > Update the MEMORY MAPPING section to contain VMA logic as it makes no
> > > > > sense to have these two sections separate.
> > > > >
> > > > > Additionally, add files which permit changes to the attributes and/or
> > > > > ranges spanned by memory mappings, in essence anything which might alter
> > > > > the output of /proc/$pid/[s]maps.
> > > > >
> > > > > This is necessarily fuzzy, as there is not quite as good separation of
> > > > > concerns as we would ideally like in the kernel. However each of these
> > > > > files interacts with the VMA and memory mapping logic in such a way as to
> > > > > be inseparatable from it, and it is important that they are maintained in
> > > > > conjunction with it.
> > > > >
> > > > > Signed-off-by: Lorenzo Stoakes <[e-mail]>
> > > > > ---
> > > > >  MAINTAINERS | 23 ++++++++---------------
> > > > >  1 file changed, 8 insertions(+), 15 deletions(-)
> > > > >
> > > > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > > > index 68d825a4c69c..fb91389addd7 100644
> > > > > --- a/MAINTAINERS
> > > > > +++ b/MAINTAINERS
> > > > > @@ -15071,7 +15071,15 @@ L:     [e-mail]
> > > > >  S:     Maintained
> > > > >  W:     http://www.linux-mm.org
> > > > >  T:     git git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
> > > > > +F:     mm/mlock.c
> > > > >  F:     mm/mmap.c
> > > > > +F:     mm/mprotect.c
> > > > > +F:     mm/mremap.c
> > > > > +F:     mm/mseal.c
> > > > > +F:     mm/vma.c
> > > > > +F:     mm/vma.h
> > > > > +F:     mm/vma_internal.h
> > > > > +F:     tools/testing/vma/
> > > > >
> > > > Will  madvise be here too ?
> > >
> > > No. We had a long discussion about this on another version of this patch :)
> > > it's blurry lines but it, in the end, is too much related to things other
> > > than VMA logic.
> > >
> > > We probably need better separation of stuff, but that's another thing...
> > >
> > > > I'd like to be added as a reviewer on mm/mseal.c.  Is there any way to
> > > > indicate this from this file ?
> > >
> > > This is something we can consider in the future, sure.
> >
> > What'd be the downsides of having an additional reviewer? Especially
> > the one who wrote the code...
> >
> > > However at this time you have had really significant issues in engaging
> > > with the community on a regular basis
> >
> > I'm not aware that this can disqualify anyone from being a reviewer of
> > a specific file.
> >
> > > so I think the community is unlikely
> > > to be open to this until you have improved in this area.
> >
> > I do not know Jeff personally, but I think the community should make
> > anyone who wants to contribute feel welcome.
>
> This is very unfair.
>
> I have personally spent several hours doing my best to try to provide
> advice and review strictly to help Jeff get series into the kernel, perhaps
> more than anybody else.
>
> My intent throughout has strictly been to HELP Jeff, to both ensure that
> mseal is as good as it can be, and that he can be a productive and
> successful member of the community.
>
> This is, and has always been, my only intent and desire here - so things
> are actually quite entirely the opposite of what you seem to think they
> are.
>
> My point here is solely that this is just an area that he needs to work on
> and I'm not enitrely sure it'd be helpful until he has done so, this is
> all.

Thanks for clarifying. It was just miscommunication.

Jeff Xu Dec. 13, 2024, 3:05 p.m. UTC | #12

Hi Lorenzo

On Fri, Dec 13, 2024 at 1:18 AM Lorenzo Stoakes
<[e-mail]> wrote:
>
> On Thu, Dec 12, 2024 at 10:50:19PM -0700, Yu Zhao wrote:
> > On Wed, Dec 11, 2024 at 11:57 AM Lorenzo Stoakes
> > <[e-mail]> wrote:
> > >
> > > On Wed, Dec 11, 2024 at 10:36:42AM -0800, Jeff Xu wrote:
> > > > On Wed, Dec 11, 2024 at 2:53 AM Lorenzo Stoakes
> > > > <[e-mail]> wrote:
> > > > >
> > > > > Update the MEMORY MAPPING section to contain VMA logic as it makes no
> > > > > sense to have these two sections separate.
> > > > >
> > > > > Additionally, add files which permit changes to the attributes and/or
> > > > > ranges spanned by memory mappings, in essence anything which might alter
> > > > > the output of /proc/$pid/[s]maps.
> > > > >
> > > > > This is necessarily fuzzy, as there is not quite as good separation of
> > > > > concerns as we would ideally like in the kernel. However each of these
> > > > > files interacts with the VMA and memory mapping logic in such a way as to
> > > > > be inseparatable from it, and it is important that they are maintained in
> > > > > conjunction with it.
> > > > >
> > > > > Signed-off-by: Lorenzo Stoakes <[e-mail]>
> > > > > ---
> > > > >  MAINTAINERS | 23 ++++++++---------------
> > > > >  1 file changed, 8 insertions(+), 15 deletions(-)
> > > > >
> > > > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > > > index 68d825a4c69c..fb91389addd7 100644
> > > > > --- a/MAINTAINERS
> > > > > +++ b/MAINTAINERS
> > > > > @@ -15071,7 +15071,15 @@ L:     [e-mail]
> > > > >  S:     Maintained
> > > > >  W:     http://www.linux-mm.org
> > > > >  T:     git git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
> > > > > +F:     mm/mlock.c
> > > > >  F:     mm/mmap.c
> > > > > +F:     mm/mprotect.c
> > > > > +F:     mm/mremap.c
> > > > > +F:     mm/mseal.c
> > > > > +F:     mm/vma.c
> > > > > +F:     mm/vma.h
> > > > > +F:     mm/vma_internal.h
> > > > > +F:     tools/testing/vma/
> > > > >
> > > > Will  madvise be here too ?
> > >
> > > No. We had a long discussion about this on another version of this patch :)
> > > it's blurry lines but it, in the end, is too much related to things other
> > > than VMA logic.
> > >
> > > We probably need better separation of stuff, but that's another thing...
> > >
> > > > I'd like to be added as a reviewer on mm/mseal.c.  Is there any way to
> > > > indicate this from this file ?
> > >
> > > This is something we can consider in the future, sure.
> >
> > What'd be the downsides of having an additional reviewer? Especially
> > the one who wrote the code...
> >
> > > However at this time you have had really significant issues in engaging
> > > with the community on a regular basis
> >
> > I'm not aware that this can disqualify anyone from being a reviewer of
> > a specific file.
> >
> > > so I think the community is unlikely
> > > to be open to this until you have improved in this area.
> >
> > I do not know Jeff personally, but I think the community should make
> > anyone who wants to contribute feel welcome.
>
> This is very unfair.
>
> I have personally spent several hours doing my best to try to provide
> advice and review strictly to help Jeff get series into the kernel, perhaps
> more than anybody else.
>
Thanks for your help (and others ) on reviewing mseal_test.c.

For the reference: I sent RFC [1] to follow up on refactor work of
selftest. To save your review time, I made minimal changes using two
test cases, and intended as a baseline/pattern for remaining
refactoring work for mseal_test.c.  If you have time to give your
comments about the RFC before the holiday break, great! I can start
refactoring the other mseal_test.  Otherwise, the after-holiday will
be fine too.

[1] https://lore.kernel.org/all/[e-mail]/

Best regards,
-Jeff

Lorenzo Stoakes Dec. 13, 2024, 3:19 p.m. UTC | #13

On Fri, Dec 13, 2024 at 07:05:06AM -0800, Jeff Xu wrote:
> Hi Lorenzo
>
> On Fri, Dec 13, 2024 at 1:18 AM Lorenzo Stoakes
> <[e-mail]> wrote:
> >
> > On Thu, Dec 12, 2024 at 10:50:19PM -0700, Yu Zhao wrote:
> > > On Wed, Dec 11, 2024 at 11:57 AM Lorenzo Stoakes
> > > <[e-mail]> wrote:
> > > >
> > > > On Wed, Dec 11, 2024 at 10:36:42AM -0800, Jeff Xu wrote:
> > > > > On Wed, Dec 11, 2024 at 2:53 AM Lorenzo Stoakes
> > > > > <[e-mail]> wrote:
> > > > > >
> > > > > > Update the MEMORY MAPPING section to contain VMA logic as it makes no
> > > > > > sense to have these two sections separate.
> > > > > >
> > > > > > Additionally, add files which permit changes to the attributes and/or
> > > > > > ranges spanned by memory mappings, in essence anything which might alter
> > > > > > the output of /proc/$pid/[s]maps.
> > > > > >
> > > > > > This is necessarily fuzzy, as there is not quite as good separation of
> > > > > > concerns as we would ideally like in the kernel. However each of these
> > > > > > files interacts with the VMA and memory mapping logic in such a way as to
> > > > > > be inseparatable from it, and it is important that they are maintained in
> > > > > > conjunction with it.
> > > > > >
> > > > > > Signed-off-by: Lorenzo Stoakes <[e-mail]>
> > > > > > ---
> > > > > >  MAINTAINERS | 23 ++++++++---------------
> > > > > >  1 file changed, 8 insertions(+), 15 deletions(-)
> > > > > >
> > > > > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > > > > index 68d825a4c69c..fb91389addd7 100644
> > > > > > --- a/MAINTAINERS
> > > > > > +++ b/MAINTAINERS
> > > > > > @@ -15071,7 +15071,15 @@ L:     [e-mail]
> > > > > >  S:     Maintained
> > > > > >  W:     http://www.linux-mm.org
> > > > > >  T:     git git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
> > > > > > +F:     mm/mlock.c
> > > > > >  F:     mm/mmap.c
> > > > > > +F:     mm/mprotect.c
> > > > > > +F:     mm/mremap.c
> > > > > > +F:     mm/mseal.c
> > > > > > +F:     mm/vma.c
> > > > > > +F:     mm/vma.h
> > > > > > +F:     mm/vma_internal.h
> > > > > > +F:     tools/testing/vma/
> > > > > >
> > > > > Will  madvise be here too ?
> > > >
> > > > No. We had a long discussion about this on another version of this patch :)
> > > > it's blurry lines but it, in the end, is too much related to things other
> > > > than VMA logic.
> > > >
> > > > We probably need better separation of stuff, but that's another thing...
> > > >
> > > > > I'd like to be added as a reviewer on mm/mseal.c.  Is there any way to
> > > > > indicate this from this file ?
> > > >
> > > > This is something we can consider in the future, sure.
> > >
> > > What'd be the downsides of having an additional reviewer? Especially
> > > the one who wrote the code...
> > >
> > > > However at this time you have had really significant issues in engaging
> > > > with the community on a regular basis
> > >
> > > I'm not aware that this can disqualify anyone from being a reviewer of
> > > a specific file.
> > >
> > > > so I think the community is unlikely
> > > > to be open to this until you have improved in this area.
> > >
> > > I do not know Jeff personally, but I think the community should make
> > > anyone who wants to contribute feel welcome.
> >
> > This is very unfair.
> >
> > I have personally spent several hours doing my best to try to provide
> > advice and review strictly to help Jeff get series into the kernel, perhaps
> > more than anybody else.
> >
> Thanks for your help (and others ) on reviewing mseal_test.c.
>

Thanks :)

> For the reference: I sent RFC [1] to follow up on refactor work of
> selftest. To save your review time, I made minimal changes using two
> test cases, and intended as a baseline/pattern for remaining
> refactoring work for mseal_test.c.  If you have time to give your
> comments about the RFC before the holiday break, great! I can start
> refactoring the other mseal_test.  Otherwise, the after-holiday will
> be fine too.
>
> [1] https://lore.kernel.org/all/[e-mail]/

Yeah I'm so sorry for not getting to this sooner, that's my bad, got tied up
looking at other things. I will definitely take a look and PLEASE do ping me if
I don't it's no problem.

I do think at this point because I'm off from next Mon it'll be post-xmas, but I
_will_ get to it.

I will do the only completely reliable thing for me re: TODOs, which is to write
it on the whiteboard :P

>
> Best regards,
> -Jeff

Cheers!

Patch

diff --git a/MAINTAINERS b/MAINTAINERS
index 68d825a4c69c..fb91389addd7 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -15071,7 +15071,15 @@  L:	[e-mail]
 S:	Maintained
 W:	http://www.linux-mm.org
 T:	git git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
+F:	mm/mlock.c
 F:	mm/mmap.c
+F:	mm/mprotect.c
+F:	mm/mremap.c
+F:	mm/mseal.c
+F:	mm/vma.c
+F:	mm/vma.h
+F:	mm/vma_internal.h
+F:	tools/testing/vma/
 
 MEMORY TECHNOLOGY DEVICES (MTD)
 M:	Miquel Raynal <[e-mail]>
@@ -25019,21 +25027,6 @@  F:	include/uapi/linux/vsockmon.h
 F:	net/vmw_vsock/
 F:	tools/testing/vsock/
 
-VMA
-M:	Andrew Morton <[e-mail]>
-M:	Liam R. Howlett <[e-mail]>
-M:	Lorenzo Stoakes <[e-mail]>
-R:	Vlastimil Babka <[e-mail]>
-R:	Jann Horn <[e-mail]>
-L:	[e-mail]
-S:	Maintained
-W:	https://www.linux-mm.org
-T:	git git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
-F:	mm/vma.c
-F:	mm/vma.h
-F:	mm/vma_internal.h
-F:	tools/testing/vma/
-
 VMALLOC
 M:	Andrew Morton <[e-mail]>
 R:	Uladzislau Rezki <[e-mail]>