Re: Re: [RFC][PATCH -mm 5/6] Freezer: Use freezing timeout more efficiently

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

On Tuesday, 10 July 2007 20:50, Oleg Nesterov wrote:
> On 07/10, Rafael J. Wysocki wrote:
> >
> > +	if (todo && freeze_user_space && !time_after(jiffies, end_time)) {
> > +		/*
> > +		 * Some tasks have not been able to freeze.  They might be stuck
> > +		 * in TASK_UNINTERRUPTIBLE waiting for the frozen tasks.  Try to
> > +		 * thaw the tasks that have frozen without clearing the freeze
> > +		 * requests of the remaining tasks and repeat.
> > +		 */
> > +		read_lock(&tasklist_lock);
> > +		do_each_thread(g, p) {
> > +			if (frozen(p)) {
> > +				p->flags &= ~PF_FROZEN;
> > +				wake_up_process(p);
> 
> I just noticed we don't use thaw_process(), this means that the retry doesn't
> play well with wait_event_freezable() introduced in the previous patch.
> 
> Suppose that kthread_stop(T) hangs and blocks the freezer, and T does

The retry mechanism only applies to user land processes. :-)

> 	while (!kthread_should_stop()) {
> 		wait_event_freezable(...);
> 		do_something();
> 	}
> 
> and it is freezed. We clear PF_FROZEN but not TIF_FREEZE, wait_event_freezable
> checks freezing() and goes to refrigerator again.

PF_FROZEN and TIF_FREEZE are mutually exclusive as long as the thawing is
not racing with refrigerator().  I don't think that this is of concern here, but we can take
task_lock() in the freezing loop.

> Another problem is that we only count UNINTERRUPTIBLE tasks to make a decision
> about retry, while wait_event_freezable() sleeps NTERRUPTIBLE.

The retry thing doesn't cover kernel threads, because they aren't supposed to
block the freezer.  They are supposed to freeze voluntarily and to know
_exactly_ what they are doing.

Greetings,
Rafael


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