On Fri, Apr 25, 2025 at 07:00:27AM -0400, Liam R. Howlett wrote: > * Lorenzo Stoakes <lorenzo.stoakes@xxxxxxxxxx> [250425 06:45]: > ... > > > > > > > > > > > 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? > > > > > > That'd completely defeat the purpose of isolating core functions to vma.c. > > > > > > Again, I don't believe that bending over backwards to support this niche > > > use is appropriate. > > > > > > And if you're making a change to vm_area_alloc(), vm_area_free(), > > > vm_area_init_from(), vm_area_dup() it'd seem like an oversight not to check > > > nommu right? > > > > > > There's already a large amount of duplicated logic there specific to nommu > > > for which precisely the same could be said, including entirely parallel > > > brk(), mmap() implementations. > > > > > > So this isn't a change in how we handle nommu. > > > > I guess an alternative is to introduce a new vma_shared.c, vma_shared.h > > pair of files here, that we try to allow userland isolation for so vma.c > > can still use for userland testing. > > > > This then aligns with your requirement, and keeps it vma-centric like > > Suren's suggestion. > > > > Or perhaps it could even be vma_init.c, vma_init.h? To denote that it > > references the initialisation and allocation, etc. of VMAs? > > Sure, the name isn't as important as the concept, but I like vma_init > better than vma_shared. > > > > > Anyway we do that, we share it across all, and it solves all > > problems... gives us the isolation for userland testing and also isolation > > in mm, while also ensuring no code duplication with nommu. > > > > That work? > > Yes, this is what I was suggesting. Ack, I wondered if you meant placing it in some existing shared file such as mm/util.c hence objecting, but this works better :) > > I really think this is the least painful way to deal with nommu. > Otherwise we will waste more time later trying to fix what was > overlooked. Sure, I definitely understand the motivation. I resent that we have to think about this, but we do have to live with it, so agreed this is the best approach. > > Thanks, > Liam > Will rework series to do this and send a v2. Cheers, Lorenzo