Re: [RFC][PATCH -mm 1/5] PM: Make freeze_processes SMP-safe

Pavel Machek
2006-11-29T23:55:23Z

Hi!

> > > I do not like the counting idea; it should be simpler to just check if
> > > all the processes are still stopped.
> >
> > I thought about that but didn't invent anything reasonable enough.
> >
> > > But I'm not sure if this is enough. What if signal is being delivered
> > > on another CPU while freezing, still being delivered while this second
> > > check runs, and then SIGCONT is delivered?
> >
> > Hm, is this possible in practice? I mean, if todo is 0 and nr_stopped doesn't
> > change, then there are no processes that can send the SIGCONT (unless someone
> > creates a kernel thread with PF_NOFREEZE that will do just that).
> >
> > Anyway, for now I've no idea how to fix this properly. Will think about it
> > tomorrow.
>
> As far as this particular problem is concerned, I think there are two possible
> solutions.
>
> One of them would be do disable the delivery of continuation signals before
> we start freezing processes, but I don't know how to do this exactly so that
> it's not racy. Also it would be quite intrusive.
>
> The other one may be something along with the lines of the appended patch.

There has to be a better solution. Stopped tasks are suspended
somewhere in kernel, right? One try_to_freeze() and problem should be
solved, in regular way, and without tricks...?

Pavel

>
> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> ---
> kernel/power/process.c | 22 +++++++++++++++++++---
> 1 file changed, 19 insertions(+), 3 deletions(-)
>
> Index: linux-2.6.19-rc6-mm2/kernel/power/process.c
> ===================================================================
> --- linux-2.6.19-rc6-mm2.orig/kernel/power/process.c 2006-11-28 22:49:30.000000000 +0100
> +++ linux-2.6.19-rc6-mm2/kernel/power/process.c 2006-11-29 00:10:05.000000000 +0100
> @@ -28,8 +28,7 @@ static inline int freezeable(struct task
> if ((p == current) ||
> (p->flags & PF_NOFREEZE) ||
> (p->exit_state == EXIT_ZOMBIE) ||
> - (p->exit_state == EXIT_DEAD) ||
> - (p->state == TASK_STOPPED))
> + (p->exit_state == EXIT_DEAD))
> return 0;
> return 1;
> }
> @@ -103,9 +102,13 @@ static unsigned int try_to_freeze_tasks(
> if (frozen(p))
> continue;
>
> + if (p->state == TASK_STOPPED && freezing(p))
> + continue;
> +
> if (p->state == TASK_TRACED &&
> (frozen(p->parent) ||
> - p->parent->state == TASK_STOPPED)) {
> + (p->parent->state == TASK_STOPPED &&
> + freezing(p->parent)))) {
> cancel_freezing(p);
> continue;
> }
> @@ -185,6 +188,18 @@ int freeze_processes(void)
> return 0;
> }
>
> +static void release_stopped_tasks(void)
> +{
> + struct task_struct *g, *p;
> +
> + read_lock(&tasklist_lock);
> + do_each_thread(g, p) {
> + if (p->state == TASK_STOPPED)
> + cancel_freezing(p);
> + } while_each_thread(g, p);
> + read_unlock(&tasklist_lock);
> +}
> +
> static void thaw_tasks(int thaw_user_space)
> {
> struct task_struct *g, *p;
> @@ -207,6 +222,7 @@ static void thaw_tasks(int thaw_user_spa
> void thaw_processes(void)
> {
> printk("Restarting tasks ... ");
> + release_stopped_tasks();
> thaw_tasks(FREEZER_KERNEL_THREADS);
> thaw_tasks(FREEZER_USER_SPACE);
> schedule();
>
> -------------------------------------------------------------------------
> 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
> _______________________________________________
> Suspend-devel mailing list
> Suspend-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/suspend-devel

--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html