Re: [PATCH -mm 1/2]: PM: Fix handling of stopped tasks

["Rafael J. Wysocki" <rjw@sisk.pl>]

Hi,

On Tuesday, 5 December 2006 11:34, Pavel Machek wrote:
> Hi!
> 
> > Currently, if a task is stopped (ie. it's in the TASK_STOPPED state), it is
> > considered by the freezer as unfreezeable.  However, there may be a race
> > between the freezer and the delivery of the continuation signal to the task
> > resulting in the task running after we have finished freezing other tasks.
> > This, in turn, may lead to undesirable effects up to and including a
> > corruption of data.
> > 
> > To prevent this from happening we first need to make the freezer consider
> > stopped tasks as freezeable.  For this purpose we need to make freezeable()
> > stop returning 0 for these tasks.  We must remember, however, that the
> > stopped tasks need not receive the continuation signal before thaw_processes()
> > is called, so as soon as PF_FREEZE is set for them try_to_freeze_tasks()
> > should stop counting them as the ones to wait for.  Additionally, if there's a
> > traced task (ie. a task in the TASK_TRACED state) the parent of which has
> > PF_FREEZE set and is stopped, try_to_freeze_tasks() should not wait for it.
> > Moreover, if there are some stopped tasks that haven't received the continuation
> > signal before thaw_processes() is called, we must clear PF_FREEZE for them so
> > that they don't go to the refrigerator when it's no longer desirable.
> 
> Actually, what do you think about this patch? It removes special
> handling of TASK_TRACED, and should do the trick, too...

Well, I don't think so, ....

> 								Pavel
> 
> diff --git a/kernel/power/process.c b/kernel/power/process.c
> index 7bcc976..d56e494 100644
> --- a/kernel/power/process.c
> +++ b/kernel/power/process.c
> @@ -26,8 +26,7 @@ static inline int freezeable(struct task
>  	    (p->flags & PF_NOFREEZE) ||
>  	    (p->exit_state == EXIT_ZOMBIE) ||
>  	    (p->exit_state == EXIT_DEAD) ||
> -	    ((p->exit_state == TASK_TRACED) && frozen(p->parent)) ||
> -	    (p->state == TASK_STOPPED))
> +	    ((p->exit_state == TASK_TRACED) && frozen(p->parent)))
>  		return 0;
>  	return 1;
>  }
> diff --git a/kernel/signal.c b/kernel/signal.c
> index 9a61944..e305ad1 100644
> --- a/kernel/signal.c
> +++ b/kernel/signal.c
> @@ -1702,7 +1702,9 @@ finish_stop(int stop_count)
>  		read_unlock(&tasklist_lock);
>  	}
>  
> -	schedule();
> +	do {
> +		schedule();
> +	} while (try_to_freeze());
>  	/*
>  	 * Now we don't run again until continued.
>  	 */

... because if you want try_to_freeze() here to trigger, then the task in
question should have PF_FREEZE set, but we don't set it anywhere.
[Actually I think this particular change is equivalent to moving the
try_to_freeze() in get_signal_to_deliver() to after the relock label. :-)]

And the special handling of traced tasks is needed to clear TIF_SIGPENDING
for a task that had been told to freeze before it's parent froze.

Greetings,
Rafael


-- 
If you don't have the time to read,
you don't have the time or the tools to write.
		- Stephen King

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV