On Thu, Apr 12, 2007 at 08:00:04PM +0400, Oleg Nesterov wrote:
> On 04/12, Srivatsa Vaddagiri wrote:
> >
> > On Tue, Apr 03, 2007 at 10:48:20PM +0530, Srivatsa Vaddagiri wrote:
> > > > Actually, we should do this before destroy_workqueue() calls
> > > > flush_workqueue().
> > > > Otherwise flush_cpu_workqueue() can hang forever in a similar manner.
> > >
> > > Yep. I guess these are a class of freezer deadlocks very similar to vfork
> > > parent waiting on child case. I get a feeling these should become common
> > > outside of kthread too (A waits on B for something, B gets frozen, which
> > > means A won't freeze causing freezer to fail). Can freezer detect this
> > > dependency somehow and thaw B automatically? Probably not that easy ..
> >
> > I wonder if there is some value in "enforcing" an order in which
> > processes get frozen i.e freeze A first before B. That may solve the
> > deadlocks we have been discussing wrt kthread_stop and flush_workqueue
> > as well.
>
> Perhaps we can add "atomic_t xxx" to task_struct.
>
> int freezing(struct task_struct *p)
> {
> return test_tsk_thread_flag(p, TIF_FREEZE)
> && atomic_read(&p->xxx) == 0;
> }
>
> void xxx_start(struct task_struct *p)
> {
> atomic_inc(p->xxx);
> thaw_process(p);
> }
>
> xxx_end(struct task_struct *p)
> {
> atomic_dec(p->xxx);
> }
>
> Now,
>
> xxx_start(p);
> ... wait for something which depends on p...
> xxx_end(p);
>
> Of course we need other changes, freeze_process() should check ->xxx, etc.
> I am not sure this makes sense.
I think this is racy.
Say, if the task which does xxx_start(p) [let's call it task q] is not
freezeable, and task p is
already frozen when q calls xxx_start, then we might be in a situation
where
- Freezer has declared the whole system to be frozen. Hence the thread
issuing the 'freeze'(suspend/hotplug) can go ahead and do whatever it wants
to.
- Task 'p' which was supposed to be frozen , is now running and
*surprise* We have a thread running on a cpu which ain't there any more!
I feel we need some kind of a global rwlock.
DEFINE_RWLOCK(freezer_status_lock);
int xxx_start(struct task_struct *p)
{
int ret = 0;
ret = read_trylock(&freezer_status_lock);
if(ret)
/* We've succeeded. So lets thaw the chap */
thaw_process(p);
/* If we've failed to acquire trylock, that means freezer doesn't
* depend on us.
* So lets simply wait without thawing p
*/
return ret;
}
void xxx_end(int state)
{
if(state)
read_unlock(&freezer_status_lock);
}
int try_to_freeze_tasks()
{
do {
/* The regular freezer code */
if (!todo && !write_trylock(&freezer_status_lock));
continue;
/* When the freezer goes back, it will find task 'p'
* woken up and hence wait for it to get frozen
*/
}while(todo);
}
void try_to_thaw_tasks()
{
.
.
.
write_unlock(&freezer_status_lock);
}
int state = xxx_start(p);
... wait for something which depends on p...
xxx_end(state);
However, now that I look at it again, I see that it will fail in our case
where we might need to nest the try_to_freeze_tasks call.
Hmm, we don't have a rwlock variant that allows multiple writers, now do
we?!
>
> Oleg.
>
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!"
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
