lkml.org 
[lkml]   [2012]   [May]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
Subject[PATCH -mm v2] pidns-guarantee-that-the-pidns-init-will-be-the-last-pidns-process-r eaped-v2-fix-fix
On 05/25, Eric W. Biederman wrote:
>
> Oleg Nesterov <oleg@redhat.com> writes:
>
> > 1. Update the comments in zap_pid_ns_processes() and __unhash_process()
>
> In zap_pid_ns_processes I wonder if we should update the big block
> comment with a little more of the theory. AKA we want as many children
> to self-reap and become EXIT_DEAD children as possible becasue it
> enables more parallelism and is thus faster.

Yes, the comment can be better, I agree.

Ideally it should explain that we need the sys_wait4() loop even if
we ignore SIGCHLD (with your patch), but at the same time we need
the wait-for-empty loop even if SIGCHLD is not ignored.

OK, I tried to make it a bit better, see below. Feel free to rewrite.

> > 2. Move the wake-up-reaper code in __unhash_process() under IS_ENABLED()
>
> I don't really care, it ceartainly looks better than an #ifdef block.
> However come to think of it, it is about time to just plain start
> removing those config options.

Probably, I do not mind if we remove CONFIG_PID_NS. But until we do this,
I think IS_ENABLED(CONFIG_PID_NS) makes sense as a documentation.

> > 3. Re-structure the wait-for-empty-children code in zap_pid_ns_processes()
> The restructuring seems basically sane.

Good.

> > + * reaped, notify the child_reaper, see zap_pid_ns_processes().
> > */
>
> How about instead:
> > /*
> > * If we are the last child process in a pid namespace to be
> > - * reaped, notify the child_reaper.
> > + * reaped, wake up the child_reaper sleeping in zap_pid_ns_processes().

OK.

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---
kernel/exit.c | 17 +++++++++--------
kernel/pid_namespace.c | 22 +++++++++++++++-------
2 files changed, 24 insertions(+), 15 deletions(-)
diff --git a/kernel/exit.c b/kernel/exit.c
index 231decb..6d66cd2 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -65,8 +65,6 @@ static void __unhash_process(struct task_struct *p, bool group_dead)
{
nr_threads--;
if (group_dead) {
- struct task_struct *parent;
-
detach_pid(p, PIDTYPE_PGID);
detach_pid(p, PIDTYPE_SID);

@@ -76,13 +74,16 @@ static void __unhash_process(struct task_struct *p, bool group_dead)

/*
* If we are the last child process in a pid namespace to be
- * reaped, notify the child_reaper.
+ * reaped, notify the reaper sleeping zap_pid_ns_processes().
*/
- parent = p->real_parent;
- if ((task_active_pid_ns(p)->child_reaper == parent) &&
- list_empty(&parent->children) &&
- (parent->flags & PF_EXITING))
- wake_up_process(parent);
+ if (IS_ENABLED(CONFIG_PID_NS)) {
+ struct task_struct *parent = p->real_parent;
+
+ if ((task_active_pid_ns(p)->child_reaper == parent) &&
+ list_empty(&parent->children) &&
+ (parent->flags & PF_EXITING))
+ wake_up_process(parent);
+ }
}
detach_pid(p, PIDTYPE_PID);
list_del_rcu(&p->thread_group);
diff --git a/kernel/pid_namespace.c b/kernel/pid_namespace.c
index 723c948..41ed867 100644
--- a/kernel/pid_namespace.c
+++ b/kernel/pid_namespace.c
@@ -179,22 +179,30 @@ void zap_pid_ns_processes(struct pid_namespace *pid_ns)
}
read_unlock(&tasklist_lock);

+ /* Firstly reap the EXIT_ZOMBIE children we may have. */
do {
clear_thread_flag(TIF_SIGPENDING);
rc = sys_wait4(-1, NULL, __WALL, NULL);
} while (rc != -ECHILD);

- read_lock(&tasklist_lock);
+ /*
+ * sys_wait4() above can't reap the TASK_DEAD children.
+ * Make sure they all go away, see __unhash_process().
+ */
for (;;) {
- __set_current_state(TASK_UNINTERRUPTIBLE);
- if (list_empty(&current->children))
- break;
+ bool need_wait = false;
+
+ read_lock(&tasklist_lock);
+ if (!list_empty(&current->children)) {
+ __set_current_state(TASK_UNINTERRUPTIBLE);
+ need_wait = true;
+ }
read_unlock(&tasklist_lock);
+
+ if (!need_wait)
+ break;
schedule();
- read_lock(&tasklist_lock);
}
- read_unlock(&tasklist_lock);
- set_current_state(TASK_RUNNING);

if (pid_ns->reboot)
current->signal->group_exit_code = pid_ns->reboot;
--
1.5.5.1



\
 
 \ /
  Last update: 2012-05-27 21:21    [from the cache]
©2003-2014 Jasper Spaans. Advertise on this site