Le 10/07/2024 à 18:09, Liam R. Howlett a écrit : > * LEROY Christophe <christophe.leroy2@xxxxxxxxxxxxxxxxxx> [240710 08:59]: >> > ... >>>>>> >>>>>> Assuming the removal of the vdso does not cause the application to seg >>>>>> fault, then the user visible change is that any vdso call after a failed >>>>>> mmap(MAP_FIXED) call would result in a seg fault. The only reason it >>>>>> would fail is if the mapping process was attempting to map a large >>>>>> enough area over the vdso (which is accounted and in the vma tree, >>>>>> afaict) and ran out of memory. Note that this situation could arise >>>>>> already since we could run out of memory (not accounting) after the >>>>>> arch_unmap() call within the kernel. >>>>>> >>>>>> The code today can suffer the same fate, but not by the accounting >>>>>> failure. It can happen due to failure to allocate a new vma, >>>>>> do_vmi_munmap() failure after the arch_unmap() call, or any of the other >>>>>> failure scenarios later in the mmap_region() function. >>>>>> >>>>>> At the very least, this requires an expanded change log. >>>>> > ... > >>>>> I mean why are they unmapping the VDSO, why is that valid, why does it need >>>>> that field to be set to NULL, is it possible to signify that in some other >>>>> way etc.? >>>> >>>> It was originally for CRIU. So a niche workload on a niche architecture. >>>> >>>> But from the commit that added it, it sounds like CRIU was using mremap, >>>> which should be handled these days by vdso_mremap(). So it could be that >>>> arch_unmap() is not actually needed for CRIU anymore. >>> >>> Oh that's interesting! >>> >>>> >>>> Then I guess we have to decide if removing our arch_unmap() would be an >>>> ABI break, regardless of whether CRIU needs it or not. >>> >>> Seems to me like an internal implementation detail that should hopefully >>> not result in anything that should have visible ABI impact? >>> >>> I guess this is something we ought to assess. It would be useful to >>> eliminate hooks where we can so we can better control VMA behaviour without >>> having to worry about an arch being able to do arbitrary things at >>> unexpected times, especially pertinent where we change the order of things. >>> >> >> I see you are talking about arch_unmap(). I didn't follow the entire >> discussion but we have some related stuff here: >> https://github.com/linuxppc/issues/issues/241 >> >> If I remember correctly arch_unmap() should have gone away we Dmitry's >> series >> https://lore.kernel.org/lkml/20210611180242.711399-1-dima@xxxxxxxxxx/#r >> but it hasn't been applied yet. >> > > That is good news! > > To review, ppc is the only arch using this now and it sounds like you > want to remove it too. Yes want to remove it but needs to be replaced by a more generic core-based equivalent. > > Considering the age of that thread and the possibility of conflict with > my series, can I drop the entire arch_unmap() function instead of > modifying the handling in core mm? I'm going to assume that's okay and > start working on this for v4 (because there hasn't been a public reply > for v4 since 2023/10/11). drop it yes but not without implementing a replacement in core mm like proposed by Dmitry. > > This would mean less arch-specific hooks and that's always a good idea. > Indeed. Christophe
