Re: [RFC PATCH(experimental) 2/2] Fix freezer-kthread_stop race

Gautham R Shenoy
2007-04-20T12:26:13Z

On Fri, Apr 20, 2007 at 01:59:29PM +0200, Rafael J. Wysocki wrote:
>
> Actually, I thought about it for a while. The thread that is going to stop
> another one may temporarily mark itself as freezable in all cases, which
> will have no effect on it, since it's not going to cally try_to_freeze(), but
> will make the freezer wait for it. Next, after returning from
> wait_for_completion(), it should restore its old freezability status and that
> should make the freezer finish.

But that will have no affect if the thread to be stopped is already
frozen. The only affect might be that now, freezer will fail whether
the thread that is going to stop another one was freezeable or not.

Concern is whether we can somehow complete these wait_for_completion()'s in
the freezing context and reduce the probabilty of freezer failing.

>
> > However, I was attempting to solve the generic problem where
> > A waits on B and B is frozen. If A is freezeable (under one of the
> > events) then the freezer will fail. So a solution would be for A to
> > somehow inform B of the dependency and postpone it's freezing.
>
> Well, I think it might be simpler to consider each case separately. This way
> we may be able to avoid introducing the additional TIF_ flag.
>

Makes sense.

> > Since akpm mentioned that flush_workqueue() needs to go, I guess, I am
> > ok with fixing only kthread_stop/kthread_should_stop for the moment.
> > Unless I can spot any other valid case :)
>
> Sure. :-)
>
> BTW, if it turns out that we need to introduce yet another freezer-related
> TIF_ flag, it may be acceptable (?) to move all of the freezer-related flags
> into a separate member of task_struct (eg. freezer_flags) that can only be
> manipulated under task_lock().
>
> I mean, we already have four of them (PF_NOFREEZE, PF_FROZEN,
> PF_FREEZER_SKIP, TIF_FREEZE), and you will need to introduce two more for
> the freezer-based CPU hotplug, so if yet another one is needed, that will make
> up almost a separate u8 field ...

I am perfectly ok with it. But I am not sure if everybody would agree to have
another field in the task struct, though in this case it does make sense :-)

>
> Greetings,
> Rafael
> -

Thanks and Regards
gautham.
--
Gautham R Shenoy
Linux Technology Center
IBM India.
"Freedom comes with a price tag of responsibility, which is still a bargain,
because Freedom is priceless!"