* Linus Torvalds [230630 13:21]: > On Fri, 30 Jun 2023 at 09:41, Liam R. Howlett 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. I think this is probably rare enough to not be worth the added complexity? > > But you're talking about the general issue of having a stack guard > area at all, _that_ isn't affected by the locking. I was thinking about the searching for free area, which falls into this category so the locking change doesn't help much to clean this up. > > 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. Thanks for the insight. Anywhere that asks for a downgrade can be changed to an unlock so here's v2 that makes that change as well. Thanks, Liam