* [PATCH] sysfs: add /sys/dev/{char, block} to lookup sysfs path by major:minor
@ 2008-04-14 16:57 Dan Williams
2008-04-14 17:00 ` H. Peter Anvin
2008-04-15 3:13 ` SL Baur
0 siblings, 2 replies; 5+ messages in thread
From: Dan Williams @ 2008-04-14 16:57 UTC (permalink / raw
To: gregkh; +Cc: linux-kernel, kay.sievers, neilb, htejun, hpa, lkml
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@suse.de>
Cc: Tejun Heo <htejun@gmail.com>
Cc: Kay Sievers <kay.sievers@vrfy.org>
Cc: Greg KH <gregkh@suse.de>
Cc: Mark Lord <lkml@rtr.ca>
Cc: H. Peter Anvin <hpa@zytor.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
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(-)
diff --git a/Documentation/filesystems/sysfs.txt b/Documentation/filesystems/sysfs.txt
index 7f27b8f..9e9c348 100644
--- a/Documentation/filesystems/sysfs.txt
+++ b/Documentation/filesystems/sysfs.txt
@@ -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/.
diff --git a/drivers/base/core.c b/drivers/base/core.c
index 24198ad..39db294 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,7 +1078,25 @@ 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)
+ goto dev_kobj_err;
+ block_kobj = kobject_create_and_add("block", dev_kobj);
+ if (!block_kobj)
+ goto block_kobj_err;
+ char_kobj = kobject_create_and_add("char", dev_kobj);
+ if (!char_kobj)
+ goto char_kobj_err;
+
return 0;
+
+ char_kobj_err:
+ kobject_put(block_kobj);
+ block_kobj_err:
+ kobject_put(dev_kobj);
+ dev_kobj_err:
+ kset_unregister(devices_kset);
+ return -ENOMEM;
}
EXPORT_SYMBOL_GPL(device_for_each_child);
@@ -1380,4 +1421,7 @@ void device_shutdown(void)
dev->driver->shutdown(dev);
}
}
+ kobject_put(char_kobj);
+ kobject_put(block_kobj);
+ kobject_put(dev_kobj);
}
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] sysfs: add /sys/dev/{char, block} to lookup sysfs path by major:minor
2008-04-14 16:57 [PATCH] sysfs: add /sys/dev/{char, block} to lookup sysfs path by major:minor Dan Williams
@ 2008-04-14 17:00 ` H. Peter Anvin
2008-04-15 3:13 ` SL Baur
1 sibling, 0 replies; 5+ messages in thread
From: H. Peter Anvin @ 2008-04-14 17:00 UTC (permalink / raw
To: Dan Williams; +Cc: gregkh, linux-kernel, kay.sievers, neilb, htejun, lkml
Dan Williams wrote:
>
> 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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] sysfs: add /sys/dev/{char, block} to lookup sysfs path by major:minor
2008-04-14 16:57 [PATCH] sysfs: add /sys/dev/{char, block} to lookup sysfs path by major:minor Dan Williams
2008-04-14 17:00 ` H. Peter Anvin
@ 2008-04-15 3:13 ` SL Baur
2008-04-15 4:46 ` Dan Williams
1 sibling, 1 reply; 5+ messages in thread
From: SL Baur @ 2008-04-15 3:13 UTC (permalink / raw
To: Dan Williams; +Cc: gregkh, linux-kernel, kay.sievers, neilb, htejun, hpa, lkml
On 4/14/08, Dan Williams <dan.j.williams@intel.com> wrote:
> 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.
> @@ -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 <steve@xemacs.org>
if that is appropriate.
-sb
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] sysfs: add /sys/dev/{char, block} to lookup sysfs path by major:minor
2008-04-15 3:13 ` SL Baur
@ 2008-04-15 4:46 ` Dan Williams
2008-04-15 7:08 ` SL Baur
0 siblings, 1 reply; 5+ messages in thread
From: Dan Williams @ 2008-04-15 4:46 UTC (permalink / raw
To: SL Baur; +Cc: gregkh, linux-kernel, kay.sievers, neilb, htejun, hpa, lkml
On Mon, Apr 14, 2008 at 8:13 PM, SL Baur <steve@xemacs.org> wrote:
> On 4/14/08, Dan Williams <dan.j.williams@intel.com> wrote:
>
> > 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.
>
>
> > @@ -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.
>
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?
> Add my
> Reviewed-by: SL Baur <steve@xemacs.org>
> if that is appropriate.
>
Yes, appropriate.
Thanks,
Dan
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] sysfs: add /sys/dev/{char, block} to lookup sysfs path by major:minor
2008-04-15 4:46 ` Dan Williams
@ 2008-04-15 7:08 ` SL Baur
0 siblings, 0 replies; 5+ messages in thread
From: SL Baur @ 2008-04-15 7:08 UTC (permalink / raw
To: Dan Williams; +Cc: gregkh, linux-kernel, kay.sievers, neilb, htejun, hpa, lkml
On 4/14/08, Dan Williams <dan.j.williams@intel.com> wrote:
> On Mon, Apr 14, 2008 at 8:13 PM, SL Baur <steve@xemacs.org> wrote:
> > On 4/14/08, Dan Williams <dan.j.williams@intel.com> 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
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2008-04-15 7:09 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-04-14 16:57 [PATCH] sysfs: add /sys/dev/{char, block} to lookup sysfs path by major:minor Dan Williams
2008-04-14 17:00 ` H. Peter Anvin
2008-04-15 3:13 ` SL Baur
2008-04-15 4:46 ` Dan Williams
2008-04-15 7:08 ` SL Baur
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).