Re: [v3 PATCH] mm: introduce arg_lock to protect arg_start|end and env_start|end in mm_struct

2018-04-13 Thread Michal Hocko
On Thu 12-04-18 09:20:24, Yang Shi wrote: > > > On 4/12/18 5:18 AM, Michal Hocko wrote: > > On Tue 10-04-18 11:28:13, Yang Shi wrote: > > > > > > On 4/10/18 9:21 AM, Yang Shi wrote: > > > > > > > > On 4/10/18 5:28 AM, Cyrill Gorcunov wrote: > > > > > On Tue, Apr 10, 2018 at 01:10:01PM +0200,

Re: [v3 PATCH] mm: introduce arg_lock to protect arg_start|end and env_start|end in mm_struct

2018-04-12 Thread Yang Shi
On 4/12/18 5:18 AM, Michal Hocko wrote: On Tue 10-04-18 11:28:13, Yang Shi wrote: On 4/10/18 9:21 AM, Yang Shi wrote: On 4/10/18 5:28 AM, Cyrill Gorcunov wrote: On Tue, Apr 10, 2018 at 01:10:01PM +0200, Michal Hocko wrote: Because do_brk does vma manipulations, for this reason it's

Re: [v3 PATCH] mm: introduce arg_lock to protect arg_start|end and env_start|end in mm_struct

2018-04-12 Thread Michal Hocko
On Tue 10-04-18 11:28:13, Yang Shi wrote: > > > On 4/10/18 9:21 AM, Yang Shi wrote: > > > > > > On 4/10/18 5:28 AM, Cyrill Gorcunov wrote: > > > On Tue, Apr 10, 2018 at 01:10:01PM +0200, Michal Hocko wrote: > > > > > Because do_brk does vma manipulations, for this reason it's > > > > > running

Re: [v3 PATCH] mm: introduce arg_lock to protect arg_start|end and env_start|end in mm_struct

2018-04-10 Thread Cyrill Gorcunov
On Tue, Apr 10, 2018 at 12:33:35PM -0700, Yang Shi wrote: ... > > The race condition is just valid when protecting start_brk, brk, start_data > and end_data with the new lock, but keep using mmap_sem in brk path. > > So, we should just need make a little tweak to have mmap_sem protect >

Re: [v3 PATCH] mm: introduce arg_lock to protect arg_start|end and env_start|end in mm_struct

2018-04-10 Thread Yang Shi
On 4/10/18 12:17 PM, Cyrill Gorcunov wrote: On Tue, Apr 10, 2018 at 11:28:13AM -0700, Yang Shi wrote: At the first glance, it looks feasible to me. Will look into deeper later. A further look told me this might be *not* feasible. It looks the new lock will not break check_data_rlimit since

Re: [v3 PATCH] mm: introduce arg_lock to protect arg_start|end and env_start|end in mm_struct

2018-04-10 Thread Cyrill Gorcunov
On Tue, Apr 10, 2018 at 11:28:13AM -0700, Yang Shi wrote: > > > > At the first glance, it looks feasible to me. Will look into deeper > > later. > > A further look told me this might be *not* feasible. > > It looks the new lock will not break check_data_rlimit since in my patch > both start_brk

Re: [v3 PATCH] mm: introduce arg_lock to protect arg_start|end and env_start|end in mm_struct

2018-04-10 Thread Yang Shi
On 4/10/18 9:21 AM, Yang Shi wrote: On 4/10/18 5:28 AM, Cyrill Gorcunov wrote: On Tue, Apr 10, 2018 at 01:10:01PM +0200, Michal Hocko wrote: Because do_brk does vma manipulations, for this reason it's running under down_write_killable(>mmap_sem). Or you mean something else? Yes, all we

Re: [v3 PATCH] mm: introduce arg_lock to protect arg_start|end and env_start|end in mm_struct

2018-04-10 Thread Yang Shi
On 4/10/18 5:28 AM, Cyrill Gorcunov wrote: On Tue, Apr 10, 2018 at 01:10:01PM +0200, Michal Hocko wrote: Because do_brk does vma manipulations, for this reason it's running under down_write_killable(>mmap_sem). Or you mean something else? Yes, all we need the new lock for is to get a

Re: [v3 PATCH] mm: introduce arg_lock to protect arg_start|end and env_start|end in mm_struct

2018-04-10 Thread Cyrill Gorcunov
On Tue, Apr 10, 2018 at 01:10:01PM +0200, Michal Hocko wrote: > > > > Because do_brk does vma manipulations, for this reason it's > > running under down_write_killable(>mmap_sem). Or you > > mean something else? > > Yes, all we need the new lock for is to get a consistent view on brk > values. I

Re: [v3 PATCH] mm: introduce arg_lock to protect arg_start|end and env_start|end in mm_struct

2018-04-10 Thread Michal Hocko
On Tue 10-04-18 14:02:42, Cyrill Gorcunov wrote: > On Tue, Apr 10, 2018 at 12:42:15PM +0200, Michal Hocko wrote: > > On Tue 10-04-18 12:40:47, Cyrill Gorcunov wrote: > > > On Tue, Apr 10, 2018 at 11:09:17AM +0200, Michal Hocko wrote: > > > > On Tue 10-04-18 05:52:54, Yang Shi wrote: > > > > [...]

Re: [v3 PATCH] mm: introduce arg_lock to protect arg_start|end and env_start|end in mm_struct

2018-04-10 Thread Cyrill Gorcunov
On Tue, Apr 10, 2018 at 12:42:15PM +0200, Michal Hocko wrote: > On Tue 10-04-18 12:40:47, Cyrill Gorcunov wrote: > > On Tue, Apr 10, 2018 at 11:09:17AM +0200, Michal Hocko wrote: > > > On Tue 10-04-18 05:52:54, Yang Shi wrote: > > > [...] > > > > So, introduce a new spinlock in mm_struct to

Re: [v3 PATCH] mm: introduce arg_lock to protect arg_start|end and env_start|end in mm_struct

2018-04-10 Thread Michal Hocko
On Tue 10-04-18 12:40:47, Cyrill Gorcunov wrote: > On Tue, Apr 10, 2018 at 11:09:17AM +0200, Michal Hocko wrote: > > On Tue 10-04-18 05:52:54, Yang Shi wrote: > > [...] > > > So, introduce a new spinlock in mm_struct to protect the concurrent > > > access to arg_start|end, env_start|end and others

Re: [v3 PATCH] mm: introduce arg_lock to protect arg_start|end and env_start|end in mm_struct

2018-04-10 Thread Cyrill Gorcunov
On Tue, Apr 10, 2018 at 11:09:17AM +0200, Michal Hocko wrote: > On Tue 10-04-18 05:52:54, Yang Shi wrote: > [...] > > So, introduce a new spinlock in mm_struct to protect the concurrent > > access to arg_start|end, env_start|end and others except start_brk and > > brk, which are still protected by

Re: [v3 PATCH] mm: introduce arg_lock to protect arg_start|end and env_start|end in mm_struct

2018-04-10 Thread Michal Hocko
On Tue 10-04-18 05:52:54, Yang Shi wrote: [...] > So, introduce a new spinlock in mm_struct to protect the concurrent > access to arg_start|end, env_start|end and others except start_brk and > brk, which are still protected by mmap_sem to avoid concurrent access > from do_brk(). Is there any

Re: [v3 PATCH] mm: introduce arg_lock to protect arg_start|end and env_start|end in mm_struct

2018-04-10 Thread Cyrill Gorcunov
On Tue, Apr 10, 2018 at 05:52:54AM +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