From: Laurent Dufour <ldufour@linux.ibm.com>
To: "Michal Koutný" <mkoutny@suse.com>,
"Cyrill Gorcunov" <gorcunov@gmail.com>
Cc: mhocko@kernel.org, akpm@linux-foundation.org,
arunks@codeaurora.org, brgl@bgdev.pl, geert+renesas@glider.be,
linux-kernel@vger.kernel.org, linux-mm@kvack.org,
mguzik@redhat.com, rppt@linux.ibm.com, vbabka@suse.cz
Subject: Re: [PATCH] prctl_set_mm: downgrade mmap_sem to read lock
Date: Thu, 18 Apr 2019 16:27:53 +0200 [thread overview]
Message-ID: <27defd37-7e4e-f919-fe0c-64e1efdafdcf@linux.ibm.com> (raw)
In-Reply-To: <20190418135039.19987-1-mkoutny@suse.com>
Le 18/04/2019 à 15:50, Michal Koutný a écrit :
> I learnt, it's, alas, too late to drop the non PRCTL_SET_MM_MAP calls
> [1], so at least downgrade the write acquisition of mmap_sem as in the
> patch below (that should be stacked on the previous one or squashed).
>
> Cyrill, you mentioned lock changes in [1] but the link seems empty. Is
> it supposed to be [2]? That could be an alternative to this patch after
> some refreshments and clarifications.
>
>
> [1] https://lore.kernel.org/lkml/20190417165632.GC3040@uranus.lan/
> [2] https://lore.kernel.org/lkml/20180507075606.870903028@gmail.com/
>
> ========
>
> Since commit 88aa7cc688d4 ("mm: introduce arg_lock to protect
> arg_start|end and env_start|end in mm_struct") we use arg_lock for
> boundaries modifications. Synchronize prctl_set_mm with this lock and
> keep mmap_sem for reading only (analogous to what we already do in
> prctl_set_mm_map).
>
> Also, save few cycles by looking up VMA only after performing basic
> arguments validation.
>
> Signed-off-by: Michal Koutný <mkoutny@suse.com>
> ---
> kernel/sys.c | 12 +++++++++---
> 1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/kernel/sys.c b/kernel/sys.c
> index 12df0e5434b8..bbce0f26d707 100644
> --- a/kernel/sys.c
> +++ b/kernel/sys.c
> @@ -2125,8 +2125,12 @@ static int prctl_set_mm(int opt, unsigned long addr,
>
> error = -EINVAL;
>
> - down_write(&mm->mmap_sem);
> - vma = find_vma(mm, addr);
> + /*
> + * arg_lock protects concurent updates of arg boundaries, we need mmap_sem for
> + * a) concurrent sys_brk, b) finding VMA for addr validation.
> + */
> + down_read(&mm->mmap_sem);
> + spin_lock(&mm->arg_lock);
>
> prctl_map.start_code = mm->start_code;
> prctl_map.end_code = mm->end_code;
> @@ -2185,6 +2189,7 @@ static int prctl_set_mm(int opt, unsigned long addr,
> if (error)
> goto out;
>
> + vma = find_vma(mm, addr);
Why is find_vma() called while holding the arg_lock ?
To limit the time the spinlock is held, would it be better to
read_lock(mmap_sem)
find_vma()
spin_lock(arg_lock)
..
out:
spin_unlock()
up_read(mmap_sem)
Not sure this would change a lot the performance anyway.
> switch (opt) {
> /*
> * If command line arguments and environment
> @@ -2218,7 +2223,8 @@ static int prctl_set_mm(int opt, unsigned long addr,
>
> error = 0;
> out:
> - up_write(&mm->mmap_sem);
> + spin_unlock(&mm->arg_lock);
> + up_read(&mm->mmap_sem);
> return error;
> }
>
>
next prev parent reply other threads:[~2019-04-18 14:28 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-04-17 12:03 [PATCH] mm: get_cmdline use arg_lock instead of mmap_sem Michal Koutný
2019-04-17 13:41 ` Michal Hocko
2019-04-17 14:41 ` Michal Koutný
2019-04-17 14:55 ` Michal Hocko
2019-04-18 13:50 ` [PATCH] prctl_set_mm: downgrade mmap_sem to read lock Michal Koutný
2019-04-18 14:09 ` Cyrill Gorcunov
2019-04-18 14:15 ` Michal Hocko
2019-04-18 14:27 ` Laurent Dufour [this message]
2019-04-18 18:23 ` Cyrill Gorcunov
2019-04-30 8:18 ` [PATCH 0/3] Reduce mmap_sem usage for args manipulation Michal Koutný
2019-04-30 8:18 ` [PATCH 1/3] mm: get_cmdline use arg_lock instead of mmap_sem Michal Koutný
2019-04-30 9:09 ` Kirill Tkhai
2019-04-30 9:38 ` Cyrill Gorcunov
2019-04-30 9:53 ` Kirill Tkhai
2019-04-30 10:45 ` Cyrill Gorcunov
2019-04-30 10:56 ` Michal Koutný
2019-04-30 13:24 ` Cyrill Gorcunov
2019-04-30 8:18 ` [PATCH 2/3] prctl_set_mm: Refactor checks from validate_prctl_map Michal Koutný
2019-04-30 9:27 ` Kirill Tkhai
2019-04-30 8:18 ` [PATCH 3/3] prctl_set_mm: downgrade mmap_sem to read lock Michal Koutný
2019-04-30 8:55 ` Kirill Tkhai
2019-04-30 9:08 ` Cyrill Gorcunov
2019-04-30 9:11 ` Kirill Tkhai
2019-05-02 12:52 ` [PATCH v3 0/2] Reduce mmap_sem usage for args manipulation Michal Koutný
2019-05-02 12:52 ` [PATCH v3 1/2] prctl_set_mm: Refactor checks from validate_prctl_map Michal Koutný
2019-05-02 20:57 ` Cyrill Gorcunov
2019-05-02 12:52 ` [PATCH v3 2/2] prctl_set_mm: downgrade mmap_sem to read lock Michal Koutný
2019-05-02 20:57 ` Cyrill Gorcunov
2019-05-06 9:28 ` Kirill Tkhai
2019-05-07 17:42 ` Michal Hocko
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=27defd37-7e4e-f919-fe0c-64e1efdafdcf@linux.ibm.com \
--to=ldufour@linux.ibm.com \
--cc=akpm@linux-foundation.org \
--cc=arunks@codeaurora.org \
--cc=brgl@bgdev.pl \
--cc=geert+renesas@glider.be \
--cc=gorcunov@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=mguzik@redhat.com \
--cc=mhocko@kernel.org \
--cc=mkoutny@suse.com \
--cc=rppt@linux.ibm.com \
--cc=vbabka@suse.cz \
/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;
as well as URLs for NNTP newsgroup(s).