Skip to contentSkip to conversation

sysfs: add /sys/dev/{char, block} to lookup sysfs path by major:minor

PATCH In review Dan Williams 3 participants 5 messages Apr 14, 2008 – Apr 15, 2008

Subsystems (2)
Documentation Driver core
lore
Share & tools
lore

Dan Williams

author
#0 lore raw

PATCH 1/1 sysfs: add /sys/dev/{char, block} to lookup sysfs path by major:minor

Why?:
There are occasions where userspace would like to access sysfs attributes for a device but it may not know how sysfs has named the device or the path. For example what is the sysfs path for /dev/disk/by-id/ata-ST3160827AS_5MT004CK? With this change a call to stat(2) returns the major:minor then userspace can see that /sys/dev/block/8:32 links to /sys/block/sdc.

What are the alternatives?:
1/ Add an ioctl to return the path: Doable, but sysfs is meant to reduce
the need to proliferate ioctl interfaces into the kernel, so this seems counter productive.

2/ Use udev to create these symlinks: Also doable, but it adds a
udev dependency to utilities that might be running in a limited environment like an initramfs.

Cc NeilBrown <[email protected]>
Cc Tejun Heo <[email protected]>
Cc Kay Sievers <[email protected]>
Cc Greg KH <[email protected]>
Cc Mark Lord <[email protected]>
Cc H. Peter Anvin <[email protected]>
Signed-off-by Dan Williams <[email protected]>

---
This is the second revision of the patch originally posted here:
http://marc.info/?l=linux-kernel&m=120795638915272&w=2

* Fixed up ENOMEM handling in devices_init()
* Added a short blurb in Documentation/filesystems/sysfs.txt

 Documentation/filesystems/sysfs.txt |    6 +++++
 drivers/base/core.c                 |   46 ++++++++++++++++++++++++++++++++++-
 2 files changed, 51 insertions(+), 1 deletions(-)
Documentation/filesystems/sysfs.txt +6 -0
index 7f27b8f..9e9c348 100644
@@ -248,6 +248,7 @@ The top level sysfs directory looks like:
 block/
 bus/
 class/
+dev/
 devices/
 firmware/
 net/

@@ -274,6 +275,11 @@ fs/ contains a directory for some filesystems.  Currently each
 filesystem wanting to export attributes must create its own hierarchy
 below fs/ (see ./fuse.txt for an example).
 
+dev/ contains two directories char/ and block/. Inside these two
+directories there are symlinks named <major>:<minor>.  These symlinks
+point to the sysfs directory for the given device.  /sys/dev provides a
+quick way to lookup the sysfs interface for a device from the result of
+a stat(2) operation.
 
 More information can driver-model specific features can be found in
 Documentation/driver-model/.
drivers/base/core.c +45 -1

101 lines not on this page · raw message

#1 lore raw
Quoted 10 lines · Dan Williams

What are the alternatives?:
1/ Add an ioctl to return the path: Doable, but sysfs is meant to reduce
the need to proliferate ioctl interfaces into the kernel, so this seems counter productive.

2/ Use udev to create these symlinks: Also doable, but it adds a
udev dependency to utilities that might be running in a limited environment like an initramfs.

3, of course, is what applications have to do today, which is to do a full-tree search of sysfs.

Given that this appears to be fairly little code it would make sense to me; one could argue if /sys/dev/block/8/32 would be better than /sys/dev/block/8:32, but (a) I don't think it matters and (b) the non-atomicity of the former would

-hpa

Reviewed-by on v1 1/1
#2 lore raw
In reply to Dan Williams
Quoted 9 lines · Dan Williams

This is the second revision of the patch originally posted here:
http://marc.info/?l=linux-kernel&m=120795638915272&w=2

* Fixed up ENOMEM handling in devices_init()
* Added a short blurb in Documentation/filesystems/sysfs.txt

  Documentation/filesystems/sysfs.txt |    6 +++++
  drivers/base/core.c                 |   46 ++++++++++++++++++++++++++++++++++-
  2 files changed, 51 insertions(+), 1 deletions(-)

I've looked this patch over and I have some comments. The logic looks correct, but there are two ugly lines.

Quoted 11 lines

@@ -775,6 +783,7 @@ int device_add(struct device *dev)

        struct device *parent = NULL;
        struct class_interface *class_intf;
        int error;

+ char devt_str[25];

@@ -925,12 +944,16 @@ void device_del(struct device *dev)

  {
        struct device *parent = dev->parent;
        struct class_interface *class_intf;

+ char devt_str[25];

May I ask why 25'? The only other user of format_dev_t that I could find in a quick grep is md (device-mapper) and they used a hardcoded 15' there. The real problem is format_dev_t and print_dev_t.

If the only other user of those macros which want to be C inlines with buffer size parameters is md, perhaps now would be a good time to clean them up before adding more users?

Otherwise, the logic looks O.K.

Add my

Reviewed-by SL Baur <[email protected]>

if that is appropriate.

-sb

author
#3 lore raw
Quoted 33 lines · SL Baur
Quoted 9 lines · Dan Williams

This is the second revision of the patch originally posted here:
http://marc.info/?l=linux-kernel&m=120795638915272&w=2

* Fixed up ENOMEM handling in devices_init()
* Added a short blurb in Documentation/filesystems/sysfs.txt

  Documentation/filesystems/sysfs.txt |    6 +++++
  drivers/base/core.c                 |   46 ++++++++++++++++++++++++++++++++++-
  2 files changed, 51 insertions(+), 1 deletions(-)

I've looked this patch over and I have some comments. The logic looks correct, but there are two ugly lines.

Quoted 4 lines

@@ -775,6 +783,7 @@ int device_add(struct device *dev)

        struct device *parent = NULL;
        struct class_interface *class_intf;
        int error;
Quoted 8 lines

+ char devt_str[25];

@@ -925,12 +944,16 @@ void device_del(struct device *dev)

  {
        struct device *parent = dev->parent;
        struct class_interface *class_intf;

+ char devt_str[25];

May I ask why 25'? The only other user of format_dev_t that I could find in a quick grep is md (device-mapper) and they used a hardcoded 15' there. The real problem is format_dev_t and print_dev_t.

Quoted 6 lines

If the only other user of those macros which want to be C inlines with buffer size parameters is md, perhaps now would be a good time to clean them up before adding more users?

Otherwise, the logic looks O.K.

Thanks for looking it over. To be honest I was iffy about those buffer sizes as well, and looking closer they are overkill. Although, they are smaller than the 32-byte buffer in bsg.c :-). 13-bytes is all that is needed given a 12-bit major and 20-bit minor. Care to send a patch to fix up format_dev_t and print_dev_t?

Quoted 4 lines

Add my

Reviewed-by SL Baur <[email protected]>

if that is appropriate.

Yes, appropriate.

Thanks,
Dan

#4 lore raw
Quoted 8 lines · Dan Williams
Quoted 1 line · SL Baur

On 4/14/08, Dan Williams <[email protected]> wrote:

Thanks for looking it over. To be honest I was iffy about those
buffer sizes as well, and looking closer they are overkill. Although, they are smaller than the 32-byte buffer in bsg.c :-). 13-bytes is all that is needed given a 12-bit major and 20-bit minor. Care to send a patch to fix up format_dev_t and print_dev_t?

O.K. I'll look into it. The call paths into format_dev_t are clear, the ones into print_dev_t look a bit more global.

-sb