Re: [PATCH] mm: Update do_vmi_align_munmap() return semantics

[Linus Torvalds]

On Fri, 30 Jun 2023 at 09:41, Liam R. Howlett <Liam.Howlett@xxxxxxxxxx> wrote:
>
> I hesitate to ask considering how much trouble I've caused with the
> 32bit map flag, but I also wonder about the stack guard now that the
> write lock is taken for stack expansion?

Which part of it? We have a lot of special code wrt the whole thing
that came from how we did the expansion that probably can - and should
- be cleaned up.

For example I didn't want to go remove our ad-hoc locking, so we still
do that "mm->page_table_lock" thing.

And I think the stack expansion does several things differently from
the "normal" vma games in general, because it explicitly didn't want
to use the normal "merge vma" code because it didn't do real locking.

But you're talking about the general issue of having a stack guard
area at all, _that_ isn't affected by the locking.

That was always a real semantic issue of "we don't want user space
stack growth to possibly grow into another vma, and because the stack
growing isn't strictly predictable, we need to have that guard area in
between to catch things when they get too close".

So the stack guard isn't there to protect stack vma's from merging.
It's there to protect users from mistakes.

And then we have all those very rare situations where we *do* want
stacks to merge, and the guard goes away, but we currently do *not*
call vma_merge(), and just leave it as two adjacent vma's because we
used to only have a read-lock.

End result: I do think that doing the locking right means that we may
be able to clean up some other code. The odd do_vmi_align_munmap()
case is just one of the more egregious special cases.

              Linus