All of lore.kernel.org
 help / color / mirror / Atom feed
From: Cyrill Gorcunov <gorcunov@gmail.com>
To: Davidlohr Bueso <dave.bueso@gmail.com>
Cc: 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, Oleg Nesterov <oleg@redhat.com>
Subject: Re: [PATCH] mm: replace mmap_sem for mm->exe_file serialization
Date: Thu, 26 Feb 2015 23:51:45 +0300	[thread overview]
Message-ID: <20150226205145.GH3041@moon> (raw)
In-Reply-To: <1424979417.10344.14.camel@stgolabs.net>

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). 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).
> 
> Now that callers have been updated and standardized[1, 2] around
> the get_mm_set_exe_file() interface, changing the locking scheme
> is quite straightforward. The exception being the prctl calls
> (ie PR_SET_MM_EXE_FILE). Because this caller actually _updates_
> the mm->exe_file, we need to handle it in the same patch that changes
> the locking rules. For this we need to reorganize prctl_set_mm_exe_file,
> such that:
> 
> o mmap_sem is taken when actually needed.
> 
> o a new set_mm_exe_file_locked() function is introduced to be used by
>   prctl. We now need to explicitly acquire the exe_file_lock as before
>   it was implicit in holding the mmap_sem for write.
> 
> o a new __prctl_set_mm_exe_file() helper is created, which actually
>   does the exe_file handling for the mm side -- needing the write
>   lock for updating the mm->flags (*sigh*). In the future we could
>   have a unique mm::exe_file_struct and keep track of MMF_EXE_FILE_CHANGED
>   on our own.
> 
> mm: improve handling of mm->exe_file
> [1] Part 1: https://lkml.org/lkml/2015/2/18/721
> [2] Part 2: https://lkml.org/lkml/2015/2/25/679
> 
> Applies on top of linux-next (20150225).
> 
> Signed-off-by: Davidlohr Bueso <dbueso@suse.de>

Hi Davidlohr, it would be interesting to know if the cleanup
bring some performance benefit?

--
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: Cyrill Gorcunov <gorcunov@gmail.com>
To: Davidlohr Bueso <dave.bueso@gmail.com>
Cc: 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, Oleg Nesterov <oleg@redhat.com>
Subject: Re: [PATCH] mm: replace mmap_sem for mm->exe_file serialization
Date: Thu, 26 Feb 2015 23:51:45 +0300	[thread overview]
Message-ID: <20150226205145.GH3041@moon> (raw)
In-Reply-To: <1424979417.10344.14.camel@stgolabs.net>

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). 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).
> 
> Now that callers have been updated and standardized[1, 2] around
> the get_mm_set_exe_file() interface, changing the locking scheme
> is quite straightforward. The exception being the prctl calls
> (ie PR_SET_MM_EXE_FILE). Because this caller actually _updates_
> the mm->exe_file, we need to handle it in the same patch that changes
> the locking rules. For this we need to reorganize prctl_set_mm_exe_file,
> such that:
> 
> o mmap_sem is taken when actually needed.
> 
> o a new set_mm_exe_file_locked() function is introduced to be used by
>   prctl. We now need to explicitly acquire the exe_file_lock as before
>   it was implicit in holding the mmap_sem for write.
> 
> o a new __prctl_set_mm_exe_file() helper is created, which actually
>   does the exe_file handling for the mm side -- needing the write
>   lock for updating the mm->flags (*sigh*). In the future we could
>   have a unique mm::exe_file_struct and keep track of MMF_EXE_FILE_CHANGED
>   on our own.
> 
> mm: improve handling of mm->exe_file
> [1] Part 1: https://lkml.org/lkml/2015/2/18/721
> [2] Part 2: https://lkml.org/lkml/2015/2/25/679
> 
> Applies on top of linux-next (20150225).
> 
> Signed-off-by: Davidlohr Bueso <dbueso@suse.de>

Hi Davidlohr, it would be interesting to know if the cleanup
bring some performance benefit?

  reply	other threads:[~2015-02-26 20:51 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 [this message]
2015-02-26 20:51   ` Cyrill Gorcunov
2015-02-27 17:36   ` Oleg Nesterov
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=20150226205145.GH3041@moon \
    --to=gorcunov@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=dave.bueso@gmail.com \
    --cc=dave@stgolabs.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=oleg@redhat.com \
    --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.