Skip to contentSkip to conversation

Flaw in the driver-model implementation of attributes

DISCUSSION Alan Stern 4 participants 10 messages Jun 16, 2003 – Jun 18, 2003 1 message not in the archive

lore
Share & tools
lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

Alan Stern

author
#1 lore raw
Quoted 20 lines · Russell King

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/...

Quoted 2 lines

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

#2 lore raw
Quoted 9 lines

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.

Quoted 4 lines

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

author
#3 lore raw
Quoted 13 lines · Patrick Mochel
Quoted 8 lines

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.

Quoted 4 lines

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.

Quoted 7 lines

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

#4 lore raw
Quoted 7 lines

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

author
#5 lore raw
Quoted 11 lines · Patrick Mochel
Quoted 6 lines

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.

Quoted 6 lines

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

#6 lore raw
Quoted 9 lines · Alan Stern

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.

#7 lore raw
In reply to Alan Stern
Quoted 23 lines · Alan Stern
Quoted 13 lines · Patrick Mochel
Quoted 8 lines

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.

Quoted 4 lines

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.

Quoted 7 lines

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.

Quoted 6 lines

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?

Quoted 2 lines

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.

Quoted 5 lines

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

author
#8 lore raw
Quoted 1 line · Greg KH

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.

Quoted 4 lines

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

#9 lore raw
Quoted 12 lines · Alan Stern
Quoted 1 line · Greg KH

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.

Quoted 4 lines

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.

Quoted 7 lines

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.

Quoted 3 lines

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

author
#10 lore raw
Quoted 5 lines · Greg KH
Quoted 4 lines

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/ ?

Quoted 7 lines

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

Quoted 3 lines

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