public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Matt Helsley <matthltc@us.ibm.com>
To: Alexey Dobriyan <adobriyan@gmail.com>
Cc: LKML <linux-kernel@vger.kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Eric Biederman <ebiederm@xmission.com>,
	Oleg Nesterov <oleg@redhat.com>, Christoph Hellwig <hch@lst.de>,
	David Howells <dhowells@redhat.com>
Subject: Re: [PATCH] Remove struct mm_struct::exe_file et al
Date: Tue, 31 Mar 2009 17:14:27 -0700	[thread overview]
Message-ID: <20090401001427.GG29821@us.ibm.com> (raw)

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.

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? 

(Which reminds me of this thread: http://lkml.org/lkml/2009/3/11/114 )

> 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.

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.

Cheers,
	-Matt Helsley

             reply	other threads:[~2009-04-01  0:14 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-04-01  0:14 Matt Helsley [this message]
2009-04-01  1:08 ` [PATCH] Remove struct mm_struct::exe_file et al Alexey Dobriyan
2009-04-01 14:53 ` David Howells
  -- strict thread matches above, loose matches on Subject: below --
2009-03-23 13:17 Alexey Dobriyan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20090401001427.GG29821@us.ibm.com \
    --to=matthltc@us.ibm.com \
    --cc=adobriyan@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=dhowells@redhat.com \
    --cc=ebiederm@xmission.com \
    --cc=hch@lst.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=oleg@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox