linux-scsi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] add transport class symlink to device object
@ 2005-08-13 15:34 James.Smart
  2005-08-13 21:39 ` Greg KH
  0 siblings, 1 reply; 31+ messages in thread
From: James.Smart @ 2005-08-13 15:34 UTC (permalink / raw)
  To: linux-scsi

In prior conversations with folks doing utilities such as lsscsi, it became
rather apparent that it is difficult to establish the relationships between
the class device and its base device. The class->device symlink is good for
one direction, but the reverse direction didn't exist. In order to go from
the device to its class device, you needed to either search all class
devices to search for a device backlink that pointed to you, or had to build
in name relationships what the subset class devices that you need to look at.

This patch adds a symlink from the base device to the class device. The name
of the symlink is "class:<classname>".  I had originally considered just
<classname> for the symlink name, but it seemed to get lost in the attributes.

-- James S

Example:

# cd /sys/class/scsi_host/host0
# ls -ld device
lrwxrwxrwx  1 root root 0 Aug 13 11:11 device -> ../../../devices/platform/host0
# cd ../../../devices/platform/host0
# pwd
/sys/devices/platform/host0
# ls
class:scsi_host  power  target0:0:0  target0:0:1  target0:0:2  target0:0:3
# ls -ld class*
lrwxrwxrwx  1 root root 0 Aug 13 07:08 class:scsi_host -> ../../../class/scsi_host/host0




diff -upNr a/drivers/base/class.c b/drivers/base/class.c
--- a/drivers/base/class.c	2005-08-13 10:46:33.000000000 -0400
+++ b/drivers/base/class.c	2005-08-13 11:06:51.000000000 -0400
@@ -495,9 +495,13 @@ int class_device_add(struct class_device
 	}
 
 	class_device_add_attrs(class_dev);
-	if (class_dev->dev)
+	if (class_dev->dev) {
+		char buf[40];
 		sysfs_create_link(&class_dev->kobj,
 				  &class_dev->dev->kobj, "device");
+		snprintf(buf, 40, "class:%s", class_dev->class->name);
+		sysfs_create_link(&class_dev->dev->kobj, &class_dev->kobj, buf);
+	}
 
 	/* notify any interfaces this device is now here */
 	if (parent) {
@@ -589,8 +593,12 @@ void class_device_del(struct class_devic
 		up(&parent->sem);
 	}
 
-	if (class_dev->dev)
+	if (class_dev->dev) {
+		char buf[40];
+		snprintf(buf, 40, "class:%s", class_dev->class->name);
+		sysfs_remove_link(&class_dev->dev->kobj, buf);
 		sysfs_remove_link(&class_dev->kobj, "device");
+	}
 	if (class_dev->devt_attr) {
 		class_device_remove_file(class_dev, class_dev->devt_attr);
 		kfree(class_dev->devt_attr);

^ permalink raw reply	[flat|nested] 31+ messages in thread
* RE: [PATCH] add transport class symlink to device object
@ 2005-08-14 12:42 James.Smart
  2005-08-14 14:17 ` James Bottomley
  0 siblings, 1 reply; 31+ messages in thread
From: James.Smart @ 2005-08-14 12:42 UTC (permalink / raw)
  To: James.Bottomley, greg; +Cc: akpm, linux-scsi

I'll trust that James B's fix resolves things. Thought my testing was
straight-forward and ok. Guess not. Humbled again as a mere mortal in
the world of sysfs and transport/container logic... :)

Anyway, the last item that needs discussion is what name should the
symlink have ?

Here's a few points on why I chose a "class:<classname>" over 
"<classobjname>".

- Visually, I get more meaning out of seeing the class's name, than
  what is usually a redundant object name (many device and class
  objects have the same name).
- If a device could ever be associated with more than 1 class, it's
  supported (and no issues of name collision).
- The prefix of "class:" highlights what it is; makes it harder to lose
  in a long list of attributes; gives a simple handle for regex
  parsing (ex: "ls class*"); and helps avoid name collision with other
  attributes.
- The class object's name is always derivable from what the symlink
  points to.
- The class name is a good hint for utilities
(ok, last 2 are a little weak - you can parse the symlink for either of
 of them).

Preferences ?

-- james

^ permalink raw reply	[flat|nested] 31+ messages in thread
* RE: [PATCH] add transport class symlink to device object
@ 2005-08-16  0:52 James.Smart
  2005-08-16  1:08 ` James Bottomley
  2005-08-16 13:37 ` Luben Tuikov
  0 siblings, 2 replies; 31+ messages in thread
From: James.Smart @ 2005-08-16  0:52 UTC (permalink / raw)
  To: James.Bottomley, matthew; +Cc: greg, akpm, linux-scsi, linux-kernel, alan, rmk

Actually, I view this as being a little odd...

What is "0000:00:04:0" in this case ? The "device" is not a serial
port, which is what the ttyXX back link would lead you to believe.
Thus, it's a serial port multiplexer that supports up to N ports,
right ? and wouldn't the more correct representation have been to
enumerate a device for each serial port ? (e.g. 0000:00:04.0/line0,
0000:00:04.0/line1, or similar)

Think if SCSI used this same style of representation. For example,
if there was no scsi target device entity, but class entities did
exist and they just pointed back to the scsi host device entry.

My vote is to make the multiplexor instantiate each serial line
as a separate device.

-- james s

> On Sun, 2005-08-14 at 16:02 +0100, Matthew Wilcox wrote:
> > /sys/class/tty/ttyS0/device -> 
> ../../../devices/parisc/0/0:0/pci0000:00/0000:00:04.0
> > /sys/class/tty/ttyS1/device -> 
> ../../../devices/parisc/0/0:0/pci0000:00/0000:00:04.0
> > /sys/class/tty/ttyS2/device -> 
> ../../../devices/parisc/0/0:0/pci0000:00/0000:00:04.0
> > /sys/class/tty/ttyS3/device -> 
> ../../../devices/parisc/0/0:0/pci0000:00/0000:00:05.0
> > /sys/class/tty/ttyS4/device -> 
> ../../../devices/parisc/0/0:0/pci0000:00/0000:00:05.0
> 
> Actually, isn't the fix to all of this to combine Greg and James'
> patches?
> 
> The Greg one fails in SCSI because we don't have unique class device
> names (by convention we use the same name as the device bus_id) and
> James' one fails for ttys because the class name isn't 
> unique.  However,
> if the link were derived from something like
> 
> <class name>:<class device name>
> 
> Then is would be unique in both cases.
> 
> Unless anyone can think of any more failing cases?
> 
> James
> 
> 
> 

^ permalink raw reply	[flat|nested] 31+ messages in thread
* RE: [PATCH] add transport class symlink to device object
@ 2005-08-16 15:50 James.Smart
  0 siblings, 0 replies; 31+ messages in thread
From: James.Smart @ 2005-08-16 15:50 UTC (permalink / raw)
  To: James.Bottomley; +Cc: matthew, greg, akpm, linux-scsi, linux-kernel, alan, rmk

> On Mon, 2005-08-15 at 20:52 -0400, James.Smart@Emulex.Com wrote:
> > What is "0000:00:04:0" in this case ? The "device" is not a serial
> > port, which is what the ttyXX back link would lead you to believe.
> > Thus, it's a serial port multiplexer that supports up to N ports,
> > right ? and wouldn't the more correct representation have been to
> > enumerate a device for each serial port ? (e.g. 0000:00:04.0/line0,
> > 0000:00:04.0/line1, or similar)
> 
> It's PCI segment 0, bus 0, slot 4, function 0, which is apparently a 3
> port serial card (probably the GSP function of a pa8800?)

I guess you missed my point. The device (the PCI serial card) isn't the
member of the class, but rather an element of the device (one of the ports
on the pci card) is the class member. Thus, I believe the device backlink
for the class entity is misleading. The backlink implies the pci serial
card itself is the port. Ignoring this, the other headache is the device
backlink gives no clue as to what port or part of the pci adapter it
corresponds to.

In my conceptual thinking, there's a "wholeness" to the relationship of a
device and it's classes. E.g. trying to illustrate this with pci rather than
scsi - if there's a 3 slot pci bridge, with an adapter in each slot, I would
expect 4 different device entities: the bridge, and one for each adapter.
Each device would then be bound to whatever class makes sense for that
adapter. What I would not expect is a single device for the bridge and 3
class devices (one for each slot) that points back to the bridge device.

-- james s

^ permalink raw reply	[flat|nested] 31+ messages in thread
* RE: [PATCH] add transport class symlink to device object
@ 2005-08-18 11:31 James.Smart
  0 siblings, 0 replies; 31+ messages in thread
From: James.Smart @ 2005-08-18 11:31 UTC (permalink / raw)
  To: rmk+lkml, greg; +Cc: matthew, akpm, linux-scsi, linux-kernel, alan

> They are class devices called ttyS0, ttyS1, ttyS2 so you can say
> they're uniquely named.
> 
> The problem is that Matthew wants to add a symlink from the device
> device to the class device to complement the class device to device
> symlink, since we end up with multiple symlinks in the devices subdir
> all called the same.
> 
> This causes serial a problem because we have multiple class devices
> per device.

Missing some key words IMHO:

"This causes serial a problem because we have multiple class devices
(and each are of the *same* class) per device"

-- james s

^ permalink raw reply	[flat|nested] 31+ messages in thread
* RE: [PATCH] add transport class symlink to device object
@ 2005-08-18 11:32 James.Smart
  0 siblings, 0 replies; 31+ messages in thread
From: James.Smart @ 2005-08-18 11:32 UTC (permalink / raw)
  To: greg, James.Bottomley
  Cc: matthew, akpm, linux-scsi, linux-kernel, alan, rmk, dtor

I can live with this.  I would have liked the "class:" prefix, but the name
does start to get long, and this is ok.

-- james s

> -----Original Message-----
> From: Greg KH [mailto:greg@kroah.com]
> Sent: Thursday, August 18, 2005 2:37 AM
> To: James Bottomley
> Cc: Matthew Wilcox; Smart, James; Andrew Morton; SCSI Mailing List;
> Linux Kernel; Alan Cox; Russell King; Dmitry Torokhov
> Subject: Re: [PATCH] add transport class symlink to device object
> 
> 
> On Wed, Aug 17, 2005 at 10:23:11PM -0700, Greg KH wrote:
> > On Mon, Aug 15, 2005 at 05:41:17PM -0500, James Bottomley wrote:
> > > Actually, isn't the fix to all of this to combine Greg and James'
> > > patches?
> > 
> > Yes it is.
> > 
> > > The Greg one fails in SCSI because we don't have unique 
> class device
> > > names (by convention we use the same name as the device 
> bus_id) and
> > > James' one fails for ttys because the class name isn't 
> unique.  However,
> > > if the link were derived from something like
> > > 
> > > <class name>:<class device name>
> > > 
> > > Then is would be unique in both cases.
> > 
> > I agree.
> > 
> > > Unless anyone can think of any more failing cases?
> > 
> > I'll try this out and see if anything breaks :)
> 
> Ok, here's the patch.  I like it, and I think it will prevent any
> duplicate symlinks from happening.  Anyone have any objections?
> 
> thanks,
> 
> greg k-h
> 
> --------------
> 
> Driver core: link device and all class devices derived from it.
> 
> To ease the task of locating class devices derived from a certain
> device create symlinks from parent device to its class devices.
> Change USB host class device name from usbX to usb_hostX to avoid
> conflict when creating aforementioned links.
> 
> Tweaked by Greg to have the symlink be "class_name:class_device_name"
> in order to prevent duplicate links.
> 
> Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
> Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
> 
> ---
>  drivers/base/class.c   |   33 +++++++++++++++++++++++++++++++--
>  drivers/usb/core/hcd.c |    2 +-
>  2 files changed, 32 insertions(+), 3 deletions(-)
> 
> --- gregkh-2.6.orig/drivers/base/class.c	2005-08-17 
> 23:27:02.000000000 -0700
> +++ gregkh-2.6/drivers/base/class.c	2005-08-17 
> 23:27:25.000000000 -0700
> @@ -452,10 +452,29 @@ void class_device_initialize(struct clas
>  	INIT_LIST_HEAD(&class_dev->node);
>  }
>  
> +static char *make_class_name(struct class_device *class_dev)
> +{
> +	char *name;
> +	int size;
> +
> +	size = strlen(class_dev->class->name) +
> +		strlen(kobject_name(&class_dev->kobj)) + 2;
> +
> +	name = kmalloc(size, GFP_KERNEL);
> +	if (!name)
> +		return ERR_PTR(-ENOMEM);
> +
> +	strcpy(name, class_dev->class->name);
> +	strcat(name, ":");
> +	strcat(name, kobject_name(&class_dev->kobj));
> +	return name;
> +}
> +
>  int class_device_add(struct class_device *class_dev)
>  {
>  	struct class * parent = NULL;
>  	struct class_interface * class_intf;
> +	char *class_name = NULL;
>  	int error;
>  
>  	class_dev = class_device_get(class_dev);
> @@ -500,9 +519,13 @@ int class_device_add(struct class_device
>  	}
>  
>  	class_device_add_attrs(class_dev);
> -	if (class_dev->dev)
> +	if (class_dev->dev) {
> +		class_name = make_class_name(class_dev);
>  		sysfs_create_link(&class_dev->kobj,
>  				  &class_dev->dev->kobj, "device");
> +		sysfs_create_link(&class_dev->dev->kobj, 
> &class_dev->kobj,
> +				  class_name);
> +	}
>  
>  	/* notify any interfaces this device is now here */
>  	if (parent) {
> @@ -519,6 +542,7 @@ int class_device_add(struct class_device
>  	if (error && parent)
>  		class_put(parent);
>  	class_device_put(class_dev);
> +	kfree(class_name);
>  	return error;
>  }
>  
> @@ -584,6 +608,7 @@ void class_device_del(struct class_devic
>  {
>  	struct class * parent = class_dev->class;
>  	struct class_interface * class_intf;
> +	char *class_name = NULL;
>  
>  	if (parent) {
>  		down(&parent->sem);
> @@ -594,8 +619,11 @@ void class_device_del(struct class_devic
>  		up(&parent->sem);
>  	}
>  
> -	if (class_dev->dev)
> +	if (class_dev->dev) {
> +		class_name = make_class_name(class_dev);
>  		sysfs_remove_link(&class_dev->kobj, "device");
> +		sysfs_remove_link(&class_dev->dev->kobj, class_name);
> +	}
>  	if (class_dev->devt_attr)
>  		class_device_remove_file(class_dev, 
> class_dev->devt_attr);
>  	class_device_remove_attrs(class_dev);
> @@ -605,6 +633,7 @@ void class_device_del(struct class_devic
>  
>  	if (parent)
>  		class_put(parent);
> +	kfree(class_name);
>  }
>  
>  void class_device_unregister(struct class_device *class_dev)
> --- gregkh-2.6.orig/drivers/usb/core/hcd.c	2005-08-17 
> 23:26:55.000000000 -0700
> +++ gregkh-2.6/drivers/usb/core/hcd.c	2005-08-17 
> 23:27:04.000000000 -0700
> @@ -782,7 +782,7 @@ static int usb_register_bus(struct usb_b
>  		return -E2BIG;
>  	}
>  
> -	bus->class_dev = class_device_create(usb_host_class, 
> MKDEV(0,0), bus->controller, "usb%d", busnum);
> +	bus->class_dev = class_device_create(usb_host_class, 
> MKDEV(0,0), bus->controller, "usb_host%d", busnum);
>  	if (IS_ERR(bus->class_dev)) {
>  		clear_bit(busnum, busmap.busmap);
>  		up(&usb_bus_list_lock);
> 

^ permalink raw reply	[flat|nested] 31+ messages in thread
end of thread, other threads:[~2005-09-01  5:57 UTC | newest]

Thread overview: 31+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2005-08-13 15:34 [PATCH] add transport class symlink to device object James.Smart
2005-08-13 21:39 ` Greg KH
2005-08-13 23:36   ` James Bottomley
2005-08-14  0:42     ` James Bottomley
2005-08-14  1:37       ` James Bottomley
2005-08-14 15:02   ` Matthew Wilcox
2005-08-14 22:25     ` Russell King
2005-08-15  0:43       ` Matthew Wilcox
2005-08-15  8:32         ` Russell King
2005-08-18  5:21           ` Greg KH
2005-08-18  6:30             ` Russell King
2005-08-18  6:41               ` Greg KH
2005-08-18  6:50                 ` Russell King
2005-08-18  7:04                   ` Greg KH
2005-08-18 11:43                     ` Matthew Wilcox
2005-08-15 22:41     ` James Bottomley
2005-08-18  5:23       ` Greg KH
2005-08-18  6:37         ` Greg KH
2005-08-18 19:50           ` Dmitry Torokhov
2005-08-31 21:43             ` Greg KH
2005-09-01  5:57               ` Dmitry Torokhov
  -- strict thread matches above, loose matches on Subject: below --
2005-08-14 12:42 James.Smart
2005-08-14 14:17 ` James Bottomley
2005-08-16  0:52 James.Smart
2005-08-16  1:08 ` James Bottomley
2005-08-16 13:41   ` Luben Tuikov
2005-08-16 13:37 ` Luben Tuikov
2005-08-16 20:53   ` Russell King
2005-08-16 15:50 James.Smart
2005-08-18 11:31 James.Smart
2005-08-18 11:32 James.Smart

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 NNTP newsgroup(s).