Re: [PATCH 2/4] mm: perform VMA allocation, freeing, duplication in mm

["Liam R. Howlett" <Liam.Howlett@oracle.com>]

* Lorenzo Stoakes <lorenzo.stoakes@oracle.com> [250425 06:09]:
> On Thu, Apr 24, 2025 at 06:22:30PM -0700, Suren Baghdasaryan wrote:
> > On Thu, Apr 24, 2025 at 2:22 PM David Hildenbrand <david@redhat.com> wrote:
> > >
> > > On 24.04.25 23:15, Lorenzo Stoakes wrote:
> > > > Right now these are performed in kernel/fork.c which is odd and a violation
> > > > of separation of concerns, as well as preventing us from integrating this
> > > > and related logic into userland VMA testing going forward, and perhaps more
> > > > importantly - enabling us to, in a subsequent commit, make VMA
> > > > allocation/freeing a purely internal mm operation.
> > > >
> > > > There is a fly in the ointment - nommu - mmap.c is not compiled if
> > > > CONFIG_MMU is not set, and there is no sensible place to put these outside
> > > > of that, so we are put in the position of having to duplication some logic
> >
> > s/to duplication/to duplicate
> 
> Ack will fix!
> 
> >
> > > > here.
> > > >
> > > > This isn't ideal, but since nommu is a niche use-case, already duplicates a
> > > > great deal of mmu logic by its nature and we can eliminate code that is not
> > > > applicable to nommu, it seems a worthwhile trade-off.
> > > >
> > > > The intent is to move all this logic to vma.c in a subsequent commit,
> > > > rendering VMA allocation, freeing and duplication mm-internal-only and
> > > > userland testable.
> > >
> > > I'm pretty sure you tried it, but what's the big blocker to have patch
> > > #3 first, so we can avoid the temporary move of the code to mmap.c ?
> >
> > Completely agree with David.
> 
> Ack! Yes this was a little bit of a silly one :P
> 
> > I peeked into 4/4 and it seems you want to keep vma.c completely
> > CONFIG_MMU-centric. I know we treat NOMMU as an unwanted child but
> > IMHO it would be much cleaner to move these functions into vma.c from
> > the beginning and have an #ifdef CONFIG_MMU there like this:
> >
> > mm/vma.c
> 
> This isn't really workable, as the _entire file_ basically contains
> CONFIG_MMU-specific stuff. so it'd be one huge #ifdef CONFIG_MMU block with
> one small #else block. It'd also be asking for bugs and issues in nommu.
> 
> I think doing it this way fits the patterns we have established for
> nommu/mmap separation, and I would say nommu is enough of a niche edge case
> for us to really not want to have to go to great lengths to find ways of
> sharing code.
> 
> I am quite concerned about us having to consider it and deal with issues
> around it so often, so want to try to avoid that as much as we can,
> ideally.

I think you're asking for more issues the way you have it now.  It could
be a very long time until someone sees that nommu isn't working,
probably an entire stable kernel cycle.  Basically the longest time it
can go before being deemed unnecessary to fix.

It could also be worse, it could end up like the arch code with bugs
over a decade old not being noticed because it was forked off into
another file.

Could we create another file for the small section of common code and
achieve your goals?

Thanks,
Liam