From: Oleg Nesterov <oleg@redhat.com>
To: Cyrill Gorcunov <gorcunov@gmail.com>
Cc: Davidlohr Bueso <dave.bueso@gmail.com>,
Andrew Morton <akpm@linux-foundation.org>,
Alexander Viro <viro@zeniv.linux.org.uk>,
linux-mm@kvack.org, linux-kernel@vger.kernel.org,
dave@stgolabs.net
Subject: Re: [PATCH] mm: replace mmap_sem for mm->exe_file serialization
Date: Fri, 27 Feb 2015 18:36:50 +0100 [thread overview]
Message-ID: <20150227173650.GA18823@redhat.com> (raw)
In-Reply-To: <20150226205145.GH3041@moon>
On 02/26, Cyrill Gorcunov wrote:
>
> On Thu, Feb 26, 2015 at 11:36:57AM -0800, Davidlohr Bueso wrote:
> > We currently use the mmap_sem to serialize the mm exe_file.
> > This is atrocious and a clear example of the misuses this
> > lock has all over the place, making any significant changes
> > to the address space locking that much more complex and tedious.
> > This also has to do of how we used to check for the vma's vm_file
> > being VM_EXECUTABLE (much of which was replaced by 2dd8ad81e31).
> >
> > This patch, therefore, removes the mmap_sem dependency and
> > introduces a specific lock for the exe_file (rwlock_t, as it is
> > read mostly and protects a trivial critical region). As mentioned,
> > the motivation is to cleanup mmap_sem (as opposed to exe_file
> > performance).
Well, I didn't see the patch, can't really comment.
But I have to admit that this looks as atrocious and a clear example of
"lets add yet another random lock which we will regret about later" ;)
rwlock_t in mm_struct just to serialize access to exe_file?
> A nice side effect of this is that we avoid taking
> > the mmap_sem (shared) in fork paths for the exe_file handling
> > (note that readers block when the rwsem is taken exclusively by
> > another thread).
Yes, this is ugly. Can't we kill this dup_mm_exe_file() and copy change
dup_mmap() to also dup ->exe_file ?
> Hi Davidlohr, it would be interesting to know if the cleanup
> bring some performance benefit?
To me the main question is whether the patch makes this code simpler
or uglier ;)
Oleg.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
WARNING: multiple messages have this Message-ID (diff)
From: Oleg Nesterov <oleg@redhat.com>
To: Cyrill Gorcunov <gorcunov@gmail.com>
Cc: Davidlohr Bueso <dave.bueso@gmail.com>,
Andrew Morton <akpm@linux-foundation.org>,
Alexander Viro <viro@zeniv.linux.org.uk>,
linux-mm@kvack.org, linux-kernel@vger.kernel.org,
dave@stgolabs.net
Subject: Re: [PATCH] mm: replace mmap_sem for mm->exe_file serialization
Date: Fri, 27 Feb 2015 18:36:50 +0100 [thread overview]
Message-ID: <20150227173650.GA18823@redhat.com> (raw)
In-Reply-To: <20150226205145.GH3041@moon>
On 02/26, Cyrill Gorcunov wrote:
>
> On Thu, Feb 26, 2015 at 11:36:57AM -0800, Davidlohr Bueso wrote:
> > We currently use the mmap_sem to serialize the mm exe_file.
> > This is atrocious and a clear example of the misuses this
> > lock has all over the place, making any significant changes
> > to the address space locking that much more complex and tedious.
> > This also has to do of how we used to check for the vma's vm_file
> > being VM_EXECUTABLE (much of which was replaced by 2dd8ad81e31).
> >
> > This patch, therefore, removes the mmap_sem dependency and
> > introduces a specific lock for the exe_file (rwlock_t, as it is
> > read mostly and protects a trivial critical region). As mentioned,
> > the motivation is to cleanup mmap_sem (as opposed to exe_file
> > performance).
Well, I didn't see the patch, can't really comment.
But I have to admit that this looks as atrocious and a clear example of
"lets add yet another random lock which we will regret about later" ;)
rwlock_t in mm_struct just to serialize access to exe_file?
> A nice side effect of this is that we avoid taking
> > the mmap_sem (shared) in fork paths for the exe_file handling
> > (note that readers block when the rwsem is taken exclusively by
> > another thread).
Yes, this is ugly. Can't we kill this dup_mm_exe_file() and copy change
dup_mmap() to also dup ->exe_file ?
> Hi Davidlohr, it would be interesting to know if the cleanup
> bring some performance benefit?
To me the main question is whether the patch makes this code simpler
or uglier ;)
Oleg.
next prev parent reply other threads:[~2015-02-27 17:38 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-02-26 19:36 [PATCH] mm: replace mmap_sem for mm->exe_file serialization Davidlohr Bueso
2015-02-26 19:36 ` Davidlohr Bueso
2015-02-26 20:51 ` Cyrill Gorcunov
2015-02-26 20:51 ` Cyrill Gorcunov
2015-02-27 17:36 ` Oleg Nesterov [this message]
2015-02-27 17:36 ` Oleg Nesterov
2015-02-27 18:34 ` Davidlohr Bueso
2015-02-27 18:34 ` Davidlohr Bueso
2015-03-11 12:21 ` Konstantin Khlebnikov
2015-03-11 12:21 ` Konstantin Khlebnikov
2015-03-11 12:40 ` Davidlohr Bueso
2015-03-11 12:40 ` Davidlohr Bueso
2015-03-11 13:26 ` Konstantin Khlebnikov
2015-03-11 13:26 ` Konstantin Khlebnikov
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=20150227173650.GA18823@redhat.com \
--to=oleg@redhat.com \
--cc=akpm@linux-foundation.org \
--cc=dave.bueso@gmail.com \
--cc=dave@stgolabs.net \
--cc=gorcunov@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=viro@zeniv.linux.org.uk \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.