Messages in this thread | | | Date | Mon, 2 Oct 2017 17:21:35 +0200 | From | Oleg Nesterov <> | Subject | Re: [PATCH v2 2/2] pid: Remove pidhash |
| |
Hi Rik,
On 10/02, Rik van Riel wrote: > > Gargi and I are looking at that code, and trying to figure out > exactly what needs to be done to make all of this correct.
see another email I sent to Gargi a minute ago,
> 2) With pid_ns_prepare_proc out of the way, we can put all the code > from below where the call to pid_ns_prepare_proc is now (except > error handing) into the main loop of pid allocation, so we can > do all that stuff under the pidmap_lock: > > for (i = ns->level; i >= 0; i--) { > ... > idr_alloc_cyclic(...) > get_pid_ns(ns); > atomic_set(&pid->count, 1); > for (...) > INIT_HLIST_HEAD(...) > ns->nr_allocated++; > ... > }
I do not see how this can fix the problem with not-fully-initialized pid returned by find_pid_ns().
As for PIDNS_ADDING/PIDNS_HASH_ADDING, _perhaps_ we can cleanup this logic a bit and do the check earlier, but imo this needs another/separate change.
I'd suggest to keep the current logic and the order of initialization and just do
for (i = ns->level; i >= 0; i--) { ...
// do not expose the new pid to find_pid_ns() until it // is fully initialized nr = idr_alloc_cyclic(&tmp->idr, /*pid*/ NULL, ...); ... }
...
spin_lock_irq(&pidmap_lock); if (!(ns->nr_hashed & PIDNS_HASH_ADDING)) goto out_unlock; for ( ; upid >= pid->numbers; --upid) { - hlist_add_head_rcu(&upid->pid_chain, - &pid_hash[pid_hashfn(upid->nr, upid->ns)]); + // finally make it visible to find_pid_ns() + idr_replace(upid->ns-idr, pid, upid->nr); upid->ns->nr_hashed++; } spin_unlock_irq(&pidmap_lock);
Or I missed something?
Oleg.
| |