Re: [RFC][PATCH -mm 2/2] Freezer: Use wait queue instead of busy looping

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

On Wednesday, 25 July 2007 15:29, Oleg Nesterov wrote:
> On 07/25, Rafael J. Wysocki wrote:
> >
> >  void refrigerator(void)
> >  {
> > @@ -50,6 +73,9 @@ void refrigerator(void)
> >  	   processes around? */
> >  	long save;
> >
> > +	refrigerator_called = 1;
> > +	wake_up(&refrigerator_waitq);
> > +
> 
> This is a bit racy. Unless I missed something, the task should not set
> refrigerator_called == 1 until it has PF_FROZEN.

No, it's just to signal that the task has entered the refrigerator, not that
it has actually frozen.

> Otherwise, try_to_freeze_tasks() can set refrigerator_called == 0 after
> refrigerator() sets it == 1, the the main loop notices this unfrozen task,
> and goes to sleep.

refrigerator_called is only reset after try_to_freeze_tasks() has found it
equal to one.  There is only a small window between checking it in
wait_event_timeout() and resetting it, but then we go to send freeze requests
to the remaining tasks and we count 'todo' from the start, so that shouldn't
be a problem.

> (I must admit, I agree with Pavel on that patch).

Well, I think it's a good idea (otherwise I wouldn't have posted it ;-)) and
the OLPC people really had a problem with try_to_freeze_tasks() looping too
fast (they reniced it to avoid this problem, but IMO the $subject patch is
nicer than that).

Greetings,
Rafael


-- 
"Premature optimization is the root of all evil." - Donald Knuth