Re: 2.6.21-rc suspend regression: sysfs deadlock

Alan Stern
2007-03-14T19:23:13Z

On Wed, 14 Mar 2007, Cornelia Huck wrote:

> On Wed, 14 Mar 2007 12:12:37 -0400 (EDT),
> Alan Stern <stern@rowland.harvard.edu> wrote:
>
> > This seems more elegant (not yet tested). Cornelia, does it look okay to
> > you?
>
> Works for me (grouping & ungrouping ctc) and looks sane. Some more
> comments below.

Thank you.

> > +struct other_task_struct {
> > + struct kobject *kobj;
> > + void (*func)(void *);
> > + void *data;
> > + struct work_struct work;
> > +};
> > +
> > +static void other_task_work(struct work_struct *work)
> > +{
> > + struct other_task_struct *ots = container_of(work,
> > + struct other_task_struct, work);
> > +
> > + (ots->func)(ots->data);
> > + kobject_put(ots->kobj);
> > + kfree(ots);
> > +}
>
> The naming seems a bit unintuitive, but I don't have a good
> alternative idea. Perhaps sysfs_work_struct, sysfs_delayed_work()?

sysfs_work_struct is too generic; other parts of sysfs might also want to
use workqueues for different purposes.

I don't like calling it "delayed"-anything, because the operations aren't
necessarily delayed! On an SMP system they might even execute before the
sysfs_access_in_other_task() call returns. (Although the two examples we
have so far can't do that because of lock contention.)

The major feature added here is that the work takes place in a different
task's context, not that it is delayed. Hence the choice of names.

Alan Stern