Eric W. Biederman [ebiederm@xxxxxxxxxxxx] wrote: | | Changing zap_pid_ns_processes to fix the problem instead of | changing the code elsewhere is one of the few solutions I have | seen that does not increase the cost of the lat_proc test from | lmbench. I think its a good fix for the problem. but I have a nit and a minor comment below. Thanks, Sukadev | | Reported-by: Louis Rilling <Louis.Rilling@xxxxxxxxxxx> Reviewed-by: Sukadev Bhattiprolu <sukadev@xxxxxxxxxxxxxxxxxx> | Signed-off-by: Eric W. Biederman <ebiederm@xxxxxxxxxxxx> | --- | kernel/pid_namespace.c | 50 ++++++++++++++++++++++++++++++++++++----------- | 1 files changed, 38 insertions(+), 12 deletions(-) | | diff --git a/kernel/pid_namespace.c b/kernel/pid_namespace.c | index a5aff94..aaf2ab0 100644 | --- a/kernel/pid_namespace.c | +++ b/kernel/pid_namespace.c | @@ -139,16 +139,20 @@ void free_pid_ns(struct kref *kref) | | void zap_pid_ns_processes(struct pid_namespace *pid_ns) | { | + struct task_struct *me = current; | int nr; | int rc; | struct task_struct *task; | | /* | - * The last thread in the cgroup-init thread group is terminating. | - * Find remaining pid_ts in the namespace, signal and wait for them | - * to exit. | + * The last task in the pid namespace-init threa group is terminating. nit: thread | + * Find remaining pids in the namespace, signal and wait for them | + * to to be reaped. | * | - * Note: This signals each threads in the namespace - even those that | + * By waiting for all of the tasks to be reaped before init is reaped | + * we provide the invariant that no task can escape the pid namespace. | + * | + * Note: This signals each task in the namespace - even those that | * belong to the same thread group, To avoid this, we would have | * to walk the entire tasklist looking a processes in this | * namespace, but that could be unnecessarily expensive if the | @@ -157,28 +161,50 @@ void zap_pid_ns_processes(struct pid_namespace *pid_ns) | * | */ | read_lock(&tasklist_lock); | - nr = next_pidmap(pid_ns, 1); | - while (nr > 0) { | - rcu_read_lock(); | + for (nr = next_pidmap(pid_ns, 0); nr > 0; nr = next_pidmap(pid_ns, nr)) { Is it necessary to start the search at nr == 0 ? We will find nr == 1 first and then immediately skip over it - bc same_thread_group() will be TRUE. | | /* | * Any nested-container's init processes won't ignore the | * SEND_SIG_NOINFO signal, see send_signal()->si_fromuser(). | */ | - task = pid_task(find_vpid(nr), PIDTYPE_PID); | - if (task) | + rcu_read_lock(); | + task = pid_task(find_pid_ns(nr, pid_ns), PIDTYPE_PID); | + if (task && !same_thread_group(task, me)) | send_sig_info(SIGKILL, SEND_SIG_NOINFO, task); Also, if we start the search at 1, we could skip the only the other possible thread in the group with (nr != my_pid_nr) but its not really an optimization. _______________________________________________ Containers mailing list Containers@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linux-foundation.org/mailman/listinfo/containers