Re: [PATCH v2 1/2] fork: extend clone3() to support CLONE_SET_TID

2019-08-02 Thread Adrian Reber
On Fri, Aug 02, 2019 at 03:52:49PM +0200, Christian Brauner wrote: > On Fri, Aug 02, 2019 at 03:46:11PM +0200, Oleg Nesterov wrote: > > On 08/02, Oleg Nesterov wrote: > > > > > > So Adrian, sorry for confusion, I think your patch is fine. Good to know. > > Yes... but do we really need the new

Re: [PATCH v2 1/2] fork: extend clone3() to support CLONE_SET_TID

2019-08-02 Thread Adrian Reber
On Fri, Aug 02, 2019 at 03:50:54PM +0200, Christian Brauner wrote: > On Fri, Aug 02, 2019 at 03:30:01PM +0200, Oleg Nesterov wrote: > > On 08/02, Christian Brauner wrote: > > > > > > On Wed, Jul 31, 2019 at 06:12:22PM +0200, Adrian Reber wrote: > > > > The main motivation to add CLONE_SET_TID to

Re: [PATCH v2 1/2] fork: extend clone3() to support CLONE_SET_TID

2019-08-02 Thread Christian Brauner
On Fri, Aug 02, 2019 at 03:46:11PM +0200, Oleg Nesterov wrote: > On 08/02, Oleg Nesterov wrote: > > > > So Adrian, sorry for confusion, I think your patch is fine. > > Yes... but do we really need the new CLONE_SET_TID ? > > set_tid == 0 has no effect, can't we simply check kargs->set_tid != 0 >

Re: [PATCH v2 1/2] fork: extend clone3() to support CLONE_SET_TID

2019-08-02 Thread Christian Brauner
On Fri, Aug 02, 2019 at 03:30:01PM +0200, Oleg Nesterov wrote: > On 08/02, Christian Brauner wrote: > > > > On Wed, Jul 31, 2019 at 06:12:22PM +0200, Adrian Reber wrote: > > > The main motivation to add CLONE_SET_TID to clone3() is CRIU. > > > > > > To restore a process with the same PID/TID CRIU

Re: [PATCH v2 1/2] fork: extend clone3() to support CLONE_SET_TID

2019-08-02 Thread Oleg Nesterov
On 08/02, Oleg Nesterov wrote: > > So Adrian, sorry for confusion, I think your patch is fine. Yes... but do we really need the new CLONE_SET_TID ? set_tid == 0 has no effect, can't we simply check kargs->set_tid != 0 before ns_capable() ? and to remind, I still think alloc_pid() should use

Re: [PATCH v2 1/2] fork: extend clone3() to support CLONE_SET_TID

2019-08-02 Thread Oleg Nesterov
On 08/02, Christian Brauner wrote: > > On Wed, Jul 31, 2019 at 06:12:22PM +0200, Adrian Reber wrote: > > The main motivation to add CLONE_SET_TID to clone3() is CRIU. > > > > To restore a process with the same PID/TID CRIU currently uses > > /proc/sys/kernel/ns_last_pid. It writes the desired (PID

Re: [PATCH v2 1/2] fork: extend clone3() to support CLONE_SET_TID

2019-08-02 Thread Oleg Nesterov
On 08/02, Oleg Nesterov wrote: > > On 08/02, Adrian Reber wrote: > > > > On Wed, Jul 31, 2019 at 07:41:36PM +0200, Oleg Nesterov wrote: > > > But the main question is how it can really help if ns->level > 0, unlikely > > > CRIU will ever need to clone the process with the same pid_nr == set_tid >

Re: [PATCH v2 1/2] fork: extend clone3() to support CLONE_SET_TID

2019-08-02 Thread Christian Brauner
On Wed, Jul 31, 2019 at 06:12:22PM +0200, Adrian Reber wrote: > The main motivation to add CLONE_SET_TID to clone3() is CRIU. > > To restore a process with the same PID/TID CRIU currently uses > /proc/sys/kernel/ns_last_pid. It writes the desired (PID - 1) to > ns_last_pid and then (quickly) does

Re: [PATCH v2 1/2] fork: extend clone3() to support CLONE_SET_TID

2019-08-02 Thread Christian Brauner
On Fri, Aug 02, 2019 at 02:47:38PM +0200, Oleg Nesterov wrote: > On 08/02, Adrian Reber wrote: > > > > On Wed, Jul 31, 2019 at 07:41:36PM +0200, Oleg Nesterov wrote: > > > But the main question is how it can really help if ns->level > 0, unlikely > > > CRIU will ever need to clone the process with

Re: [PATCH v2 1/2] fork: extend clone3() to support CLONE_SET_TID

2019-08-02 Thread Oleg Nesterov
On 08/02, Adrian Reber wrote: > > On Wed, Jul 31, 2019 at 07:41:36PM +0200, Oleg Nesterov wrote: > > But the main question is how it can really help if ns->level > 0, unlikely > > CRIU will ever need to clone the process with the same pid_nr == set_tid > > in the ns->parent chain. > > Not sure I

Re: [PATCH v2 1/2] fork: extend clone3() to support CLONE_SET_TID

2019-08-02 Thread Adrian Reber
On Wed, Jul 31, 2019 at 07:41:36PM +0200, Oleg Nesterov wrote: > On 07/31, Adrian Reber wrote: > > > > Extending clone3() to support CLONE_SET_TID makes it possible restore a > > process using CRIU without accessing /proc/sys/kernel/ns_last_pid and > > race free (as long as the desired PID/TID is

Re: [PATCH v2 1/2] fork: extend clone3() to support CLONE_SET_TID

2019-07-31 Thread Oleg Nesterov
On 07/31, Adrian Reber wrote: > > Extending clone3() to support CLONE_SET_TID makes it possible restore a > process using CRIU without accessing /proc/sys/kernel/ns_last_pid and > race free (as long as the desired PID/TID is available). I personally like this... but please see the question below.

Re: [PATCH v2 1/2] fork: extend clone3() to support CLONE_SET_TID

2019-07-31 Thread Dmitry Safonov
On 7/31/19 5:49 PM, Dmitry Safonov wrote: > Hi Adrian, > > On 7/31/19 5:12 PM, Adrian Reber wrote: > [..] >> @@ -2530,14 +2530,12 @@ noinline static int copy_clone_args_from_user(struct >> kernel_clone_args *kargs, >>struct clone_args __user *uargs, >>

Re: [PATCH v2 1/2] fork: extend clone3() to support CLONE_SET_TID

2019-07-31 Thread Dmitry Safonov
Hi Adrian, On 7/31/19 5:12 PM, Adrian Reber wrote: [..] > @@ -2530,14 +2530,12 @@ noinline static int copy_clone_args_from_user(struct > kernel_clone_args *kargs, > struct clone_args __user *uargs, > size_t

[PATCH v2 1/2] fork: extend clone3() to support CLONE_SET_TID

2019-07-31 Thread Adrian Reber
The main motivation to add CLONE_SET_TID to clone3() is CRIU. To restore a process with the same PID/TID CRIU currently uses /proc/sys/kernel/ns_last_pid. It writes the desired (PID - 1) to ns_last_pid and then (quickly) does a clone(). This works most of the time, but it is racy. It is also slow