BUG(?): class_device_driver_link()

BUG(?): class_device_driver_link()

[Alan Stern]

Greg:

I'm not sure if this is a bug or not, but it is inconsistent behavior in 
sysfs.

When a class_device is added, if it has a regular device associated with
it and that device has a driver, a symlink is added from the class_device
to the driver.  However, if the class_device is added _first_ and the
driver later, this symlink is not created.  It's not clear that there's
any good way to create it, especially if the class_device is added by the
bus layer and the device driver itself is unaware of the class_device.

Is this a known problem?  It definitely affects the sd driver, and maybe 
others.

There is a related side-effect that is a bit unpleasant.  The symlink from
the class_device to the driver increments the driver's refcount.  Since
the driver is unaware of the class_device, it doesn't know to remove the
symlink when its release() method runs.  Consequently, if the driver is
modprobed before the device exists and rmmod'ed after the device is added,
the rmmod will hang until the device also goes away.  But if the driver is 
modprobed _after_ the device exists then the rmmod will complete 
immediately.

Perhaps the answer is that the bus layer must take these things into 
account.  Or perhaps a struct device should somehow know about all the 
class_devices that refer to it.

Alan Stern

Re: BUG(?): class_device_driver_link()

[Greg KH]

On Fri, Jun 18, 2004 at 04:12:32PM -0400, Alan Stern wrote:
> Greg:
> 
> I'm not sure if this is a bug or not, but it is inconsistent behavior in 
> sysfs.
> 
> When a class_device is added, if it has a regular device associated with
> it and that device has a driver, a symlink is added from the class_device
> to the driver.  However, if the class_device is added _first_ and the
> driver later, this symlink is not created.  It's not clear that there's
> any good way to create it, especially if the class_device is added by the
> bus layer and the device driver itself is unaware of the class_device.

Yes, this is the way it was designed.  The thinking was that a device
would be registered to a driver by the time the class code was
initialized for it.

> Is this a known problem?  It definitely affects the sd driver, and maybe 
> others.

The sd use of classes is a monumental hack.  So much that every time I
see one of them in the hall at work I run the other way just to avoid
talking about it again :)

No, seriously, if this is a problem for the sd driver, we should fix it.

> There is a related side-effect that is a bit unpleasant.  The symlink from
> the class_device to the driver increments the driver's refcount.

Yes, that is a recent change.

> Since
> the driver is unaware of the class_device, it doesn't know to remove the
> symlink when its release() method runs.

The symlink should be removed by the class, right?

> Consequently, if the driver is
> modprobed before the device exists and rmmod'ed after the device is added,
> the rmmod will hang until the device also goes away.  But if the driver is 
> modprobed _after_ the device exists then the rmmod will complete 
> immediately.
> 
> Perhaps the answer is that the bus layer must take these things into 
> account.  Or perhaps a struct device should somehow know about all the 
> class_devices that refer to it.

Hm, I was not wanting to have to have struct device know about classes,
only classes knowing about struct devices.  Any input here would be
appreciated.

thanks,

greg k-h

Re: BUG(?): class_device_driver_link()

[Alan Stern]

On Fri, 18 Jun 2004, Greg KH wrote:

> On Fri, Jun 18, 2004 at 04:12:32PM -0400, Alan Stern wrote:
> > Greg:
> > 
> > I'm not sure if this is a bug or not, but it is inconsistent behavior in 
> > sysfs.
> > 
> > When a class_device is added, if it has a regular device associated with
> > it and that device has a driver, a symlink is added from the class_device
> > to the driver.  However, if the class_device is added _first_ and the
> > driver later, this symlink is not created.  It's not clear that there's
> > any good way to create it, especially if the class_device is added by the
> > bus layer and the device driver itself is unaware of the class_device.
> 
> Yes, this is the way it was designed.  The thinking was that a device
> would be registered to a driver by the time the class code was
> initialized for it.

That seems reasonable, but obviously it doesn't work if the driver is
loaded much later.  I don't know what happens when the driver is loaded by
the hotplug system following a device attach, for example, but probably
that won't work either.

> > Is this a known problem?  It definitely affects the sd driver, and maybe 
> > others.
> 
> The sd use of classes is a monumental hack.  So much that every time I
> see one of them in the hall at work I run the other way just to avoid
> talking about it again :)
> 
> No, seriously, if this is a problem for the sd driver, we should fix it.

I'm not sure that it's a problem -- all that happens is the "driver"
symlink under the class_device's directory isn't present.  Also, it will 
affect every SCSI device, not just SCSI disks, since the class_device 
management is done by the central SCSI core.

> > There is a related side-effect that is a bit unpleasant.  The symlink from
> > the class_device to the driver increments the driver's refcount.
> 
> Yes, that is a recent change.

Okay, that explains why I never used to have these difficulties.

> > Since
> > the driver is unaware of the class_device, it doesn't know to remove the
> > symlink when its release() method runs.
> 
> The symlink should be removed by the class, right?

Sure.  But when you rmmod the driver, the class doesn't know that anything
has happened.  There is no link from the driver to the class_device; the
links all go the other way.

> > Consequently, if the driver is
> > modprobed before the device exists and rmmod'ed after the device is added,
> > the rmmod will hang until the device also goes away.  But if the driver is 
> > modprobed _after_ the device exists then the rmmod will complete 
> > immediately.
> > 
> > Perhaps the answer is that the bus layer must take these things into 
> > account.  Or perhaps a struct device should somehow know about all the 
> > class_devices that refer to it.
> 
> Hm, I was not wanting to have to have struct device know about classes,
> only classes knowing about struct devices.  Any input here would be
> appreciated.

I don't know what the best approach is.  Linus would probably say that 
having rmmod hang until the device is gone isn't a problem since unloading 
drivers is only done for development.  As a developer, I find it somewhat 
inconvenient.

Perhaps James will have some ideas about how changes to the SCSI core 
might improve the situation.

Alan Stern

Re: BUG(?): class_device_driver_link()

[Greg KH]

On Fri, Jun 18, 2004 at 05:36:52PM -0400, Alan Stern wrote:
> On Fri, 18 Jun 2004, Greg KH wrote:
> 
> > On Fri, Jun 18, 2004 at 04:12:32PM -0400, Alan Stern wrote:
> > > Greg:
> > > 
> > > I'm not sure if this is a bug or not, but it is inconsistent behavior in 
> > > sysfs.
> > > 
> > > When a class_device is added, if it has a regular device associated with
> > > it and that device has a driver, a symlink is added from the class_device
> > > to the driver.  However, if the class_device is added _first_ and the
> > > driver later, this symlink is not created.  It's not clear that there's
> > > any good way to create it, especially if the class_device is added by the
> > > bus layer and the device driver itself is unaware of the class_device.
> > 
> > Yes, this is the way it was designed.  The thinking was that a device
> > would be registered to a driver by the time the class code was
> > initialized for it.
> 
> That seems reasonable, but obviously it doesn't work if the driver is
> loaded much later.

The driver is the piece of code that creates a class device, so that can
not happen (scsi model excluded).

> > > Is this a known problem?  It definitely affects the sd driver, and maybe 
> > > others.
> > 
> > The sd use of classes is a monumental hack.  So much that every time I
> > see one of them in the hall at work I run the other way just to avoid
> > talking about it again :)
> > 
> > No, seriously, if this is a problem for the sd driver, we should fix it.
> 
> I'm not sure that it's a problem -- all that happens is the "driver"
> symlink under the class_device's directory isn't present.  Also, it will 
> affect every SCSI device, not just SCSI disks, since the class_device 
> management is done by the central SCSI core.
> 
> > > There is a related side-effect that is a bit unpleasant.  The symlink from
> > > the class_device to the driver increments the driver's refcount.
> > 
> > Yes, that is a recent change.
> 
> Okay, that explains why I never used to have these difficulties.
> 
> > > Since
> > > the driver is unaware of the class_device, it doesn't know to remove the
> > > symlink when its release() method runs.
> > 
> > The symlink should be removed by the class, right?
> 
> Sure.  But when you rmmod the driver, the class doesn't know that anything
> has happened.  There is no link from the driver to the class_device; the
> links all go the other way.

Again, the driver owns the class device.  scsi has something wrong
again.  Time to stop avoiding everyone at work...

thanks,

greg k-h

Re: BUG(?): class_device_driver_link()

[James Bottomley]

On Fri, 2004-06-18 at 19:03, Greg KH wrote:
> Again, the driver owns the class device.  scsi has something wrong
> again.  Time to stop avoiding everyone at work...

Well, the SCSI model for using these things isn't exactly the same as a
more standard entity like a PCI device.

For every SCSI device the mid-layer scans, we allocate a generic device.

We have various drivers in the driver model corresponding to our Upper
Layer Drivers (disc[sd] tape[st] etc), although there are SCSI devices
(like processors) that will get no driver at all bound.

We then use classes to export *device* interfaces, like one class of all
devices, another for Parallel interface devices, another for Fibre
Channel devices and so on.

We expect the class interface to work whether or not a driver is
present, because the class as we've implemented it is an interface to a
device property, not a driver property (and we also expect the class
interface to span multiple drivers...tapes and discs may all be attached
to a parallel bus, etc).

It sounds like the mismatch is interface on device rather than interface
on driver, but I don't see a way we could make the interface on driver
work for us because we need the interface even if a driver isn't bound.

James

Re: BUG(?): class_device_driver_link()

[Alan Stern]

On 18 Jun 2004, James Bottomley wrote:

> Well, the SCSI model for using these things isn't exactly the same as a
> more standard entity like a PCI device.
> 
> For every SCSI device the mid-layer scans, we allocate a generic device.
> 
> We have various drivers in the driver model corresponding to our Upper
> Layer Drivers (disc[sd] tape[st] etc), although there are SCSI devices
> (like processors) that will get no driver at all bound.
> 
> We then use classes to export *device* interfaces, like one class of all
> devices, another for Parallel interface devices, another for Fibre
> Channel devices and so on.
> 
> We expect the class interface to work whether or not a driver is
> present, because the class as we've implemented it is an interface to a
> device property, not a driver property (and we also expect the class
> interface to span multiple drivers...tapes and discs may all be attached
> to a parallel bus, etc).
> 
> It sounds like the mismatch is interface on device rather than interface
> on driver, but I don't see a way we could make the interface on driver
> work for us because we need the interface even if a driver isn't bound.

You could change things so that the "driver" exported to the driver-model
core really lives in the SCSI mid-layer and simply contains stubs that
reflect things like remove(), shutdown(), and so on to the actual
upper-layer driver.  Or am I missing some fundmental reason why this 
wouldn't be a good idea?

Alan Stern