Re: Flaw in the driver-model implementation of attributes

Re: Flaw in the driver-model implementation of attributes

[Clayton Weaver]

(Doubting that there is a sysfs faq anywhere
yet, ...)

What is a sysfs "class", as in /sys/class/...?

What do sysfs classes have in common? How is
a /sys/class/ different from a /sys/devices,
/sys/bus, etc?

In re: the current discussion, are the "usb-storage" attributes under discussion
something that the vfs would need to know
about(/sys/block/)? Something that a pci
bus would need to know about? Something that
a usb controller would need to know about?

Vfs models are virtual, so vfs having its
own sysfs tree for block devices does not
create any confusion relative to an
organization based on the the hardware
connection tree in the machine.

But when you are considering where to place
attributes meant to be evaluated by low-level
hardware drivers, it is easier to follow if the
organization follows a

  bus (ie pci, for example)
    host-controller/mux
      bus
        device
          [bus
            device...]

organization. (The bracketed branch is for a
cascade or bridge device.)

So how does "class" fit into that model?
Does it signify a domain of buses, a set
of bus attributes (should be under the
"/sysfs/bustype/busnum/" in that case),
a set of controller attributes, a set
of device attributes, or some software
abstraction like block or char device
attributes?

Regards,

Clayton Weaver
<mailto: cgweav@email.com>


-- 
__________________________________________________________
Sign-up for your own FREE Personalized E-mail at Mail.com
http://www.mail.com/?sr=signup

CareerBuilder.com has over 400,000 jobs. Be smarter about your job search
http://corp.mail.com/careers

Re: Flaw in the driver-model implementation of attributes

[Kevin P. Fleming]

Clayton Weaver wrote:

> (Doubting that there is a sysfs faq anywhere
> yet, ...)

Sounds like we're getting there!

> 
> What is a sysfs "class", as in /sys/class/...?

It is an abstraction. It is a group of objects that implement common 
functionality, and have common attributes and behaviors.

> 
> What do sysfs classes have in common? How is
> a /sys/class/ different from a /sys/devices,
> /sys/bus, etc?

/sys/bus, /sys/block are just special-case classes that get their own 
top-level directory. They could just easily have been put under 
/sys/class/block, /sys/class/bus.

> 
> In re: the current discussion, are the "usb-storage" attributes under discussion
> something that the vfs would need to know
> about(/sys/block/)? Something that a pci
> bus would need to know about? Something that
> a usb controller would need to know about?

IMHO, no. Any attributes specific to a usb-storage device are not 
something that any other layer would care about. As an example, a 
flash-memory USB key I have here support software write protection; 
while I don't know if the usb-storage driver currently exposes that, it 
could, and that would be very specific to usb-storage. Any userspace 
application that wanted to manipulate the state of that protection would 
look at /sys/class/usb-storage/... for devices it could potentially 
manage. It doesn't need to how or where those devices are connected, or 
even what type of media they may be. It only needs to know that they are 
usb-storage devices, and that they have a "writeprotect" attribute 
exposed in the appropriate place.

Re: Flaw in the driver-model implementation of attributes

[Patrick Mochel]

> > What is a sysfs "class", as in /sys/class/...?
> 
> It is an abstraction. It is a group of objects that implement common 
> functionality, and have common attributes and behaviors.

Not quite. A class represents a class of devices, or the function that a 
device performs, e.g. disk or sound. 

> > What do sysfs classes have in common? How is
> > a /sys/class/ different from a /sys/devices,
> > /sys/bus, etc?
> 
> /sys/bus, /sys/block are just special-case classes that get their own 
> top-level directory. They could just easily have been put under 
> /sys/class/block, /sys/class/bus.

No. If you want to go that far, 'devices' could go under there as well, 
and we'd eventually just have one top-level directory: /sys/class :) 

The top-level directories in sysfs represent classes of objects, not 
necessarily tied to any driver model concepts. The reason it's so 
driver-model heavy now is because that's how the whole thing originated. 


	-pat

RE: Flaw in the driver-model implementation of attributes

[Perez-Gonzalez, Inaky]

> From: Alan Stern [mailto:stern@rowland.harvard.edu]
> 
> On Thu, 19 Jun 2003, Patrick Mochel wrote:
> 
> > SCSI copied USB in this respect. I've always been skeptical about the
> > representation, though Greg had good reason to initially do this. I
wonder
> > if that object could be moved over /sys/class/usb-host/ these days..
> 
> I wonder about the apparent proliferation of entries under /sys/class/.
> If people continue to add things like /sys/class/usb-storage/ right at the
> top level, won't it become rather unwieldy?  What would be a good place to
> put something like that?

That was exactly my point with the "it-should-go-where-the-
physical-device-is" suggestion. /sys/class will become something
as wild as /proc is now IANF.

Iñaky Pérez-González -- Not speaking for Intel -- all opinions are my own
(and my fault)

RE: Flaw in the driver-model implementation of attributes

[Perez-Gonzalez, Inaky]

> From: viro@parcelfarce.linux.theplanet.co.uk
[mailto:viro@parcelfarce.linux.theplanet.co.uk]
> 
> > So what? _every_ block device will have some form of physical
> > back-up that can be linked back into sysfs.
> 
> ... except ones that will not.  Wonderful.  I bow to that logics - there
> is nothing it wouldn't cover.

Thank you }:) - we like it or not, data goes somewhere.

> > In the tree structure it makes sense, because each block
> > device, at the end is or a partition (and thus is embedded
> > in a "true" block device) or a true block device on a 1:1
> > relationship with a physical device.
> 
> BS.  There is nothing to stop you from having a block device that talks
> to userland process instead of any form of hardware.  As the matter of
> fact, we already have such a beast - nbd.  There is also RAID - where

Sure, there: /sys/devices/"virtual"/nbd/0

> there fsck is 1:1 here?  There's also such thing as RAID5 over partitions
> that sit on several disks - where do you see 1:1 or 1:n or n:1?

/sys/devices/"virtual"/raid/0

> There is such thing as e.g. encrypted loop over NFS.  There are all
> sorts of interesting things, with all sorts of interesting relationship
> to some pieces of hardware.

/sys/devices/"virtual"/loopback/0

Don't you have to do "-o loop" when you mount a loopback?
... same thing happens with nbd and RAID, you have to 
tell the kernel to create the actual devices (or it 
does); that they show up nowhere in sysfs (yet) is different.

Iñaky Pérez-González -- Not speaking for Intel -- all opinions are my own
(and my fault)

RE: Flaw in the driver-model implementation of attributes

[Perez-Gonzalez, Inaky]

> From: viro@parcelfarce.linux.theplanet.co.uk
> On Tue, Jun 17, 2003 at 08:44:50PM -0700, Perez-Gonzalez, Inaky wrote:
> 
> > Maybe this is going to kill my argument as an analogy, but think
> > about a C++ class hierarchy, where belonging to a class means
> > ...
> 
> But there is no inheritance here.  Block device and IDE disk are
> different objects and relation is not "A is B with <...>", it's
> "among other things, A happens to use B in a way <...>".

Well, the device is an IDE disk that is linked through an IDE 
controller that is linked through a PCI controller to the system bus. 

That IDE device presents an interface. The block layer just presents 
the common interface that all block devices present (and that IDE
and SCSI disks are able to provide) - there is no inheritance, but
the concept is the same.
 
> Moreover, there is no such thing as "physical device 
> of that block device". There might be many.  There 
> might be none.  IOW, we have a bunch of constructors 
> for class "block device" and some of them happen to have
> some kinds of physical devices among their arguments. 

[I happen not to know the block layer as well as you and many others 
do, so please correct me where I am wrong ...]

So what? _every_ block device will have some form of physical 
back-up that can be linked back into sysfs.

In cases like this, doesn't it make sense to have some 
/sys/devices/SOMETHING/ hierarchy for those  "logical" or "virtual" 
devices that back-up those block devices? 

You could even say that RAID and ramdisks -as used in the example 
above- would belong to /sys/devices/"virtual"/raid/ and 
...../ramdisks/; after all, you have to create those devices before
being able to attach them (last time I checked):

(using subdirs for each layer for clarity)

/sys/devices/"virtual"/raid/0
  attr1
  attr2     ...   sysfs specific attrs 
  block/    ... block layer specific attrs
    attr1
    attr2
    component1 -> /sys/devices/pci/FOO/block/part1  
    component1 -> /sys/devices/pci/BAR/block/part4  

(I would also love to see the block device node being dropped in 
the corresponding "block" directory, but that's another story).

And extrapolating even more, I'd expect to see something 
like this for the block devices that are part of a physical device 
(partitions, I mean):

/sys/blabla/pci/3.14159/ide0/0.0
  attr1
  attr2     ...   sysfs specific attrs 
  block/    ... block layer specific attrs
    attr1
    attr2  
    part0/  ... partition specific stuff
      attr1
      attr...
    part1/
      attr1
      attr2 ...
  ide/      ... ide layer specific attrs
    attr1
    attr2 

In the tree structure it makes sense, because each block
device, at the end is or a partition (and thus is embedded
in a "true" block device) or a true block device on a 1:1
relationship with a physical device.

Iñaky Pérez-González -- Not speaking for Intel -- all opinions are my own
(and my fault)

Re: Flaw in the driver-model implementation of attributes

[viro]

On Wed, Jun 18, 2003 at 12:48:59AM -0700, Perez-Gonzalez, Inaky wrote:
 
> [I happen not to know the block layer as well as you and many others 
> do, so please correct me where I am wrong ...]
> 
> So what? _every_ block device will have some form of physical 
> back-up that can be linked back into sysfs.

... except ones that will not.  Wonderful.  I bow to that logics - there
is nothing it wouldn't cover.
 
> In cases like this, doesn't it make sense to have some 
> /sys/devices/SOMETHING/ hierarchy for those  "logical" or "virtual" 
> devices that back-up those block devices? 
> 
> You could even say that RAID and ramdisks -as used in the example 
> above- would belong to /sys/devices/"virtual"/raid/ and 
> ...../ramdisks/; after all, you have to create those devices before
> being able to attach them (last time I checked):
 
> In the tree structure it makes sense, because each block
> device, at the end is or a partition (and thus is embedded
> in a "true" block device) or a true block device on a 1:1
> relationship with a physical device.

BS.  There is nothing to stop you from having a block device that talks
to userland process instead of any form of hardware.  As the matter of
fact, we already have such a beast - nbd.  There is also RAID - where
there fsck is 1:1 here?  There's also such thing as RAID5 over partitions
that sit on several disks - where do you see 1:1 or 1:n or n:1?
There is such thing as e.g. encrypted loop over NFS.  There are all
sorts of interesting things, with all sorts of interesting relationship
to some pieces of hardware.

Re: Flaw in the driver-model implementation of attributes

[Alan Stern]

On Wed, 18 Jun 2003 viro@parcelfarce.linux.theplanet.co.uk wrote:

> BS.  There is nothing to stop you from having a block device that talks
> to userland process instead of any form of hardware.  As the matter of
> fact, we already have such a beast - nbd.  There is also RAID - where
> there fsck is 1:1 here?  There's also such thing as RAID5 over partitions
> that sit on several disks - where do you see 1:1 or 1:n or n:1?
> There is such thing as e.g. encrypted loop over NFS.  There are all
> sorts of interesting things, with all sorts of interesting relationship
> to some pieces of hardware.

This is the sort of thing that bothers me.  Block devices deserve their 
own "view", so we have /sys/block/ -- perhaps to be renamed 
/sys/class/block/.  Fine.

But what other sorts of things deserve their own "view" as well?  Some
are already established, maybe others aren't.  How's a developer supposed
to know whether the driver he's working on deserves its own entry in
/sys/class/ or not?  How's a user supposed to know where in the hierarchy
to look for a particular device?

Here's a suggestion for something that would definitely help.  Create a
listing (maybe in Documentation/driver-model/) of all the major kernel
subsystems that deserve to have their own entries in /sys/class/ (or the
equivalent).  Explain clearly that any device driver that registers with
one of those subsystems will receive a directory in the /sys/class/
hierarchy where it can register its class devices, and say what the name
of that directory will be.  Explain that a driver that doesn't register
with one of these subsystems will simply have to create its own entry in
/sys/devices/ under its parent node.

Not all this infrastructure has been created yet.  For instance, there 
isn't at the moment any place under /sys/class/usb/ for a USB host 
controller driver to register its class device.  But if these ideas were 
formalized and written down, it would be straightforward to fill in the 
missing pieces.

Alan Stern

Re: Flaw in the driver-model implementation of attributes

[Greg KH]

On Wed, Jun 18, 2003 at 10:32:22AM -0400, Alan Stern wrote:
> On Wed, 18 Jun 2003 viro@parcelfarce.linux.theplanet.co.uk wrote:
> 
> > BS.  There is nothing to stop you from having a block device that talks
> > to userland process instead of any form of hardware.  As the matter of
> > fact, we already have such a beast - nbd.  There is also RAID - where
> > there fsck is 1:1 here?  There's also such thing as RAID5 over partitions
> > that sit on several disks - where do you see 1:1 or 1:n or n:1?
> > There is such thing as e.g. encrypted loop over NFS.  There are all
> > sorts of interesting things, with all sorts of interesting relationship
> > to some pieces of hardware.
> 
> This is the sort of thing that bothers me.  Block devices deserve their 
> own "view", so we have /sys/block/ -- perhaps to be renamed 
> /sys/class/block/.  Fine.
> 
> But what other sorts of things deserve their own "view" as well?  Some
> are already established, maybe others aren't.  How's a developer supposed
> to know whether the driver he's working on deserves its own entry in
> /sys/class/ or not?  How's a user supposed to know where in the hierarchy
> to look for a particular device?

Ok, have you _read_ the documentation on the driver model?  In it,
classes and devices are clearly spelled out as to what the differences
are, and what shows up where.

See Pat's linux.conf.au 2003 paper for much more detail.

And yes, I need to fix up the Documentation/driver_model/class.txt with
the most recent info...

In short, devices describe physical things that are present in the
computer system.  Classes describe a type of device, be it virtual or
physical.  Almost always, classes refer to something a user uses through
the /dev filesystem (like mice, tty, block, audio, etc.)

So no, there is not always a 1:1 mapping from classes to devices, that
is why the driver model does not enforce such a mapping at all.  You can
have multiple "struct class_device" structures that point to the same
"struct device" or no "struct device" at all.

Hope this helps to clear up the confusion that seems to be happening.

thanks,

greg k-h

Re: Flaw in the driver-model implementation of attributes

[Alan Stern]

On Wed, 18 Jun 2003, Greg KH wrote:

> Ok, have you _read_ the documentation on the driver model?  In it,
> classes and devices are clearly spelled out as to what the differences
> are, and what shows up where.

Yes, of course I've read it.  It's lacking a number of important details.

For example, nowhere in devices.txt does it say that a device driver
should not create attributes in the struct device's directory since that
directory is owned by the bus driver.  (That's a very easy mistake to
make; at first sight it appears to be the natural way for a driver to
expose details of how it controls a device.)  In fact, nowhere in the
documentation does it say that you shouldn't attach an attribute to an 
object unless you own that object.  Maybe that seems obvious, but when a 
driver is bound to a device can't it be said to "own" that device in some 
sense?

The class.txt document does _not_ explain clearly what the differences
between devices and classes (or more properly, class devices) are.  It
merely says that "A device class describes a type of device..."  It
doesn't say what sorts of devices get to belong to a class; it doesn't
even list the existing classes yet!  (As you are obviously aware.)

Let me ask you this:  Given a device that doesn't fit clearly into any of 
the existing classes, how would you decide whether or not to create a new 
class for it?

> See Pat's linux.conf.au 2003 paper for much more detail.
> 
> And yes, I need to fix up the Documentation/driver_model/class.txt with
> the most recent info...
> 
> In short, devices describe physical things that are present in the
> computer system.  Classes describe a type of device, be it virtual or
> physical.  Almost always, classes refer to something a user uses through
> the /dev filesystem (like mice, tty, block, audio, etc.)

Yes, but _which_ physical things correspond to devices?  And how should 
the parent-child relationships be decided?

Consider a concrete example: a USB host controller.  Let's say that on my
system /sys/devices/pci0/0000:00:07.2 is an OHCI HC.  That particular
object is created by the PCI bus driver, and directly below it is
/sys/devices/pci0/0000:00:07.2/usb1 -- what physical thing does that
correspond to?  Is it the virtual root hub?  It's created by the USB core;
where does the object created by the HC driver belong?

Or have I misunderstood, and was it intended from the start that _all_ the
objects under /sys/devices/ should be created by bus drivers, while _all_
the objects created by device drivers belong somewhere else?  Is that
somewhere else always under /sys/class/ (or /sys/block/)?  And where in
the documentation is this spelled out?

> So no, there is not always a 1:1 mapping from classes to devices, that
> is why the driver model does not enforce such a mapping at all.  You can
> have multiple "struct class_device" structures that point to the same
> "struct device" or no "struct device" at all.
> 
> Hope this helps to clear up the confusion that seems to be happening.

I'm still struggling... :-)

Alan Stern

Re: Flaw in the driver-model implementation of attributes

[Patrick Mochel]

> > Ok, have you _read_ the documentation on the driver model?  In it,
> > classes and devices are clearly spelled out as to what the differences
> > are, and what shows up where.
> 
> Yes, of course I've read it.  It's lacking a number of important details.

Hey, we've tried. I realize it's missing details, and though I know it's 
important to keep it updated, many other things take precedence. 

> For example, nowhere in devices.txt does it say that a device driver
> should not create attributes in the struct device's directory since that
> directory is owned by the bus driver.  (That's a very easy mistake to
> make; at first sight it appears to be the natural way for a driver to
> expose details of how it controls a device.)  In fact, nowhere in the
> documentation does it say that you shouldn't attach an attribute to an 
> object unless you own that object.  Maybe that seems obvious, but when a 
> driver is bound to a device can't it be said to "own" that device in some 
> sense?

It's "bound" to the device, but it does not own the object. 

Note that only recently have we realized the full importance of creating 
attributes _only_ for objects that you own. It's exposed bugs recently, 
and hasn't had a chance to make it in to the documentation. 

> Let me ask you this:  Given a device that doesn't fit clearly into any of 
> the existing classes, how would you decide whether or not to create a new 
> class for it?

If it does not fit into the existing classes, then there is probably a new 
class that needs to be created for it. While you're at it, please update 
the documentation and set a good example for the rest of us ;)

> Yes, but _which_ physical things correspond to devices?  And how should 
> the parent-child relationships be decided?
> 
> Consider a concrete example: a USB host controller.  Let's say that on my
> system /sys/devices/pci0/0000:00:07.2 is an OHCI HC.  That particular
> object is created by the PCI bus driver, and directly below it is
> /sys/devices/pci0/0000:00:07.2/usb1 -- what physical thing does that
> correspond to?  Is it the virtual root hub?  It's created by the USB core;
> where does the object created by the HC driver belong?
> 
> Or have I misunderstood, and was it intended from the start that _all_ the
> objects under /sys/devices/ should be created by bus drivers, while _all_
> the objects created by device drivers belong somewhere else?  

Yes.

> Is that
> somewhere else always under /sys/class/ (or /sys/block/)?  

Yes, /sys/class

> And where in
> the documentation is this spelled out?

It should be in the first sections of each driver model paper - The 
hierarchy under /sys/devices/ represents the physical hierarchy of the 
system itself. Each object represented in it is intended to map directly 
to a physical device. Physical devices are discovered and registered by 
bus drivers in the system. 


	-pat

Re: Flaw in the driver-model implementation of attributes

[Alan Stern]

On Thu, 19 Jun 2003, Patrick Mochel wrote:

> > > Ok, have you _read_ the documentation on the driver model?  In it,
> > > classes and devices are clearly spelled out as to what the differences
> > > are, and what shows up where.
> > 
> > Yes, of course I've read it.  It's lacking a number of important details.
> 
> Hey, we've tried. I realize it's missing details, and though I know it's 
> important to keep it updated, many other things take precedence. 

Believe me, I understand how hard it is to keep documentation in sync with 
a developing system.

> > Let me ask you this:  Given a device that doesn't fit clearly into any of 
> > the existing classes, how would you decide whether or not to create a new 
> > class for it?
> 
> If it does not fit into the existing classes, then there is probably a new 
> class that needs to be created for it. While you're at it, please update 
> the documentation and set a good example for the rest of us ;)

I'll offer a deal.  When you and Greg have gotten the current set of 
updates into the documentation, let me know and I'll add in explanations 
of all the stuff that has puzzled me and been thrashed out in this thread.

At least I seem to be making progress.  Thanks for the help.

Alan Stern

Re: Flaw in the driver-model implementation of attributes

[Alan Stern]

Here's another way to express some of my questions.

Consider that many of the buses in a computer system have interconnects 
that are sufficiently complicated to warrant their own driver.  For 
example, a SCSI adapter card can be regarded as an interconnect between 
a PCI bus and a SCSI bus.  A USB host controller is an interconnect 
between a PCI bus and a USB bus.  Each of these requires its own driver.

Should the interconnect's driver add a device under /sys/devices/, and if 
so, where?

Right now they don't.  For example, I have a SCSI card in my system.  It
shows up as /sys/devices/pci0/0000:00:0d.0/ -- that's the PCI bus driver's
view of the card.  It also shows up as
/sys/devices/pci0/0000:00:0d.0/host1/ -- that's the SCSI core's view of 
the same card.  So there are two devices representing the same physical 
object, and neither of them is created by the aic7xxx driver.

The same thing happens with USB.  /sys/devices/pci0/0000:00:07.2/ and 
/sys/devices/pci0/0000:00:07.2/usb1/ are two views of the same host 
controller, created by the PCI and USB bus drivers.  There's no entry 
created by the host controller driver.

Now presumably this fits the driver model okay; I haven't seen any 
requirement that _every_ device/driver combination must have an entry 
under /sys/devices/.  Still, supposing the driver _did_ want to create its 
own object, where would it go?

Would the SCSI host adapter device show up as 
/sys/devices/pci0/0000:00:0d.0/aic7xxx/host1/ -- intermediate between the 
PCI and SCSI bus driver entries?  Should it appear as 
/sys/devices/pci0/0000:00:0d.0/host1/aic7xxx/ -- under the SCSI bus entry?  
Or should it not appear under /sys/devices/ at all but somewhere else 
instead, such as /sys/class/scsi_host/host1/aic7xxx/ ?

The same questions apply to the USB host controller example.

Bear in mind that these interconnects are not the same manner of device as
a disk or a mouse.  They're not things the user normally manipulates and
they aren't accessed through /dev/.  Does that mean that they don't
deserve to go under /sys/classes/ ?


To change the topic slightly, consider this small inconsistency in sysfs.  
The IDE bus driver creates entries for each IDE channel.  So for example, 
/sys/devices/pci0/0000:00:07.1/ide0/0.0/ represents the master device on 
channel 0 whereas /sys/devices/pci0/0000:00:07.1/ide1/1.1 represents the 
slave device on channel 1.  The SCSI bus driver, on the other hand, does 
not create intermediate levels in the hierarchy for channels or targets.  
So for example, /sys/devices/pci0/0000:00:0d.0/host1/1:0:5:0/ is the entry 
for host 1, channel 0, target 5, LUN 0.  There's nothing in between the 
host level and the LUN level.

Is that the sort of decision that's left up to the bus driver author?  
Should there be any sort of enforced consistency, or doesn't it matter?

Still trying to grasp the fundamental principles at work here...

Alan Stern

Re: Flaw in the driver-model implementation of attributes

[Patrick Mochel]

> Consider that many of the buses in a computer system have interconnects 
> that are sufficiently complicated to warrant their own driver.  For 
> example, a SCSI adapter card can be regarded as an interconnect between 
> a PCI bus and a SCSI bus.  A USB host controller is an interconnect 
> between a PCI bus and a USB bus.  Each of these requires its own driver.
> 
> Should the interconnect's driver add a device under /sys/devices/, and if 
> so, where?

No, it's already created for them. The USB controller you speak of is a 
PCI device. The driver is a PCI driver. There is already an object that 
represents the physical device in the hierarchy; you do not need to create 
a virtual one to represent it. 

> Right now they don't.  For example, I have a SCSI card in my system.  It
> shows up as /sys/devices/pci0/0000:00:0d.0/ -- that's the PCI bus driver's
> view of the card.  It also shows up as
> /sys/devices/pci0/0000:00:0d.0/host1/ -- that's the SCSI core's view of 
> the same card.  So there are two devices representing the same physical 
> object, and neither of them is created by the aic7xxx driver.
> 
> The same thing happens with USB.  /sys/devices/pci0/0000:00:07.2/ and 
> /sys/devices/pci0/0000:00:07.2/usb1/ are two views of the same host 
> controller, created by the PCI and USB bus drivers.  There's no entry 
> created by the host controller driver.

SCSI copied USB in this respect. I've always been skeptical about the
representation, though Greg had good reason to initially do this. I wonder 
if that object could be moved over /sys/class/usb-host/ these days.. 

> Now presumably this fits the driver model okay; I haven't seen any 
> requirement that _every_ device/driver combination must have an entry 
> under /sys/devices/.  Still, supposing the driver _did_ want to create its 
> own object, where would it go?

Well, what does the object represent? A per-device driver-specific object? 
Is it really a separate object, or does it have a SCSI host object 
embedded in it? Usually, from what I've seen, if there is a per-device 
driver-specific object, it contains an embedded (class) object. This 
object is what the driver registers with the infrastructure for that 
device type.

That embedded structure should contain an embedded struct class_device, 
which is registered with the class (when the driver registers it's 
object). That gives it presence under /sys/class/whatever/

It also gives the driver an object that it owns, via embedding, which it 
is free to export attributes for.

> Bear in mind that these interconnects are not the same manner of device as
> a disk or a mouse.  They're not things the user normally manipulates and
> they aren't accessed through /dev/.  Does that mean that they don't
> deserve to go under /sys/classes/ ?

No, they should still go under /sys/class. 

> To change the topic slightly, consider this small inconsistency in sysfs.  
> The IDE bus driver creates entries for each IDE channel.  So for example, 
> /sys/devices/pci0/0000:00:07.1/ide0/0.0/ represents the master device on 
> channel 0 whereas /sys/devices/pci0/0000:00:07.1/ide1/1.1 represents the 
> slave device on channel 1.  The SCSI bus driver, on the other hand, does 
> not create intermediate levels in the hierarchy for channels or targets.  
> So for example, /sys/devices/pci0/0000:00:0d.0/host1/1:0:5:0/ is the entry 
> for host 1, channel 0, target 5, LUN 0.  There's nothing in between the 
> host level and the LUN level.
> 
> Is that the sort of decision that's left up to the bus driver author?  
> Should there be any sort of enforced consistency, or doesn't it matter?

Yes. :) 

There should be consistency, but it is left up to the authors. We don't 
have the time to babysit each subsystem's sysfs exports. We anticipate a 
de facto standard to arise and everyone to conform to it, especially when 
users point out inconsistencies in them.. 


	-pat

Re: Flaw in the driver-model implementation of attributes

[Alan Stern]

On Thu, 19 Jun 2003, Patrick Mochel wrote:

> SCSI copied USB in this respect. I've always been skeptical about the
> representation, though Greg had good reason to initially do this. I wonder 
> if that object could be moved over /sys/class/usb-host/ these days.. 

I wonder about the apparent proliferation of entries under /sys/class/.  
If people continue to add things like /sys/class/usb-storage/ right at the
top level, won't it become rather unwieldy?  What would be a good place to
put something like that?

----

> Well, what does the object represent? A per-device driver-specific object? 
> Is it really a separate object, or does it have a SCSI host object 
> embedded in it? Usually, from what I've seen, if there is a per-device 
> driver-specific object, it contains an embedded (class) object. This 
> object is what the driver registers with the infrastructure for that 
> device type.
> 
> That embedded structure should contain an embedded struct class_device, 
> which is registered with the class (when the driver registers it's 
> object). That gives it presence under /sys/class/whatever/
> 
> It also gives the driver an object that it owns, via embedding, which it 
> is free to export attributes for.

On Thu, 19 Jun 2003, Mike Anderson wrote:

> Actually host1 is created by the aic7xxx driver through registration
> with the SCSI subsystem. The aic7xxx allocates the structure, registers
> it, unregisters it, and drops the ref on it when it is done with it.

[Two different replies to related questions of mine.]

This makes sense.  But it doesn't match my experience working with the 
SCSI core.  (That experience is connected with usb-storage, not aic7xxx.)

When a host driver registers a newly-detected host adapter with the core,
it doesn't provide a structure representing that adapter.  It provides a
Scsi Host Template, but that's not the same thing.  Instead, the core
creates a struct Scsi_Host on the driver's behalf.  Embedded in it is a
struct device, which is registered under /sys/devices/, and a struct
class_device, which is registered under /sys/class/scsi_host/.  The driver
doesn't own these structures; the SCSI core does.

Furthermore, there is no release() for the embedded struct class_device,
so there's no way for the driver to know when it goes away.  (That lack is
liable to cause a problem at some point.  I noticed that until very
recently there was no release() method for struct class_device at all.)  
There _is_ a release notification for the overall struct Scsi_Host in
scsi_remove_host(), but it occurs at the wrong time.  It is called right
after the embedded struct device has been deregistered, not when its
reference count drops to 0.

So what Mike says the SCSI core does, and what Pat says it ought to do, 
isn't what actually happens.  Is this simply an indication that the work 
of completing the SCSI code to the driver model isn't yet complete?  It 
seems as though a very large change would be needed to make things work as 
Pat described.

In the meantime, where should I register my class device for the 
usb-storage driver?  Should it go in 
/sys/class/scsi_host/host1/usb-storage/, under the existing class device?  
That might be simplest.  Or should there be a new class just for it?

Alan Stern

Re: Flaw in the driver-model implementation of attributes

[Greg KH]

On Thu, Jun 19, 2003 at 05:14:42PM -0400, Alan Stern wrote:
> 
> So what Mike says the SCSI core does, and what Pat says it ought to do, 
> isn't what actually happens.  Is this simply an indication that the work 
> of completing the SCSI code to the driver model isn't yet complete?  It 
> seems as though a very large change would be needed to make things work as 
> Pat described.

I think it's more of the matter that Mike is still changing the way the
SCSI core works with the class and driver model.  I suggest you take
this to the linux-scsi list to hash out the way scsi should and does
work in order to find the best solution for usb-storage, as I guess that
it also would be the best solution for all other SCSI host drivers.

> In the meantime, where should I register my class device for the 
> usb-storage driver?

Why not hold off until the scsi people are finished with their changes?
If after that, you _really_ need to be putting usb-storage only
attributes in the sysfs tree somewhere, we can talk again.

thanks,

greg "scsi makes my head hurt" k-h

Re: Flaw in the driver-model implementation of attributes

[Alan Stern]

On Thu, 19 Jun 2003, Greg KH wrote:

> I think it's more of the matter that Mike is still changing the way the
> SCSI core works with the class and driver model.  I suggest you take
> this to the linux-scsi list to hash out the way scsi should and does
> work in order to find the best solution for usb-storage, as I guess that
> it also would be the best solution for all other SCSI host drivers.

Okay.

> > In the meantime, where should I register my class device for the 
> > usb-storage driver?
> 
> Why not hold off until the scsi people are finished with their changes?
> If after that, you _really_ need to be putting usb-storage only
> attributes in the sysfs tree somewhere, we can talk again.

Sounds reasonable.  By the way, do you plan to create a 
/sys/class/usb_host/ class for the OHCI and ECHI drivers?

Alan Stern

Re: Flaw in the driver-model implementation of attributes

[Greg KH]

On Fri, Jun 20, 2003 at 10:22:21AM -0400, Alan Stern wrote:
> > Why not hold off until the scsi people are finished with their changes?
> > If after that, you _really_ need to be putting usb-storage only
> > attributes in the sysfs tree somewhere, we can talk again.
> 
> Sounds reasonable.  By the way, do you plan to create a 
> /sys/class/usb_host/ class for the OHCI and ECHI drivers?

Yes, and for the UHCI driver too :)

Unless someone else wants to send me a patch...

thanks,

greg k-h

Re: Flaw in the driver-model implementation of attributes

[Greg KH]

On Fri, Jun 20, 2003 at 11:32:51AM -0700, Greg KH wrote:
> On Fri, Jun 20, 2003 at 10:22:21AM -0400, Alan Stern wrote:
> > > Why not hold off until the scsi people are finished with their changes?
> > > If after that, you _really_ need to be putting usb-storage only
> > > attributes in the sysfs tree somewhere, we can talk again.
> > 
> > Sounds reasonable.  By the way, do you plan to create a 
> > /sys/class/usb_host/ class for the OHCI and ECHI drivers?
> 
> Yes, and for the UHCI driver too :)
> 
> Unless someone else wants to send me a patch...

This is now in 2.5.74.  Combined with the module reference patch for
attributes that I just posted I now think that the problems discussed in
this thread are solved, right?

thanks,

greg k-h

Re: Flaw in the driver-model implementation of attributes

[Alan Stern]

On Wed, 2 Jul 2003, Greg KH wrote:

> This is now in 2.5.74.  Combined with the module reference patch for
> attributes that I just posted I now think that the problems discussed in
> this thread are solved, right?

Yes.  Any remaining difficulties are more-or-less internal SCSI matters.
I'm glad that, annoying though it may have been for some people, this
conversation had some positive results.

And by the way, I was serious about that offer of contributing to the 
documentation once you and Pat have added your current updates.

Alan Stern

Re: Flaw in the driver-model implementation of attributes

[Mike Anderson]

Alan Stern [stern@rowland.harvard.edu] wrote:
> Right now they don't.  For example, I have a SCSI card in my system.  It
> shows up as /sys/devices/pci0/0000:00:0d.0/ -- that's the PCI bus driver's
> view of the card.  It also shows up as
> /sys/devices/pci0/0000:00:0d.0/host1/ -- that's the SCSI core's view of 
> the same card.  So there are two devices representing the same physical 
> object, and neither of them is created by the aic7xxx driver.

Actually host1 is created by the aic7xxx driver through registration
with the SCSI subsystem. The aic7xxx allocates the structure, registers
it, unregisters it, and drops the ref on it when it is done with it.

The SCSI subsystems view of hosts that have registered with the subsystem
is through /class/scsi_host.

> To change the topic slightly, consider this small inconsistency in sysfs.  
> The IDE bus driver creates entries for each IDE channel.  So for example, 
> /sys/devices/pci0/0000:00:07.1/ide0/0.0/ represents the master device on 
> channel 0 whereas /sys/devices/pci0/0000:00:07.1/ide1/1.1 represents the 
> slave device on channel 1.  The SCSI bus driver, on the other hand, does 
> not create intermediate levels in the hierarchy for channels or targets.  
> So for example, /sys/devices/pci0/0000:00:0d.0/host1/1:0:5:0/ is the entry 
> for host 1, channel 0, target 5, LUN 0.  There's nothing in between the 
> host level and the LUN level.
> 

Currently the scsi device in the SCSI subsystem is LUN.  While the
future may lead to having a having a target as a child of the host it
would be a large change for 2.5 / 2.6. 

> Is that the sort of decision that's left up to the bus driver author?  
> Should there be any sort of enforced consistency, or doesn't it matter?

Each transport / bus may have a unique nexus between a parent and child.
Interpretation of a name outside of the name space registering or
owning the name would lead to bad info (i.e.  interpretation of these
two child nodes using the same policy "pci0/0000:00:07.1"
"host1/1:0:5:0/")  Enforcing a overall consistency could lead to
creating pseudo hierarchies just to fit a consistency model.

-andmike
--
Michael Anderson
andmike@us.ibm.com

RE: Flaw in the driver-model implementation of attributes

[Perez-Gonzalez, Inaky]

> From: Kevin P. Fleming [mailto:kpfleming@cox.net]
>
> These are two wildly differing views, but yes, they are the same device.
> These differing attributes do _not_ belong in the same directory. An
> application looking at your IDE devices does not really care how the
> block subsystem perceives those devices (i.e. hdparm). Conversely, an
> application looking at your block devices should not care about what
> underlying physical devices (if any :-) they are associated with. 

But that is describing a physical mapping vs. a logical view.

I can think of many different "views" I want to see (IDE, block,
physical, block sorted by size, the ones who are green and sing 
in Basque...) but only one is unique and inherently defined: the
physical one. 

I guess I agree with Alan; I'd prefer something like:

/sys/devices/pci0/0000:00:07.1/ide0/0.0/
  attr1
  attr2 ...   sysfs specific attrs 
  block-attr1
  block-attr2 ... block layer specific attrs
  ide-attr1
  ide-attr2 ... ide layer specific attrs

(or make that TAG- a subdirectory of the device if you wish,
that is just an example). 

This way, for each device I have an *unique* location where to
track it, and an easy way to hunt down attributes specific to
each layer that controls it.

And when, when I want to make logical views (again, by IDE, by
block device ...), I can do that from user space and create a
tree full of symlinks to the unique, physical locations.

Maybe this is going to kill my argument as an analogy, but think
about a C++ class hierarchy, where belonging to a class means
to inherit that class' methods. When an object is instantiated
and its class inherits a lot of other classes, it inherits all
the methods of those classes. Your methods are the attrs, and
you can access them with the same pointer, you don't  need to
look somewhere else ...

Iñaky Pérez-González -- Not speaking for Intel -- all opinions are my own
(and my fault)

Re: Flaw in the driver-model implementation of attributes

[viro]

On Tue, Jun 17, 2003 at 08:44:50PM -0700, Perez-Gonzalez, Inaky wrote:

> Maybe this is going to kill my argument as an analogy, but think
> about a C++ class hierarchy, where belonging to a class means
> to inherit that class' methods. When an object is instantiated
> and its class inherits a lot of other classes, it inherits all
> the methods of those classes. Your methods are the attrs, and
> you can access them with the same pointer, you don't  need to
> look somewhere else ...

But there is no inheritance here.  Block device and IDE disk are
different objects and relation is not "A is B with <...>", it's
"among other things, A happens to use B in a way <...>".  

Moreover, there is no such thing as "physical device of that block device".
There might be many.  There might be none.  IOW, we have a bunch of
constructors for class "block device" and some of them happen to have
some kinds of physical devices among their arguments.  That's it.

Re: Flaw in the driver-model implementation of attributes

[Alan Stern]

On Mon, 16 Jun 2003, Russell King wrote:

> Have you noticed the two symlinks which are created from the class
> device.  For example:
> 
> /sysfs/class/pcmcia_socket/
> |-- pcmcia_socket0
> |   |-- device -> ../../../devices/pci0/0000:00:01.0
> |   |-- driver -> ../../../bus/pci/drivers/yenta_cardbus
> |   `-- status
> `-- pcmcia_socket1
>     |-- device -> ../../../devices/pci0/0000:00:01.1
>     |-- driver -> ../../../bus/pci/drivers/yenta_cardbus
>     `-- status
> 
> This means you can access the physical device attributes using (eg):
> 	/sysfs/class/pcmcia_socket/pcmcia_socket0/device/resource
> the driver attributes:
> 	/sysfs/class/pcmcia_socket/pcmcia_socket0/driver/...
> and the class attributes:
> 	/sysfs/class/pcmcia_socket/pcmcia_socket0/...
> 
> You don't have to try to work out where in /sys/bus and /sys/devices the
> driver and device respectively are - that's already done for you.

This doesn't provide any really good place to put device attributes that 
are owned by the driver.  They can't go in
	/sysfs/class/pcmcia_socket/pcmcia_socket0/device/...
because the driver doesn't own the device.  They can't go in
	/sysfs/class/pcmcia_socket/pcmcia_socket0/driver/...
because they aren't attributes of the _driver_, they're attributes of the 
_device_.  And they certainly aren't class attributes.

So where would you put them?  You'd have to create another subdirectory of
	/sysfs/class/pcmcia_socket/pcmcia_socket0/
owned by the driver.  No really good name for this subdirectory spings 
to mind, and it's still kind of awkward.  But doable.

Alan Stern

Re: Flaw in the driver-model implementation of attributes

[Patrick Mochel]

> This doesn't provide any really good place to put device attributes that 
> are owned by the driver.  They can't go in
> 	/sysfs/class/pcmcia_socket/pcmcia_socket0/device/...
> because the driver doesn't own the device.  They can't go in
> 	/sysfs/class/pcmcia_socket/pcmcia_socket0/driver/...
> because they aren't attributes of the _driver_, they're attributes of the 
> _device_.  And they certainly aren't class attributes.
> 
> So where would you put them?  You'd have to create another subdirectory of
> 	/sysfs/class/pcmcia_socket/pcmcia_socket0/
> owned by the driver.  No really good name for this subdirectory spings 
> to mind, and it's still kind of awkward.  But doable.

What is wrong with putting them in 

/sysfs/class/pcmcia_socket/pcmcia_socket0/

? The driver owns that object. And, if it is device-specific feature, then 
it is likely related to what class the device belongs to, and is therefore 
relevant for that directory.


	-pat

Re: Flaw in the driver-model implementation of attributes

[Alan Stern]

On Mon, 16 Jun 2003, Patrick Mochel wrote:

> > This doesn't provide any really good place to put device attributes that 
> > are owned by the driver.  They can't go in
> > 	/sysfs/class/pcmcia_socket/pcmcia_socket0/device/...
> > because the driver doesn't own the device.  They can't go in
> > 	/sysfs/class/pcmcia_socket/pcmcia_socket0/driver/...
> > because they aren't attributes of the _driver_, they're attributes of the 
> > _device_.  And they certainly aren't class attributes.
> > 
> > So where would you put them?  You'd have to create another subdirectory of
> > 	/sysfs/class/pcmcia_socket/pcmcia_socket0/
> > owned by the driver.  No really good name for this subdirectory spings 
> > to mind, and it's still kind of awkward.  But doable.
> 
> What is wrong with putting them in 
> 
> /sysfs/class/pcmcia_socket/pcmcia_socket0/
> 
> ? The driver owns that object. And, if it is device-specific feature, then 
> it is likely related to what class the device belongs to, and is therefore 
> relevant for that directory.

Are you sure?  Suppose a pcmcia disk drive is plugged in to that socket.  
Why is a disk driver going to name its object "pcmcia_socket0"?  It must
be the pcmcia socket driver that owns the object, not the disk driver.  So 
then where does the disk driver put the disk-related attributes?  Don't 
say in /sys/class/pcmcia_socket/pcmcia_socket0/device/, because the driver 
doesn't own that object either.

Anyway, in my case it's a USB device.  At the moment /sys/class/usb
doesn't contain anything AFAICT, although no doubt that can be changed.

Greg, what is the current organization of directories for USB buses and 
devices?  Would it make sense to let each driver register its devices as
/sys/class/usb/bus#/dev#/ ?  Would that better be done under /sys/bus/usb 
?  Or is something else already there?  Or should the usb-storage driver, 
for example, create its own class?

Alan Stern

Re: Flaw in the driver-model implementation of attributes

[Patrick Mochel]

> Are you sure?  Suppose a pcmcia disk drive is plugged in to that socket.  
> Why is a disk driver going to name its object "pcmcia_socket0"?  It must
> be the pcmcia socket driver that owns the object, not the disk driver.  So 
> then where does the disk driver put the disk-related attributes?  Don't 
> say in /sys/class/pcmcia_socket/pcmcia_socket0/device/, because the driver 
> doesn't own that object either.

Well, are you talking about a socket or a disk? Obviously, those are very 
different devices, and hence have very different objects that you create 
for them. 

One way or another, you should be exporting attributes under the directory 
of the object that you create. If it's a socket, put it under the socket 
directory above. If it's a disk, then it will have a directory in another 
location for which you can export attributes. 

Do you have a specific example, or are you just hypothesizing? 


	-pat

Re: Flaw in the driver-model implementation of attributes

[Alan Stern]

On Mon, 16 Jun 2003, Patrick Mochel wrote:

> > Are you sure?  Suppose a pcmcia disk drive is plugged in to that socket.  
> > Why is a disk driver going to name its object "pcmcia_socket0"?  It must
> > be the pcmcia socket driver that owns the object, not the disk driver.  So 
> > then where does the disk driver put the disk-related attributes?  Don't 
> > say in /sys/class/pcmcia_socket/pcmcia_socket0/device/, because the driver 
> > doesn't own that object either.
> 
> Well, are you talking about a socket or a disk? Obviously, those are very 
> different devices, and hence have very different objects that you create 
> for them. 
> 
> One way or another, you should be exporting attributes under the directory 
> of the object that you create. If it's a socket, put it under the socket 
> directory above. If it's a disk, then it will have a directory in another 
> location for which you can export attributes. 
> 
> Do you have a specific example, or are you just hypothesizing? 

I have a specific example.  I want to create attribute files to export the
state of the usb-storage driver for each of the devices it manages.  The
problem is that the driver doesn't create the objects representing the
devices themselves -- that's done by the SCSI core.  Usb-storage acts more
like a transport conduit or a SCSI host driver.  But creating attributes
without creating the objects they belong to doesn't fit your model, so no
wonder I ran into trouble.

[Parenthetically, I don't see what's so bad about creating attributes for 
objects you don't own.  Provided it's guaranteed that after 
device_remove_file() returns no one will call the attribute's show() or 
store() methods, and provided you are made aware (by some other mechanism) 
when the object needs to go away, what harm can result?]

There are still aspects of the whole thing I find confusing or illogical.  
Consider this requirement, that attributes should only be created for
objects that the software owns.  In practice this must lead to the
creation of several objects for each actual device, because each different
layer of software managing that device will need to have its own object.  
For example, on my system the master device on the first PCI IDE channel
is a hard disk, hda in fact.  This means that
/sys/devices/pci0/0000:00:07.1/ide0/0.0/ and /sys/block/hda/ refer to the
same physical device.  One is created by the IDE bus driver, the other by
a block device driver.  Granted, there are links from one to the other,
but it still indicates that the organization of sysfs reflects the
software organization of the kernel as much as the physical organization
of the computer system.

This will have to be true in general.  A device is attached to a bus, so
there will be an object created by the bus driver as well as an object
created by the device driver.  Both refer to the same actual physical
device; that there are two sysfs nodes is an artifact of the way the
kernel is written.  And in fact, there are probably cases where the two
drivers live in the same program module and so only need to create own
object which they can jointly own.  In other cases the device driver
doesn't need to register its device in sysfs at all, so there's only the
bus driver's object.

Furthermore, the organization of sysfs itself is difficult to follow.  
Object representing physical devices are supposed to live in the hierarchy
below /sys/devices/, right?  Except for things like disks, which live
below /sys/block/.  And other things that live elsewhere.  It's not at all
clear where to look when searching for the object that represents a
particular device.  What's the principle for deciding whether to put
something under /sys/devices/ or /sys/class/ ?  I notice that the SCSI
system has put objects representing my USB CD-RW drive in
/sys/devices/pci0/0000:00:07.2/usb1/1-1/1-1:0/host0/ and in
/sys/class/scsi_host/host0/ .  These objects are created by the same
software and represent the same thing -- why does there need to be two of
them?  At a minimum, why shouldn't one be a link to the other?  How come
they contain differing attributes? -- and if I were to look for a
particular attribute, how would I know which directory to look in?

In the absence of a firm organizational policy, sysfs will become a 
nightmare of objects, attributes, and links distributed all over the place 
higgledy-piggledy, and no one will know where anything goes.  Now maybe 
things aren't as bad as I've made out, especially seeing as how I'm 
relatively new to all this.  But if there is some such policy, it doesn't 
appear to be clearly stated anywhere.

Sorry for rambling on and maybe appearing a bit sarcastic at times.

Alan Stern

Re: Flaw in the driver-model implementation of attributes

[Kevin P. Fleming]

Alan Stern wrote:

> For example, on my system the master device on the first PCI IDE channel
> is a hard disk, hda in fact.  This means that
> /sys/devices/pci0/0000:00:07.1/ide0/0.0/ and /sys/block/hda/ refer to the
> same physical device.  One is created by the IDE bus driver, the other by
> a block device driver.  Granted, there are links from one to the other,
> but it still indicates that the organization of sysfs reflects the
> software organization of the kernel as much as the physical organization
> of the computer system.
> 

PMJFI, and I'm not driver model expert, but I can think I can answer 
this one. Yes, you are correct, these two sysfs directories are 
associated with the same physical devices. However, they are definitely 
two different things.

The first is an IDE device. It has attributes common to IDE devices, 
like DMA/PIO mode, cable type, bus speed, etc.

The second is a block device. A _generic_ block device. It has 
attributes like length, dev (its device number), I/O scheduler settings, 
etc.

These are two wildly differing views, but yes, they are the same device. 
These differing attributes do _not_ belong in the same directory. An 
application looking at your IDE devices does not really care how the 
block subsystem perceives those devices (i.e. hdparm). Conversely, an 
application looking at your block devices should not care about what 
underlying physical devices (if any :-) they are associated with.

Re: Flaw in the driver-model implementation of attributes

[Greg KH]

On Mon, Jun 16, 2003 at 05:29:00PM -0400, Alan Stern wrote:
> On Mon, 16 Jun 2003, Patrick Mochel wrote:
> 
> > > This doesn't provide any really good place to put device attributes that 
> > > are owned by the driver.  They can't go in
> > > 	/sysfs/class/pcmcia_socket/pcmcia_socket0/device/...
> > > because the driver doesn't own the device.  They can't go in
> > > 	/sysfs/class/pcmcia_socket/pcmcia_socket0/driver/...
> > > because they aren't attributes of the _driver_, they're attributes of the 
> > > _device_.  And they certainly aren't class attributes.
> > > 
> > > So where would you put them?  You'd have to create another subdirectory of
> > > 	/sysfs/class/pcmcia_socket/pcmcia_socket0/
> > > owned by the driver.  No really good name for this subdirectory spings 
> > > to mind, and it's still kind of awkward.  But doable.
> > 
> > What is wrong with putting them in 
> > 
> > /sysfs/class/pcmcia_socket/pcmcia_socket0/
> > 
> > ? The driver owns that object. And, if it is device-specific feature, then 
> > it is likely related to what class the device belongs to, and is therefore 
> > relevant for that directory.
> 
> Are you sure?  Suppose a pcmcia disk drive is plugged in to that socket.  
> Why is a disk driver going to name its object "pcmcia_socket0"?  It must
> be the pcmcia socket driver that owns the object, not the disk driver.  So 
> then where does the disk driver put the disk-related attributes?  Don't 
> say in /sys/class/pcmcia_socket/pcmcia_socket0/device/, because the driver 
> doesn't own that object either.

All disk info is in the /sys/block directory, does that work for you?

> Anyway, in my case it's a USB device.  At the moment /sys/class/usb
> doesn't contain anything AFAICT, although no doubt that can be changed.

It will contain things if you have a device that uses the USB major
plugged in.

> Greg, what is the current organization of directories for USB buses and 
> devices?  Would it make sense to let each driver register its devices as
> /sys/class/usb/bus#/dev#/ ?  Would that better be done under /sys/bus/usb 
> ?  Or is something else already there?  Or should the usb-storage driver, 
> for example, create its own class?

I think the scsi core will create you a directory that you can use that
will have the proper lifetime that you are looking for.  If not, I can
look into doing something else for some of the other USB devices that
are not using the USB major.

thanks,

greg k-h

Re: Flaw in the driver-model implementation of attributes

[Alan Stern]

On Mon, 16 Jun 2003, Greg KH wrote:

> All disk info is in the /sys/block directory, does that work for you?

Not scsi disk info.  (Or maybe it should be there but it isn't.)  And no,
it doesn't work for me because it's owned by the scsi core, not my driver.

> I think the scsi core will create you a directory that you can use that
> will have the proper lifetime that you are looking for.  If not, I can
> look into doing something else for some of the other USB devices that
> are not using the USB major.

I don't think it would be appropriate to use that directory, since my 
driver wouldn't own it.

How about creating a /sys/class/usb/usb-storage/ directory, under which
there could be a directory for each USB mass-storage device?  Or would it 
be better to create a usb-storage.# directory under the interface's 
directory in /sys/devices/ ?

It's worth pointing out that both the OHCI and EHCI drivers also do the
same wrong thing.  They create their attribute files in a directory
owned by the PCI driver.

Alan Stern

Re: Flaw in the driver-model implementation of attributes

[Greg KH]

On Tue, Jun 17, 2003 at 01:29:31PM -0400, Alan Stern wrote:
> On Mon, 16 Jun 2003, Greg KH wrote:
> 
> > All disk info is in the /sys/block directory, does that work for you?
> 
> Not scsi disk info.  (Or maybe it should be there but it isn't.)  And no,
> it doesn't work for me because it's owned by the scsi core, not my driver.
> 
> > I think the scsi core will create you a directory that you can use that
> > will have the proper lifetime that you are looking for.  If not, I can
> > look into doing something else for some of the other USB devices that
> > are not using the USB major.
> 
> I don't think it would be appropriate to use that directory, since my 
> driver wouldn't own it.
> 
> How about creating a /sys/class/usb/usb-storage/ directory, under which
> there could be a directory for each USB mass-storage device?  Or would it 
> be better to create a usb-storage.# directory under the interface's 
> directory in /sys/devices/ ?

class/usb-storage/ would be fine with me.

> It's worth pointing out that both the OHCI and EHCI drivers also do the
> same wrong thing.  They create their attribute files in a directory
> owned by the PCI driver.

Yup, you are correct, time to add class/usb-host/ :)

thanks,

greg k-h

Re: Flaw in the driver-model implementation of attributes

[Alan Stern]

On Tue, 17 Jun 2003, Greg KH wrote:

> > How about creating a /sys/class/usb/usb-storage/ directory, under which
> > there could be a directory for each USB mass-storage device?  Or would it 
> > be better to create a usb-storage.# directory under the interface's 
> > directory in /sys/devices/ ?
> 
> class/usb-storage/ would be fine with me.
> 
> > It's worth pointing out that both the OHCI and EHCI drivers also do the
> > same wrong thing.  They create their attribute files in a directory
> > owned by the PCI driver.
> 
> Yup, you are correct, time to add class/usb-host/ :)

By the time we're done, there will be /sys/class/drivertype for every
driver in the system!

Seriously, do you think /sys/class/usb-storage is really appropriate?  It 
would be like creating /sys/class/ide-devices.  Wouldn't something under 
/sys/class/usb/ be better, say /sys/class/usb/mass-storage/ ?

Or would it be still better simply to create something like 
/sys/devices/pci0/0000:00:07.2/usb1/1-1/1-1:0/storage/ ?  A little 
confusing perhaps, because it would refer to the same physical device as 
its parent, but not inappropriate.

Alan Stern

Flaw in the driver-model implementation of attributes

[Alan Stern]

If you're already aware of this, please forgive the intrusion.

There's a general problem in the driver model's implementation of 
attribute files, in connection with loadable kernel modules.  The 
sysfs_ops structure stores function pointers with no means for identifying 
the module that contains the corresponding code.  As a result, it's 
possible to call through one of these pointers even after the module has 
been unloaded, causing an oops.

It's not hard to provoke this sort of situation.  A user process can
open a sysfs device file, for instance, and delay trying to read it until 
the module containing the device driver has been removed.  When the read 
does occur, it runs into trouble.

I don't know enough about the innards of the system to be able to fix this
properly.  One possible approach works like this.  Modify fs/sysfs/file.c
to make fill_read_buffer() and flush_write_buffer() acquire some sort of
read lock on file->f_dentry before they set attr =
file->f_dentry->d_fsdata.  Modify sysfs_remove_file() to acquire a write
lock and have it set file->f_dentry->d_fsdata to NULL.  Then it will only
be necessary to avoid calling ops->show() or ops->store() if attr is NULL.  
This guarantees that no caller will execute the show() or store() methods
after syfs_remove_file() has returned, so a driver that cleans up after 
itself correctly will not be invoked after it has been unloaded.

Alan Stern

Re: Flaw in the driver-model implementation of attributes

[Jeremy Fitzhardinge]

On Sun, 2003-06-15 at 09:42, Alan Stern wrote:
> If you're already aware of this, please forgive the intrusion.
> 
> There's a general problem in the driver model's implementation of 
> attribute files, in connection with loadable kernel modules.  The 
> sysfs_ops structure stores function pointers with no means for identifying 
> the module that contains the corresponding code.  As a result, it's 
> possible to call through one of these pointers even after the module has 
> been unloaded, causing an oops.
> 
> It's not hard to provoke this sort of situation.  A user process can
> open a sysfs device file, for instance, and delay trying to read it until 
> the module containing the device driver has been removed.  When the read 
> does occur, it runs into trouble.

I've seen this oops when a program has its cwd in a /sys directory
corresponding to a removed (or replaced) module.  I think active
references to a part of the /sys namespace corresponding to a module
should just pin the module.  But I haven't looked into it really.
	
	J

Re: Flaw in the driver-model implementation of attributes

[Alan Stern]

On 15 Jun 2003, Jeremy Fitzhardinge wrote:

> On Sun, 2003-06-15 at 09:42, Alan Stern wrote:
> > If you're already aware of this, please forgive the intrusion.
> > 
> > There's a general problem in the driver model's implementation of 
> > attribute files, in connection with loadable kernel modules.  The 
> > sysfs_ops structure stores function pointers with no means for identifying 
> > the module that contains the corresponding code.  As a result, it's 
> > possible to call through one of these pointers even after the module has 
> > been unloaded, causing an oops.
> > 
> > It's not hard to provoke this sort of situation.  A user process can
> > open a sysfs device file, for instance, and delay trying to read it until 
> > the module containing the device driver has been removed.  When the read 
> > does occur, it runs into trouble.
> 
> I've seen this oops when a program has its cwd in a /sys directory
> corresponding to a removed (or replaced) module.  I think active
> references to a part of the /sys namespace corresponding to a module
> should just pin the module.  But I haven't looked into it really.

The question of which references should pin a module is a design decision 
that I prefer to leave to others.  But I would think that a reference the 
module can quickly and easily revoke probably shouldn't pin it, whereas 
one the module isn't even aware of probably should.

That's the difference between the example you gave and the problem I
cited.  In your example, the fact that your program parked its cwd in a
sysfs directory would force the reference count of the corresponding
kobject to be nonzero.  That the module was willing to deallocate the
kobject and exit even though the reference count was still positive is a
simple programming error.  But in the situation I described, the design of
the driver model makes it impossible for a driver to know whether any user
processes still have references to a device attribute file after
device_remove_file() has been called.

Alan Stern

Re: Flaw in the driver-model implementation of attributes

[Greg KH]

On Sun, Jun 15, 2003 at 12:42:26PM -0400, Alan Stern wrote:
> If you're already aware of this, please forgive the intrusion.
> 
> There's a general problem in the driver model's implementation of 
> attribute files, in connection with loadable kernel modules.  The 
> sysfs_ops structure stores function pointers with no means for identifying 
> the module that contains the corresponding code.  As a result, it's 
> possible to call through one of these pointers even after the module has 
> been unloaded, causing an oops.

That's why CONFIG_MODULE_UNLOAD is a new option, if you don't want to
take the risk, don't enable it :)

> It's not hard to provoke this sort of situation.  A user process can
> open a sysfs device file, for instance, and delay trying to read it until 
> the module containing the device driver has been removed.  When the read 
> does occur, it runs into trouble.

Then don't let your module unload until _all_ instances of your
structures are gone.  You can tell if this is true or not, it's just up
to the implementor :)

Look at the new pcmcia code for just such an example.

If this is in regards to the scsi usage of sysfs, I've been talking to
Mike Anderson a lot about this recently.  People are having to realize a
few new things with regards to kernel programming that previously they
have not had to worry about:
  - in the past, the only thing that could ever go away while a kernel
    was running was a module.  So people worried about module reference
    counts, and hence the /proc useage of module counts.
  - With the advent of hotplug devices, we now have to worry about
    individual devices going away, not so much modules.  This is causing
    layers like the scsi one to be changed a lot to accommodate this
    (any pci scsi device can be removed at any time thanks to pci
    hotplug systems, and the scsi layer can't really handle this very
    well at all until very recently.)

So, the driver model helps out with handling the fact that devices can
go away at any time a lot easier than anything we've had before.  It's
now up to the individual kernel modules to control their own module
reference counts to handle if they want to be able to be unloaded before
all of their references are gone now or not.

Look at the usb hid drivers, they _never_ set their reference count :)

Hope this helps,

greg k-h

Re: Flaw in the driver-model implementation of attributes

[Russell King]

On Mon, Jun 16, 2003 at 10:08:26AM -0700, Greg KH wrote:
> Then don't let your module unload until _all_ instances of your
> structures are gone.  You can tell if this is true or not, it's just up
> to the implementor :)

Greg, I believe Alan does have a valid concern.  Eg, how is the following
handled?

- PCI device driver module is loaded
- device driver gets handed a pci device
- device driver attaches a file to the struct device corresponding to the
  PCI device.
- userspace opens new file (this does not increment the device drivers
  use count.)
- device driver is rmmod'd
- device driver removes its references to the pci device
- device driver unloads
- user reads from opened file.

> Look at the new pcmcia code for just such an example.

In the pcmcia case, we effectively own the object (class device) we're
attaching the files to, so we can ensure that the module is not unloaded
until the class device files are closed and all references to the object
are gone.

IMO, if you don't own the object (and therefore don't know its lifetime),
you shouldn't be adding sysfs or device model attributes of any kind to
that object.

-- 
Russell King (rmk@arm.linux.org.uk)                The developer of ARM Linux
             http://www.arm.linux.org.uk/personal/aboutme.html

Re: Flaw in the driver-model implementation of attributes

[Alan Stern]

On Mon, 16 Jun 2003, Russell King wrote:

> On Mon, Jun 16, 2003 at 10:08:26AM -0700, Greg KH wrote:
> > Then don't let your module unload until _all_ instances of your
> > structures are gone.  You can tell if this is true or not, it's just up
> > to the implementor :)
> 
> Greg, I believe Alan does have a valid concern.  Eg, how is the following
> handled?
> 
> - PCI device driver module is loaded
> - device driver gets handed a pci device
> - device driver attaches a file to the struct device corresponding to the
>   PCI device.
> - userspace opens new file (this does not increment the device drivers
>   use count.)
> - device driver is rmmod'd
> - device driver removes its references to the pci device
> - device driver unloads
> - user reads from opened file.

Thank you, that was exactly my point.  Opening the file increments the 
device's refcount but not the driver's.

> > Look at the new pcmcia code for just such an example.
> 
> In the pcmcia case, we effectively own the object (class device) we're
> attaching the files to, so we can ensure that the module is not unloaded
> until the class device files are closed and all references to the object
> are gone.
> 
> IMO, if you don't own the object (and therefore don't know its lifetime),
> you shouldn't be adding sysfs or device model attributes of any kind to
> that object.

That's not practical.  How else can a device driver provide 
device-specific configuration options or information in sysfs?  In many 
cases the device is owned by the bus, not the device driver.

By the way, both the EHCI and OHCI host controller drivers do this.  It's 
easy to cause an oops by delaying a read of (for example) the "periodic" 
file they create until after the driver is unloaded.

Alan Stern

Re: Flaw in the driver-model implementation of attributes

[Patrick Mochel]

> > In the pcmcia case, we effectively own the object (class device) we're
> > attaching the files to, so we can ensure that the module is not unloaded
> > until the class device files are closed and all references to the object
> > are gone.
> > 
> > IMO, if you don't own the object (and therefore don't know its lifetime),
> > you shouldn't be adding sysfs or device model attributes of any kind to
> > that object.
> 
> That's not practical.  How else can a device driver provide 
> device-specific configuration options or information in sysfs?  In many 
> cases the device is owned by the bus, not the device driver.

Read it again. :) 

The driver is able to create a class-specific object for the device, which 
it owns entirely. Russell is suggesting that if you're going to export 
attributes, you best be doing them as attributes of the object you own. 

Otherwise, if you're a subordinate module, then you should explicitly 
increment the refcount of the module of the object you're exporting 
attributes for, and decrement it when your module is unloaded. Then again, 
this may happen implicitly with modules..


	-pat

Re: Flaw in the driver-model implementation of attributes

[viro]

On Mon, Jun 16, 2003 at 01:54:34PM -0400, Alan Stern wrote:

> > IMO, if you don't own the object (and therefore don't know its lifetime),
> > you shouldn't be adding sysfs or device model attributes of any kind to
> > that object.
> 
> That's not practical.  How else can a device driver provide 
> device-specific configuration options or information in sysfs?  In many 
> cases the device is owned by the bus, not the device driver.

Practical or not, when you put sysfs object into a structure, you take
full responsibility for the lifetime of that structure.  Period.

Note that problems exist even when kernel is non-modular.  Even if code
stays in place, the data getting freed under you is just as bad.  And
that can trivially happen without any modules.

Re: Flaw in the driver-model implementation of attributes

[Alan Stern]

On Mon, 16 Jun 2003 viro@parcelfarce.linux.theplanet.co.uk wrote:

> On Mon, Jun 16, 2003 at 01:54:34PM -0400, Alan Stern wrote:
> 
> > That's not practical.  How else can a device driver provide 
> > device-specific configuration options or information in sysfs?  In many 
> > cases the device is owned by the bus, not the device driver.
> 
> Practical or not, when you put sysfs object into a structure, you take
> full responsibility for the lifetime of that structure.  Period.

I agree.  The problem here is that the sysfs/driver-model core is putting
an object into a structure (i.e., a pointer to the device_attribute
structure is being stored in the dentry for a user-owned file) without
taking the corresponding responsibility for the lifetime of the driver
code that the attribute refers to.  The struct driver's reference count is 
_not_ incremented; in fact device_create_file() doesn't even _have_ a 
struct driver argument to say who the caller is.

You could make a good case that this ought to be fixed by changing either
the sysfs or the driver model code.  But in either case, the fault lies in
the core implementation, not in the driver.

> Note that problems exist even when kernel is non-modular.  Even if code
> stays in place, the data getting freed under you is just as bad.  And
> that can trivially happen without any modules.

Of course.

Alan Stern

Re: Flaw in the driver-model implementation of attributes

[Patrick Mochel]

> I agree.  The problem here is that the sysfs/driver-model core is putting
> an object into a structure (i.e., a pointer to the device_attribute
> structure is being stored in the dentry for a user-owned file) without
> taking the corresponding responsibility for the lifetime of the driver
> code that the attribute refers to.  The struct driver's reference count is 
> _not_ incremented; in fact device_create_file() doesn't even _have_ a 
> struct driver argument to say who the caller is.

That's not the problem, only one possible solution. The problem is that 
the driver does not own the object it's exporting the attribute for. 

The object who owns the attributes gets its refcount incremented when the 
file is opened. If the module owns that object, it can take measures on 
unload to wait for that reference count to reach 0. 

If a piece of code exports an attribute for an object that it does not 
own, it must explicitly modify the reference count for that object. There 
is no other way to be safe. 

Note that by pinning the driver in memory when you create a sysfs file 
that it owns will prevent the module from ever being unloaded. That is 
possible, but not desirable. Note that that change would make your point 
moot. :) 

Also, device_create_file() does not have a driver argument, because 
drivers are not the only entities that may create files for a device. The 
owning bus and the core itself may also use that call. Drivers are 
probably the least qualified (most unsafe) entities to do so.


	-pat

Re: Flaw in the driver-model implementation of attributes

[Alan Stern]

I'm forced to agree with most of what you say.

On Mon, 16 Jun 2003, Patrick Mochel wrote:

> That's not the problem, only one possible solution. The problem is that 
> the driver does not own the object it's exporting the attribute for. 
> 
> The object who owns the attributes gets its refcount incremented when the 
> file is opened. If the module owns that object, it can take measures on 
> unload to wait for that reference count to reach 0. 

Yes.  This may delay the rmmod system call (make it wait until some other 
user process has closed the attribute file), but who cares about that?

> If a piece of code exports an attribute for an object that it does not 
> own, it must explicitly modify the reference count for that object. There 
> is no other way to be safe. 

Even that's not enough to be safe.  Modifying the reference count doesn't 
help because this isn't a problem of the object being deleted, it's a 
problem of a struct device_attributes being deleted -- and they don't have 
reference counts since they aren't objects.

> Note that by pinning the driver in memory when you create a sysfs file 
> that it owns will prevent the module from ever being unloaded. That is 
> possible, but not desirable. Note that that change would make your point 
> moot. :) 

Again I agree.  That was never my intention.  The change I originally
suggested would simply make device_remove_file() atomic with respect
to readers and writers of the attribute file:  It wouldn't return until
all current readers or writers had left the show()/store() routine, and
after it returned nobody would call show()/store().  I still don't see any 
reason not to implement something like this.

> Also, device_create_file() does not have a driver argument, because 
> drivers are not the only entities that may create files for a device. The 
> owning bus and the core itself may also use that call. Drivers are 
> probably the least qualified (most unsafe) entities to do so.

Most unsafe, perhaps.  But least qualified?  Who can possibly be more 
qualified to display the state or affect the functioning of a device than 
its driver?!

However, you may very well approve of my previous suggestion (crossed in 
the email) for a driver-specific subdirectory of the device directory.  I 
think it's more awkward, but it fits in your framework better.

Alan Stern

Re: Flaw in the driver-model implementation of attributes

[Martin Diehl]

On Mon, 16 Jun 2003, Russell King wrote:

> On Mon, Jun 16, 2003 at 10:08:26AM -0700, Greg KH wrote:
> > Then don't let your module unload until _all_ instances of your
> > structures are gone.  You can tell if this is true or not, it's just up
> > to the implementor :)
> 
> Greg, I believe Alan does have a valid concern.  Eg, how is the following
> handled?
> 
> - PCI device driver module is loaded
> - device driver gets handed a pci device
> - device driver attaches a file to the struct device corresponding to the
>   PCI device.

with old procfs one would like to set the owner field of the 
corresponding struct proc_dir_entry and/or file_operations at this point.

> - userspace opens new file (this does not increment the device drivers
>   use count.)

given owner=THIS_MODULE was set, this would bump the module's use count

> - device driver is rmmod'd

and this could never happen while the procfs file (or directory) is still 
referenced

> - device driver removes its references to the pci device
> - device driver unloads
> - user reads from opened file.

Admittedly I haven't looked deeper into sysfs yet, but I was under the
assumption/hope there would be a similar approach to make module 
refcounting working there?

Martin

Re: Flaw in the driver-model implementation of attributes

[viro]

On Mon, Jun 16, 2003 at 08:00:33PM +0200, Martin Diehl wrote:
> with old procfs one would like to set the owner field of the 
> corresponding struct proc_dir_entry and/or file_operations at this point.
> 
> > - userspace opens new file (this does not increment the device drivers
> >   use count.)
> 
> given owner=THIS_MODULE was set, this would bump the module's use count

... and for objects that have lifetime different from that of any module
this approach fucks up with procfs just as badly as with sysctl or sysfs.

Folks, _forget_ modules.  ->owner is OK for many things, but for stuff
like procfs it's not enough.  It only protects code.  procfs and sysfs
entries are _data_.  And in cases when it is OK the data is protected
by separate refcounts.

Folks, please, stop assuming that rmmod is the root of all evil and/or
place to deal with said evil.  Objects can be and are destroyed regardless
of rmmod.