Re: 2.6.11-rc2-mm1

[Dmitry Torokhov]

On Tue, 25 Jan 2005 18:24:50 +0300, Evgeniy Polyakov
<johnpol@xxxxxxxxxxx> wrote:
> On Tue, 2005-01-25 at 14:23 +0000, Christoph Hellwig wrote:
> > > > +static void pc8736x_fini(void)
> > > > +{
> > > > + sc_del_sc_dev(&pc8736x_dev);
> > > > +
> > > > + while (atomic_read(&pc8736x_dev.refcnt)) {
> > > > +         printk(KERN_INFO "Waiting for %s to became free: refcnt=%d.\n",
> > > > +                         pc8736x_dev.name, atomic_read(&pc8736x_dev.refcnt));
> > > > +
> > > > +         set_current_state(TASK_INTERRUPTIBLE);
> > > > +         schedule_timeout(HZ);
> > > > +
> > > > +         if (current->flags & PF_FREEZE)
> > > > +                 refrigerator(PF_FREEZE);
> > > > +
> > > > +         if (signal_pending(current))
> > > > +                 flush_signals(current);
> > > > + }
> > > > +}
> > > >
> > > > And who gurantess this won't deadlock?  Please use a dynamically allocated
> > > > driver model device and it's refcounting, thanks.
> > >
> > > Sigh.
> > >
> > > Christoph, please read the code before doing such comments.
> > > I very respect your review and opinion, but only until you respect
> > > others.
> >
> > The code above pretty much means you can keep rmmod stalled forever.
> 
> Yes, and it is better than removing module whose structures are in use.
> SuperIO core is asynchronous in it's nature, one can use logical device
> through superio core and remove it's module on other CPU, above loop
> will wait untill all reference counters are dropped.

I have a slightly different concern - the superio is a completely new
subsystem and it should be integtrated with the driver model
("superio" bus?). Right now it looks like it is reimplementing most of
the abstractions (device lists, driver lists, matching, probing).
Moving to driver model significatntly affects lifetime rules for the
objects, etc. etc. and will definitely not allow code such as above.

It would be nice it we get things right from the start.

-- 
Dmitry
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/