On Thu, Mar 27, 2008 at 12:38 PM, H. Peter Anvin <hpa@xxxxxxxxx> wrote:
> Kay Sievers wrote:
>
> >
> > >
> > > Probably something like /sys/class/block/MAJ:MIN
> > >
> >
> > "Devices directories" are not supposed to contain duplicate entries.
> > It would slow-down, or may even break things.
> >
> >
> > > or /sys/class/devnums/bMAJ:MIN?
> > >
> >
> > These are no devices belonging to the class "devnums", so it may
> > confuse things which crawl these directories to get "all devices".
> > Current coldplug-like setups will likely add duplicate devices with
> > the wrong subsystem. There are also bus-devices with have a dev_t, and
> > that will make them show up in /sys/class, which might confuse some
> > tools too.
> >
> > I guess we will need to find some other solution as a /sys/class/ for
> > that. And we must prefix the links with 'c' and 'b' because dev_t is
> > not unique across char and block devices.
> >
> >
>
> It doesn't really seem to be to belong under class at all. I would suggest
> /sys/dev/char/ and /sys/dev/block/, for char and block respectively.
>
This thread fizzled out without a patch... here goes:
[ note: I'm replying via gmail, so if it has whitespace mangled the
patch please see the attachment ]
-----snip---->
sysfs: add /sys/dev/{char,block} to lookup sysfs path by major:minor
From: Dan Williams <dan.j.williams@xxxxxxxxx>
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 <neilb@xxxxxxx>
Cc: Tejun Heo <htejun@xxxxxxxxx>
Cc: Kay Sievers <kay.sievers@xxxxxxxx>
Cc: Greg KH <gregkh@xxxxxxx>
Cc: Mark Lord <lkml@xxxxxx>
Cc: H. Peter Anvin <hpa@xxxxxxxxx>
Signed-off-by: Dan Williams <dan.j.williams@xxxxxxxxx>
---
drivers/base/core.c | 37 ++++++++++++++++++++++++++++++++++++-
1 files changed, 36 insertions(+), 1 deletions(-)
diff --git a/drivers/base/core.c b/drivers/base/core.c
index 24198ad..de925f8 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -27,6 +27,9 @@
int (*platform_notify)(struct device *dev) = NULL;
int (*platform_notify_remove)(struct device *dev) = NULL;
+static struct kobject *dev_kobj;
+static struct kobject *char_kobj;
+static struct kobject *block_kobj;
#ifdef CONFIG_BLOCK
static inline int device_is_not_partition(struct device *dev)
@@ -759,6 +762,11 @@ static void device_remove_class_symlinks(struct
device *dev)
sysfs_remove_link(&dev->kobj, "subsystem");
}
+static struct kobject *device_to_dev_kobj(struct device *dev)
+{
+ return dev->class == &block_class ? block_kobj : char_kobj;
+}
+
/**
* device_add - add device to device hierarchy.
* @dev: device.
@@ -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];
dev = get_device(dev);
if (!dev || !strlen(dev->bus_id)) {
@@ -806,9 +815,16 @@ int device_add(struct device *dev)
goto attrError;
if (MAJOR(dev->devt)) {
+ struct kobject *kobj = device_to_dev_kobj(dev);
+
error = device_create_file(dev, &devt_attr);
if (error)
goto ueventattrError;
+
+ format_dev_t(devt_str, dev->devt);
+ error = sysfs_create_link(kobj, &dev->kobj, devt_str);
+ if (error)
+ goto devtattrError;
}
error = device_add_class_symlinks(dev);
@@ -854,6 +870,9 @@ int device_add(struct device *dev)
device_remove_class_symlinks(dev);
SymlinkError:
if (MAJOR(dev->devt))
+ sysfs_remove_link(device_to_dev_kobj(dev), devt_str);
+ devtattrError:
+ if (MAJOR(dev->devt))
device_remove_file(dev, &devt_attr);
ueventattrError:
device_remove_file(dev, &uevent_attr);
@@ -925,12 +944,16 @@ void device_del(struct device *dev)
{
struct device *parent = dev->parent;
struct class_interface *class_intf;
+ char devt_str[25];
device_pm_remove(dev);
if (parent)
klist_del(&dev->knode_parent);
- if (MAJOR(dev->devt))
+ if (MAJOR(dev->devt)) {
+ format_dev_t(devt_str, dev->devt);
+ sysfs_remove_link(device_to_dev_kobj(dev), devt_str);
device_remove_file(dev, &devt_attr);
+ }
if (dev->class) {
device_remove_class_symlinks(dev);
@@ -1055,6 +1078,15 @@ int __init devices_init(void)
devices_kset = kset_create_and_add("devices", &device_uevent_ops, NULL);
if (!devices_kset)
return -ENOMEM;
+ dev_kobj = kobject_create_and_add("dev", NULL);
+ if (!dev_kobj)
+ return -ENOMEM;
+ block_kobj = kobject_create_and_add("block", dev_kobj);
+ if (!block_kobj)
+ return -ENOMEM;
+ char_kobj = kobject_create_and_add("char", dev_kobj);
+ if (!char_kobj)
+ return -ENOMEM;
return 0;
}
@@ -1380,4 +1412,7 @@ void device_shutdown(void)
dev->driver->shutdown(dev);
}
}
+ kobject_put(char_kobj);
+ kobject_put(block_kobj);
+ kobject_put(dev_kobj);
}
sysfs: add /sys/dev/{char,block} to lookup sysfs path by major:minor
From: Dan Williams <dan.j.williams@xxxxxxxxx>
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 <neilb@xxxxxxx>
Cc: Tejun Heo <htejun@xxxxxxxxx>
Cc: Kay Sievers <kay.sievers@xxxxxxxx>
Cc: Greg KH <gregkh@xxxxxxx>
Cc: Mark Lord <lkml@xxxxxx>
Cc: H. Peter Anvin <hpa@xxxxxxxxx>
Signed-off-by: Dan Williams <dan.j.williams@xxxxxxxxx>
---
drivers/base/core.c | 37 ++++++++++++++++++++++++++++++++++++-
1 files changed, 36 insertions(+), 1 deletions(-)
diff --git a/drivers/base/core.c b/drivers/base/core.c
index 24198ad..de925f8 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -27,6 +27,9 @@
int (*platform_notify)(struct device *dev) = NULL;
int (*platform_notify_remove)(struct device *dev) = NULL;
+static struct kobject *dev_kobj;
+static struct kobject *char_kobj;
+static struct kobject *block_kobj;
#ifdef CONFIG_BLOCK
static inline int device_is_not_partition(struct device *dev)
@@ -759,6 +762,11 @@ static void device_remove_class_symlinks(struct device *dev)
sysfs_remove_link(&dev->kobj, "subsystem");
}
+static struct kobject *device_to_dev_kobj(struct device *dev)
+{
+ return dev->class == &block_class ? block_kobj : char_kobj;
+}
+
/**
* device_add - add device to device hierarchy.
* @dev: device.
@@ -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];
dev = get_device(dev);
if (!dev || !strlen(dev->bus_id)) {
@@ -806,9 +815,16 @@ int device_add(struct device *dev)
goto attrError;
if (MAJOR(dev->devt)) {
+ struct kobject *kobj = device_to_dev_kobj(dev);
+
error = device_create_file(dev, &devt_attr);
if (error)
goto ueventattrError;
+
+ format_dev_t(devt_str, dev->devt);
+ error = sysfs_create_link(kobj, &dev->kobj, devt_str);
+ if (error)
+ goto devtattrError;
}
error = device_add_class_symlinks(dev);
@@ -854,6 +870,9 @@ int device_add(struct device *dev)
device_remove_class_symlinks(dev);
SymlinkError:
if (MAJOR(dev->devt))
+ sysfs_remove_link(device_to_dev_kobj(dev), devt_str);
+ devtattrError:
+ if (MAJOR(dev->devt))
device_remove_file(dev, &devt_attr);
ueventattrError:
device_remove_file(dev, &uevent_attr);
@@ -925,12 +944,16 @@ void device_del(struct device *dev)
{
struct device *parent = dev->parent;
struct class_interface *class_intf;
+ char devt_str[25];
device_pm_remove(dev);
if (parent)
klist_del(&dev->knode_parent);
- if (MAJOR(dev->devt))
+ if (MAJOR(dev->devt)) {
+ format_dev_t(devt_str, dev->devt);
+ sysfs_remove_link(device_to_dev_kobj(dev), devt_str);
device_remove_file(dev, &devt_attr);
+ }
if (dev->class) {
device_remove_class_symlinks(dev);
@@ -1055,6 +1078,15 @@ int __init devices_init(void)
devices_kset = kset_create_and_add("devices", &device_uevent_ops, NULL);
if (!devices_kset)
return -ENOMEM;
+ dev_kobj = kobject_create_and_add("dev", NULL);
+ if (!dev_kobj)
+ return -ENOMEM;
+ block_kobj = kobject_create_and_add("block", dev_kobj);
+ if (!block_kobj)
+ return -ENOMEM;
+ char_kobj = kobject_create_and_add("char", dev_kobj);
+ if (!char_kobj)
+ return -ENOMEM;
return 0;
}
@@ -1380,4 +1412,7 @@ void device_shutdown(void)
dev->driver->shutdown(dev);
}
}
+ kobject_put(char_kobj);
+ kobject_put(block_kobj);
+ kobject_put(dev_kobj);
}
