Messages in this thread | | | Date | Wed, 7 Aug 2019 16:26:10 +0200 | From | Oleg Nesterov <> | Subject | Re: [PATCH v3 1/2] fork: extend clone3() to support CLONE_SET_TID |
| |
On 08/06, Adrian Reber wrote: > > +struct pid *alloc_pid(struct pid_namespace *ns, int set_tid) > { > struct pid *pid; > enum pid_type type; > @@ -186,12 +186,35 @@ struct pid *alloc_pid(struct pid_namespace *ns) > if (idr_get_cursor(&tmp->idr) > RESERVED_PIDS) > pid_min = RESERVED_PIDS; > > - /* > - * Store a null pointer so find_pid_ns does not find > - * a partially initialized PID (see below). > - */ > - nr = idr_alloc_cyclic(&tmp->idr, NULL, pid_min, > - pid_max, GFP_ATOMIC); > + if (set_tid) { > + /* > + * Also fail if a PID != 1 is requested > + * and no PID 1 exists. > + */ > + if ((set_tid >= pid_max) || ((set_tid != 1) && > + (idr_is_empty(&tmp->idr)))) {
too many parentheses ;) this is purely cosmetic, up to you, but to me
if (set_tid >= pid_max || (set_tid != 1 && idr_is_empty(&tmp->idr))) {
looks a bit more readable.
> + spin_unlock_irq(&pidmap_lock); > + retval = -EINVAL; > + goto out_free;
This doesn't look right, you need idr_preload_end() before goto out_free.
But I'd suggest to simply do
nr = -EINVAL; if (set_tid < pid_max && (set_tid != 1 || idr_is_empty(&tmp->idr))) nr = idr_alloc(&tmp->idr, NULL, set_tid, set_tid + 1, GFP_ATOMIC);
...
this is more robust.
Oleg.
| |