Re: [PATCH] MAINTAINERS: update MEMORY MAPPING section

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Dec 12, 2024 at 10:50:19PM -0700, Yu Zhao wrote:
> On Wed, Dec 11, 2024 at 11:57 AM Lorenzo Stoakes
> <lorenzo.stoakes@xxxxxxxxxx> 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
> > > <lorenzo.stoakes@xxxxxxxxxx> 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 <lorenzo.stoakes@xxxxxxxxxx>
> > > > ---
> > > >  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:     linux-mm@xxxxxxxxx
> > > >  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




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux