Messages in this thread |  | | Date | Wed, 1 Apr 2009 05:08:44 +0400 | From | Alexey Dobriyan <> | Subject | Re: [PATCH] Remove struct mm_struct::exe_file et al |
| |
On Tue, Mar 31, 2009 at 05:14:27PM -0700, Matt Helsley wrote: > On March 23rd Alexey Dobriyan <adobriyan@gmail.com> wrote: > > Commit 925d1c401fa6cfd0df5d2e37da8981494ccdec07 aka "procfs task exe symlink". > > introduced struct mm_struct::exe_file and struct mm_struct::num_exe_file_vmas. > > > > The rationale is weak: unifying MMU and no-MMU version of /proc/*/exe > > code. For this a) struct mm_struct becomes bigger, b) mmap/munmap/exit become > > slower, c) patch adds more code than removes in fact. ->exe_file maybe well > > Your patch adds a VMA walk with the mmap semaphore held when openning > /proc/*/exe. That gives unrelated tasks the opportunity to contend on each > others mmap semaphores just by doing a readlink on /proc/*/exe.
This is dealt with in proc_fd_access_allowed() with ptrace check.
> Did you have any measurements to report showing an improvement in performance > with your patch or are you expecting it to be "slower" based purely on code > inspection?
The performance is restored to previous levels, because code became the same as before ->exe_file.
> > defined, but doesn't make sense always. After original executable is unmapped, > > /proc/*/exe will still report it and, more importantly, pin corresponding > > struct file. ->num_exe_file_vmas is even worse -- it just counts number of > > ->exe_file does not pin the file. It uses the num_exe_file_vmas count to drop > the mm->exe_file reference when the last executable VMA is gone. Since > MAP_EXECUTABLE (and hence VM_EXECUTABLE) is only applied to the first executable > file opened during exec (stored in ->exe_file) and since userspace can't use it > (the mmap syscall code masks it out), num_exe_file_vmas counts VMAs related to > exe_file.
OK.
> So ->exe_file does not pin the file any more or less than the old VMA does... > > > executable file-backed VMAs even if those VMAs aren't related to ->exe_file. > > and this part of your argument is incorrect as well. > > > After commit 8feae13110d60cc6287afabc2887366b0eb226c2 aka > > "NOMMU: Make VMAs per MM as for MMU-mode linux" no-MMU kernels also > > maintain list of VMAs in ->mmap, so we can switch back for MMU version > > of /proc/*/exe. > > Unless the executable is fully unmapped ->exe_file won't change even if > something else manages to get mapped at a lower address. In contrast, the MMU > version is sensitive to changes in the order of the VMAs over the lifetime of > the task. It has worked in the past because MAP_EXECUTABLE only gets applied > during exec, MAP_EXECUTABLE can only be applied by the kernel, all binary > format handlers place the executable low in the mmap, and nothing else > maps MAP_EXECUTABLE regions. While ->exe_file still relies on > MAP_EXECUTABLE, it does not care about the mapped address because it > does not walk the VMAs.
Both VMA walk for /proc/*/exe and ->exe_file are heuristics and rely on the fact that userspace can't create explicit VM_EXECUTABLE mappings. Take out that and ->exe_file becomes incorrect by design -- which is the point of the patch to remove incorectness.
|  |