[PATCH] has_stopped_jobs: s/task_is_stopped/SIGNAL_STOP_STOPPED/

[PATCH] has_stopped_jobs: s/task_is_stopped/SIGNAL_STOP_STOPPED/

[Oleg Nesterov]

has_stopped_jobs() naively checks task_is_stopped(group_leader). This
was always wrong even without ptrace, group_leader can be dead. And
given that ptrace can change the state to TRACED this is wrong even
in the single-threaded case.

Change the code to check SIGNAL_STOP_STOPPED and simplify the code,
retval + break/continue doesn't make this trivial code more readable.

We could probably add the usual "|| signal->group_stop_count" check
but I don't think this makes sense, the task can start the group-stop
right after the check anyway.

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---

 kernel/exit.c |   12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

--- ptrace/kernel/exit.c~2_has_stopped_jobs	2011-06-28 17:50:27.000000000 +0200
+++ ptrace/kernel/exit.c	2011-07-07 21:09:00.000000000 +0200
@@ -266,18 +266,16 @@ int is_current_pgrp_orphaned(void)
 	return retval;
 }
 
-static int has_stopped_jobs(struct pid *pgrp)
+static bool has_stopped_jobs(struct pid *pgrp)
 {
-	int retval = 0;
 	struct task_struct *p;
 
 	do_each_pid_task(pgrp, PIDTYPE_PGID, p) {
-		if (!task_is_stopped(p))
-			continue;
-		retval = 1;
-		break;
+		if (p->signal->flags & SIGNAL_STOP_STOPPED)
+			return true;
 	} while_each_pid_task(pgrp, PIDTYPE_PGID, p);
-	return retval;
+
+	return false;
 }
 
 /*

Re: [PATCH] has_stopped_jobs: s/task_is_stopped/SIGNAL_STOP_STOPPED/

[Tejun Heo]

On Thu, Jul 07, 2011 at 09:33:54PM +0200, Oleg Nesterov wrote:
> has_stopped_jobs() naively checks task_is_stopped(group_leader). This
> was always wrong even without ptrace, group_leader can be dead. And
> given that ptrace can change the state to TRACED this is wrong even
> in the single-threaded case.
> 
> Change the code to check SIGNAL_STOP_STOPPED and simplify the code,
> retval + break/continue doesn't make this trivial code more readable.
> 
> We could probably add the usual "|| signal->group_stop_count" check
> but I don't think this makes sense, the task can start the group-stop
> right after the check anyway.
> 
> Signed-off-by: Oleg Nesterov <oleg@redhat.com>

 Acked-by: Tejun Heo <tj@kernel.org>

As for "|| signal->group_stop_count", it might be better to use the
same wrapper here too for consistency if we add one.

Thank you.

-- 
tejun