On 3/26/18 11:29 PM, Michal Hocko wrote:
On Tue 27-03-18 02:20:39, Yang Shi wrote:
[...]
The patch looks reasonable to me. Maybe it would be better to be more
explicit about the purpose of the patch. As others noticed, this alone
wouldn't solve the mmap_sem contention issues. I _think_ that if
On Tue 27-03-18 21:52:17, Cyrill Gorcunov wrote:
> On Tue, Mar 27, 2018 at 02:38:11PM -0400, Yang Shi wrote:
> > > Why do we need to hold mmap_sem here and call find_vma, when only
> > > PR_SET_MM_ENV_END: is consuming it? I guess we can replace it wit the
> > > new lock and take the mmap_sem only
On Tue, Mar 27, 2018 at 02:38:11PM -0400, Yang Shi wrote:
> > Why do we need to hold mmap_sem here and call find_vma, when only
> > PR_SET_MM_ENV_END: is consuming it? I guess we can replace it wit the
> > new lock and take the mmap_sem only for PR_SET_MM_ENV_END.
>
> Actually, I didn't think of w
On 3/27/18 3:32 AM, Cyrill Gorcunov wrote:
On Mon, Mar 26, 2018 at 05:59:49PM -0400, Yang Shi wrote:
Say we've two syscalls running prctl_set_mm_map in parallel, and imagine
one have @start_brk = 20 @brk = 10 and second caller has @start_brk = 30
and @brk = 20. Since now the call is guarded by
On 3/27/18 2:29 AM, Michal Hocko wrote:
On Tue 27-03-18 02:20:39, Yang Shi wrote:
[...]
The patch looks reasonable to me. Maybe it would be better to be more
explicit about the purpose of the patch. As others noticed, this alone
wouldn't solve the mmap_sem contention issues. I _think_ that if y
On Tue 27-03-18 16:31:23, Mateusz Guzik wrote:
> On Tue, Mar 27, 2018 at 08:29:39AM +0200, Michal Hocko wrote:
> > On Tue 27-03-18 02:20:39, Yang Shi wrote:
> > [...]
> > The patch looks reasonable to me. Maybe it would be better to be more
> > explicit about the purpose of the patch. As others not
On Tue, Mar 27, 2018 at 08:29:39AM +0200, Michal Hocko wrote:
> On Tue 27-03-18 02:20:39, Yang Shi wrote:
> [...]
> The patch looks reasonable to me. Maybe it would be better to be more
> explicit about the purpose of the patch. As others noticed, this alone
> wouldn't solve the mmap_sem contention
On Mon, Mar 26, 2018 at 06:12:55PM -0400, Yang Shi wrote:
> > + if (unlikely(arg_start > arg_end || env_start > env_end)) {
> > + cond_resched();
> > + goto retry;
>
> Can't it trap into dead loop if the condition is always false?
Yes, unfortunately it can.
> > + }
> >
>
On Tue, Mar 27, 2018 at 07:00:56AM +0900, Tetsuo Handa wrote:
>
> > To be fair I would prefer to drop this old per-field
> > interface completely. This per-field interface was rather an ugly
> > solution from my side.
>
> But this is userspace visible API and thus we cannot change.
H
On Mon, Mar 26, 2018 at 05:59:49PM -0400, Yang Shi wrote:
> > Say we've two syscalls running prctl_set_mm_map in parallel, and imagine
> > one have @start_brk = 20 @brk = 10 and second caller has @start_brk = 30
> > and @brk = 20. Since now the call is guarded by _read_ the both calls
> > unlocked
On Tue 27-03-18 02:20:39, Yang Shi wrote:
[...]
The patch looks reasonable to me. Maybe it would be better to be more
explicit about the purpose of the patch. As others noticed, this alone
wouldn't solve the mmap_sem contention issues. I _think_ that if you
were more explicit about the mmap_sem abu
On 3/26/18 6:00 PM, Tetsuo Handa wrote:
Cyrill Gorcunov wrote:
On Tue, Mar 27, 2018 at 06:10:09AM +0900, Tetsuo Handa wrote:
On 2018/03/27 4:21, Cyrill Gorcunov wrote:
That said I think using read-lock here would be a bug.
If I understand correctly, the caller can't set both fields atomical
Cyrill Gorcunov wrote:
> On Tue, Mar 27, 2018 at 06:10:09AM +0900, Tetsuo Handa wrote:
> > On 2018/03/27 4:21, Cyrill Gorcunov wrote:
> > > That said I think using read-lock here would be a bug.
> >
> > If I understand correctly, the caller can't set both fields atomically, for
> > prctl() does no
On 3/26/18 3:21 PM, Cyrill Gorcunov wrote:
On Mon, Mar 26, 2018 at 11:37:25AM -0700, Matthew Wilcox wrote:
On Tue, Mar 27, 2018 at 02:20:39AM +0800, Yang Shi wrote:
+++ b/kernel/sys.c
@@ -1959,7 +1959,7 @@ static int prctl_set_mm_map(int opt, const void __user
*addr, unsigned long data
On Tue, Mar 27, 2018 at 06:10:09AM +0900, Tetsuo Handa wrote:
> On 2018/03/27 4:21, Cyrill Gorcunov wrote:
> > That said I think using read-lock here would be a bug.
>
> If I understand correctly, the caller can't set both fields atomically, for
> prctl() does not receive both fields at one call.
On 3/26/18 5:10 PM, Tetsuo Handa wrote:
On 2018/03/27 4:21, Cyrill Gorcunov wrote:
That said I think using read-lock here would be a bug.
If I understand correctly, the caller can't set both fields atomically, for
prctl() does not receive both fields at one call.
prctl(PR_SET_MM, PR_SET_M
On 3/26/18 3:42 PM, Mateusz Guzik wrote:
On Tue, Mar 27, 2018 at 02:20:39AM +0800, Yang Shi wrote:
mmap_sem is on the hot path of kernel, and it very contended, but it is
abused too. It is used to protect arg_start|end and evn_start|end when
reading /proc/$PID/cmdline and /proc/$PID/environ, b
On 2018/03/27 4:21, Cyrill Gorcunov wrote:
> That said I think using read-lock here would be a bug.
If I understand correctly, the caller can't set both fields atomically, for
prctl() does not receive both fields at one call.
prctl(PR_SET_MM, PR_SET_MM_ARG_START xor PR_SET_MM_ARG_END xor
PR_SE
On Tue, Mar 27, 2018 at 02:20:39AM +0800, Yang Shi wrote:
> mmap_sem is on the hot path of kernel, and it very contended, but it is
> abused too. It is used to protect arg_start|end and evn_start|end when
> reading /proc/$PID/cmdline and /proc/$PID/environ, but it doesn't make
> sense since those p
On Mon, Mar 26, 2018 at 11:37:25AM -0700, Matthew Wilcox wrote:
> On Tue, Mar 27, 2018 at 02:20:39AM +0800, Yang Shi wrote:
> > +++ b/kernel/sys.c
> > @@ -1959,7 +1959,7 @@ static int prctl_set_mm_map(int opt, const void
> > __user *addr, unsigned long data
> > return error;
>
On Tue, Mar 27, 2018 at 02:20:39AM +0800, Yang Shi wrote:
> +++ b/kernel/sys.c
> @@ -1959,7 +1959,7 @@ static int prctl_set_mm_map(int opt, const void __user
> *addr, unsigned long data
> return error;
> }
>
> - down_write(&mm->mmap_sem);
> + down_read(&mm->mm
mmap_sem is on the hot path of kernel, and it very contended, but it is
abused too. It is used to protect arg_start|end and evn_start|end when
reading /proc/$PID/cmdline and /proc/$PID/environ, but it doesn't make
sense since those proc files just expect to read 4 values atomically and
not related
22 matches
Mail list logo
