Message-ID: <Pine.LNX.4.44L0.0704191744380.6354-100000@iolanthe.rowland.org>
Date:	Thu, 19 Apr 2007 18:37:14 -0400 (EDT)
From:	Alan Stern <stern@...land.harvard.edu>
To:	Dmitry Torokhov <dmitry.torokhov@...il.com>
cc:	Tejun Heo <htejun@...il.com>,
	Cornelia Huck <cornelia.huck@...ibm.com>,
	linux-kernel <linux-kernel@...r.kernel.org>,
	Greg K-H <greg@...ah.com>,
	Rusty Russell <rusty@...tcorp.com.au>
Subject: Re: [PATCH RFD] alternative kobject release wait mechanism

On Thu, 19 Apr 2007, Dmitry Torokhov wrote:

> On 4/19/07, Alan Stern <stern@...land.harvard.edu> wrote:
> > On Wed, 18 Apr 2007, Dmitry Torokhov wrote:
> >
> > > I am still do not understand why this is needed. Would it not be
> > > simplier just to use a reference to struct device instead of embedding
> > > it in a larger structure if their lifetimes are different and one does
> > > not have a subsystem that takes care of releasing logic.
> > >
> > >
> > > Pretty much drivers have 2 options:
> > >
> > > struct my_device {
> > >         void *private_data;
> > >         struct device dev;
> > > };
> >
> > Actually people use dev_[gs]et_drvdata() instead of a separate
> > private_data pointer.  That way there's no need for the my_device
> > container.
> >
> 
> No, drvdata belongs to driver that is bound to a device. We are
> talking about private data created and managed by driver that provides
> device.

Oh, sorry, I misunderstood.

> Again, if we are talking about driver bound to a device then devices
> ->release() is irrelevant. If we are talking about driver that created
> device then driver's ->remove() is irrelevant.

The problem is that device structures carry references to their parents.  
So even if a driver does use this option and is able to give up its own 
references immediately, it can't make any guarantees about drivers of 
child devices.  If every driver followed your scheme we'd be okay; until 
then any ancestor of a device with a non-immediate-detach driver would not 
be able to do immediate detach.

There's also another problem.  Even though all the references to
my_dev->dev may be gone, there will probably still be outstanding
references to private_data structure.  The driver's exit() routine will 
have to wait until all those references are gone.  How can it do that 
safely?  Presumably the private_data will include its own refcount and 
some release routine will run when the count drops to 0.  That routine 
will be part of the driver's module -- so how can it enable itself to be 
unloaded?


> > > With current sysfs orphaning attributes upon removal request there is
> > > no issue of accessing driver-private data through references obtained
> > > via ether embedded or referenced dev structure so everything is fine.
> >
> > Not so.  There are other pathways besides sysfs which can cause a driver
> > to access its data structures.
> >
> 
> Which ones? These needs to be identified and treated with "immediate
> disconnect" that you advocated earlier. Once active users of device's
> services are gone you can zap it.

Among the worst offenders are character devices.  None of the subsystem
providers offering char device registration performs immediate detach --
they are a lot like sysfs used to be.  (In fact, they probably _can't_
provide it since read() or write() calls can block indefinitely in the
lower-level driver; the subsystem may have no way to force them to
fail-fast.)  So every lower-level driver which registers a char device
needs to provide its own synchronization and immediate detach, and at the
moment I doubt very many of them do.  And of those which do, a large
percentage rely on the BKL for synchronization!

Here's another awkward possibility.  Suppose a workqueue routine tries to 
unregister a device.  Suppose the device's driver needs to do something in 
a process context, so it has a work item pending in the same workqueue.  
The driver's remove() method would need to flush the workqueue (in order 
to perform immediate detach), but doing so would cause a deadlock.

Alan Stern

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

Powered by blists - more mailing lists