lkml.org 
[lkml]   [2009]   [Mar]   [31]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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.


\
 
 \ /
  Last update: 2009-04-01 03:03    [W:0.057 / U:5.268 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site