* [PATCH] ses: Handle non-unique element descriptors
@ 2011-07-04 15:16 Ankit Jain
2011-07-04 15:35 ` James Bottomley
0 siblings, 1 reply; 5+ messages in thread
From: Ankit Jain @ 2011-07-04 15:16 UTC (permalink / raw)
To: linux-scsi; +Cc: James Bottomley, Hannes Reinecke
Some SES devices give non-unique Element Descriptors as part of the
Element Descriptor diag page. Since we use these for creating sysfs
entries, they need to be unique.
Eg:
$ sg_ses -p 7 /dev/sg0
FTS CORP TXS6_SAS20BPX12 0500
enclosure services device
Element descriptor In diagnostic page:
generation code: 0x0
element descriptor by type list
Element type: Array device, subenclosure id: 0
Overall descriptor: ArrayDevicesInSubEnclsr0
Element 1 descriptor: ArrayDevice00
Element 2 descriptor: ArrayDevice01
Element 3 descriptor: ArrayDevice02
Element 4 descriptor: ArrayDevice03
Element 5 descriptor: ArrayDevice03
Element 6 descriptor: ArrayDevice03
Element 7 descriptor: ArrayDevice03
Element 8 descriptor: ArrayDevice03
Element 9 descriptor: ArrayDevice03
Element 10 descriptor: ArrayDevice03
Element 11 descriptor: ArrayDevice03
Element 12 descriptor: ArrayDevice03
Based on scsi-misc
Signed-off-by: Ankit Jain <jankit@suse.de>
---
diff --git a/drivers/misc/enclosure.c b/drivers/misc/enclosure.c
index 00e5fca..8087055 100644
--- a/drivers/misc/enclosure.c
+++ b/drivers/misc/enclosure.c
@@ -239,6 +239,24 @@ static void enclosure_component_release(struct device *dev)
put_device(dev->parent);
}
+struct enclosure_component *
+enclosure_component_find_by_name(struct enclosure_device *edev,
+ const char *name)
+{
+ int i;
+ if (!edev || !name || !name[0])
+ return NULL;
+
+ for (i=0; i<edev->components; i++) {
+ struct enclosure_component *ecomp = &edev->component[i];
+ if (ecomp->number != -1 && !strcmp(dev_name(&ecomp->cdev), name))
+ return ecomp;
+ }
+
+ return NULL;
+}
+EXPORT_SYMBOL_GPL(enclosure_component_find_by_name);
+
static const struct attribute_group *enclosure_groups[];
/**
diff --git a/drivers/scsi/ses.c b/drivers/scsi/ses.c
index eba183c..abfd962 100644
--- a/drivers/scsi/ses.c
+++ b/drivers/scsi/ses.c
@@ -421,12 +421,17 @@ static void ses_enclosure_data_process(struct enclosure_device *edev,
if (type_ptr[0] == ENCLOSURE_COMPONENT_DEVICE ||
type_ptr[0] == ENCLOSURE_COMPONENT_ARRAY_DEVICE) {
- if (create)
+ if (create) {
+ if (enclosure_component_find_by_name(edev, name))
+ /* name is not unique, already used
+ * set to NULL, so that enclosure_component_register
+ * will assign us a new one */
+ name = NULL;
ecomp = enclosure_component_register(edev,
components++,
type_ptr[0],
name);
- else
+ } else
ecomp = &edev->component[components++];
if (!IS_ERR(ecomp) && addl_desc_ptr)
diff --git a/include/linux/enclosure.h b/include/linux/enclosure.h
index 9a33c5f..7e48ba9 100644
--- a/include/linux/enclosure.h
+++ b/include/linux/enclosure.h
@@ -122,6 +122,8 @@ void enclosure_unregister(struct enclosure_device *);
struct enclosure_component *
enclosure_component_register(struct enclosure_device *, unsigned int,
enum enclosure_component_type, const char *);
+struct enclosure_component *
+enclosure_component_find_by_name(struct enclosure_device *, const char *);
int enclosure_add_device(struct enclosure_device *enclosure, int component,
struct device *dev);
int enclosure_remove_device(struct enclosure_device *, struct device *);
--
Ankit Jain
SUSE Labs
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] ses: Handle non-unique element descriptors 2011-07-04 15:16 [PATCH] ses: Handle non-unique element descriptors Ankit Jain @ 2011-07-04 15:35 ` James Bottomley 2011-07-04 22:59 ` Ankit Jain 0 siblings, 1 reply; 5+ messages in thread From: James Bottomley @ 2011-07-04 15:35 UTC (permalink / raw) To: Ankit Jain; +Cc: linux-scsi, James Bottomley, Hannes Reinecke On Mon, 2011-07-04 at 20:46 +0530, Ankit Jain wrote: > Some SES devices give non-unique Element Descriptors as part of the > Element Descriptor diag page. Since we use these for creating sysfs > entries, they need to be unique. > > Eg: > $ sg_ses -p 7 /dev/sg0 > FTS CORP TXS6_SAS20BPX12 0500 > enclosure services device > Element descriptor In diagnostic page: > generation code: 0x0 > element descriptor by type list > Element type: Array device, subenclosure id: 0 > Overall descriptor: ArrayDevicesInSubEnclsr0 > Element 1 descriptor: ArrayDevice00 > Element 2 descriptor: ArrayDevice01 > Element 3 descriptor: ArrayDevice02 > Element 4 descriptor: ArrayDevice03 > Element 5 descriptor: ArrayDevice03 > Element 6 descriptor: ArrayDevice03 > Element 7 descriptor: ArrayDevice03 > Element 8 descriptor: ArrayDevice03 > Element 9 descriptor: ArrayDevice03 > Element 10 descriptor: ArrayDevice03 > Element 11 descriptor: ArrayDevice03 > Element 12 descriptor: ArrayDevice03 What is the external visible labelling of this topology? It's completely weird that the enclosure would burn in non-unique names unless there's some reason for it. > Signed-off-by: Ankit Jain <jankit@suse.de> > > --- > diff --git a/drivers/misc/enclosure.c b/drivers/misc/enclosure.c > index 00e5fca..8087055 100644 > --- a/drivers/misc/enclosure.c > +++ b/drivers/misc/enclosure.c > @@ -239,6 +239,24 @@ static void enclosure_component_release(struct device *dev) > put_device(dev->parent); > } > > +struct enclosure_component * > +enclosure_component_find_by_name(struct enclosure_device *edev, > + const char *name) > +{ > + int i; > + if (!edev || !name || !name[0]) > + return NULL; > + > + for (i=0; i<edev->components; i++) { > + struct enclosure_component *ecomp = &edev->component[i]; > + if (ecomp->number != -1 && !strcmp(dev_name(&ecomp->cdev), name)) > + return ecomp; > + } > + > + return NULL; > +} > +EXPORT_SYMBOL_GPL(enclosure_component_find_by_name); > + > static const struct attribute_group *enclosure_groups[]; > > /** > diff --git a/drivers/scsi/ses.c b/drivers/scsi/ses.c > index eba183c..abfd962 100644 > --- a/drivers/scsi/ses.c > +++ b/drivers/scsi/ses.c > @@ -421,12 +421,17 @@ static void ses_enclosure_data_process(struct enclosure_device *edev, > if (type_ptr[0] == ENCLOSURE_COMPONENT_DEVICE || > type_ptr[0] == ENCLOSURE_COMPONENT_ARRAY_DEVICE) { > > - if (create) > + if (create) { > + if (enclosure_component_find_by_name(edev, name)) > + /* name is not unique, already used > + * set to NULL, so that enclosure_component_register > + * will assign us a new one */ > + name = NULL; This just assigns a random name ... if we actually have one, we should probably just make it unique. James > ecomp = enclosure_component_register(edev, > components++, > type_ptr[0], > name); > - else > + } else > ecomp = &edev->component[components++]; > > if (!IS_ERR(ecomp) && addl_desc_ptr) > diff --git a/include/linux/enclosure.h b/include/linux/enclosure.h > index 9a33c5f..7e48ba9 100644 > --- a/include/linux/enclosure.h > +++ b/include/linux/enclosure.h > @@ -122,6 +122,8 @@ void enclosure_unregister(struct enclosure_device *); > struct enclosure_component * > enclosure_component_register(struct enclosure_device *, unsigned int, > enum enclosure_component_type, const char *); > +struct enclosure_component * > +enclosure_component_find_by_name(struct enclosure_device *, const char *); > int enclosure_add_device(struct enclosure_device *enclosure, int component, > struct device *dev); > int enclosure_remove_device(struct enclosure_device *, struct device *); ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] ses: Handle non-unique element descriptors 2011-07-04 15:35 ` James Bottomley @ 2011-07-04 22:59 ` Ankit Jain 2011-07-04 23:00 ` James Bottomley 0 siblings, 1 reply; 5+ messages in thread From: Ankit Jain @ 2011-07-04 22:59 UTC (permalink / raw) To: James Bottomley; +Cc: linux-scsi, James Bottomley, Hannes Reinecke On 07/04/2011 09:05 PM, James Bottomley wrote: > On Mon, 2011-07-04 at 20:46 +0530, Ankit Jain wrote: >> Some SES devices give non-unique Element Descriptors as part of the >> Element Descriptor diag page. Since we use these for creating sysfs >> entries, they need to be unique. >> >> Eg: >> $ sg_ses -p 7 /dev/sg0 >> FTS CORP TXS6_SAS20BPX12 0500 >> enclosure services device >> Element descriptor In diagnostic page: >> generation code: 0x0 >> element descriptor by type list >> Element type: Array device, subenclosure id: 0 >> Overall descriptor: ArrayDevicesInSubEnclsr0 >> Element 1 descriptor: ArrayDevice00 >> Element 2 descriptor: ArrayDevice01 >> Element 3 descriptor: ArrayDevice02 >> Element 4 descriptor: ArrayDevice03 >> Element 5 descriptor: ArrayDevice03 >> Element 6 descriptor: ArrayDevice03 >> Element 7 descriptor: ArrayDevice03 >> Element 8 descriptor: ArrayDevice03 >> Element 9 descriptor: ArrayDevice03 >> Element 10 descriptor: ArrayDevice03 >> Element 11 descriptor: ArrayDevice03 >> Element 12 descriptor: ArrayDevice03 > > What is the external visible labelling of this topology? It's > completely weird that the enclosure would burn in non-unique names > unless there's some reason for it. I'm not sure what you mean by "external visible labelling". The system has a SAS expanded backplane. I don't have access to the hardware now, but p7 looked like this: $ sg_ses -p 7 /dev/sg0 FTS CORP TXS6_SAS20BPX12 0500 enclosure services device Element descriptor In diagnostic page: generation code: 0x0 element descriptor by type list Element type: Array device, subenclosure id: 0 Overall descriptor: ArrayDevicesInSubEnclsr0 Element 1 descriptor: ArrayDevice00 Element 2 descriptor: ArrayDevice01 Element 3 descriptor: ArrayDevice02 Element 4 descriptor: ArrayDevice03 Element 5 descriptor: ArrayDevice03 Element 6 descriptor: ArrayDevice03 Element 7 descriptor: ArrayDevice03 Element 8 descriptor: ArrayDevice03 Element 9 descriptor: ArrayDevice03 Element 10 descriptor: ArrayDevice03 Element 11 descriptor: ArrayDevice03 Element 12 descriptor: ArrayDevice03 Element type: SAS connector, subenclosure id: 0 Overall descriptor: ConnectorsInSubEnclsr0 Element 1 descriptor: Connector04 Element 2 descriptor: Connector05 Element 3 descriptor: Connector06 Element 4 descriptor: Connector07 Element 5 descriptor: Connector00 Element 6 descriptor: Connector01 Element 7 descriptor: Connector02 Element 8 descriptor: Connector03 Element 9 descriptor: Connector08 Element 10 descriptor: Connector09 Element 11 descriptor: Connector10 Element 12 descriptor: Connector11 Element type: Temperature sense, subenclosure id: 0 Overall descriptor: TempSensorsInSubEnclsr0 Element 1 descriptor: TempSense01 Element 2 descriptor: TempSense02 Element 3 descriptor: TempSense03 Element type: Voltage sensor, subenclosure id: 0 Overall descriptor: VoltageSensorsInSubEnclsr0 Element 1 descriptor: VoltageSense01 Element 2 descriptor: VoltageSense02 Element 3 descriptor: VoltageSense03 Element 4 descriptor: VoltageSense04 Element type: Current sensor, subenclosure id: 0 Overall descriptor: CurrentSensorsInSubEnclsr0 Element 1 descriptor: CurrentSense01 Element type: Enclosure, subenclosure id: 0 Overall descriptor: EnclosureElementInSubEnclsr0 Element 1 descriptor: EnclosureElement01 Element type: SAS expander, subenclosure id: 0 Overall descriptor: SAS Expander Element 1 descriptor: Expander0 AFAICS, the spec doesn't seem to say that the names have to be unique. But, now that I think about it a bit, possibly some of the "array device slots" might have been vacant, and so descriptors could have been repeated. This could probably be checked with the status page. But then I'm not sure, whether sysfs entries should or shouldn't be created for some cases (eg. status code!=OK). >> - if (create) >> + if (create) { >> + if (enclosure_component_find_by_name(edev, name)) >> + /* name is not unique, already used >> + * set to NULL, so that enclosure_component_register >> + * will assign us a new one */ >> + name = NULL; > > This just assigns a random name ... if we actually have one, we should > probably just make it unique. Agreed. Would something like (below) be acceptable? I'll post the patch separately, if it is: --- diff --git a/drivers/misc/enclosure.c b/drivers/misc/enclosure.c index 00e5fca..f4b53fd 100644 --- a/drivers/misc/enclosure.c +++ b/drivers/misc/enclosure.c @@ -239,6 +239,23 @@ static void enclosure_component_release(struct device *dev) put_device(dev->parent); } +static struct enclosure_component * +enclosure_component_find_by_name(struct enclosure_device *edev, + const char *name) +{ + int i; + if (!edev || !name || !name[0]) + return NULL; + + for (i=0; i<edev->components; i++) { + struct enclosure_component *ecomp = &edev->component[i]; + if (ecomp->number != -1 && !strcmp(dev_name(&ecomp->cdev), name)) + return ecomp; + } + + return NULL; +} + static const struct attribute_group *enclosure_groups[]; /** @@ -276,9 +293,13 @@ enclosure_component_register(struct enclosure_device *edev, ecomp->number = number; cdev = &ecomp->cdev; cdev->parent = get_device(&edev->edev); - if (name && name[0]) - dev_set_name(cdev, "%s", name); - else + + if (name && name[0]) { + if (enclosure_component_find_by_name (edev, name)) + dev_set_name(cdev, "%s_%u", name, number); + else + dev_set_name(cdev, "%s", name); + } else dev_set_name(cdev, "%u", number); cdev->release = enclosure_component_release; -- Ankit Jain SUSE Labs ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] ses: Handle non-unique element descriptors 2011-07-04 22:59 ` Ankit Jain @ 2011-07-04 23:00 ` James Bottomley 2011-07-11 10:24 ` Ankit Jain 0 siblings, 1 reply; 5+ messages in thread From: James Bottomley @ 2011-07-04 23:00 UTC (permalink / raw) To: Ankit Jain; +Cc: linux-scsi, Hannes Reinecke On Tue, 2011-07-05 at 04:29 +0530, Ankit Jain wrote: > On 07/04/2011 09:05 PM, James Bottomley wrote: > > On Mon, 2011-07-04 at 20:46 +0530, Ankit Jain wrote: > >> Some SES devices give non-unique Element Descriptors as part of the > >> Element Descriptor diag page. Since we use these for creating sysfs > >> entries, they need to be unique. > >> > >> Eg: > >> $ sg_ses -p 7 /dev/sg0 > >> FTS CORP TXS6_SAS20BPX12 0500 > >> enclosure services device > >> Element descriptor In diagnostic page: > >> generation code: 0x0 > >> element descriptor by type list > >> Element type: Array device, subenclosure id: 0 > >> Overall descriptor: ArrayDevicesInSubEnclsr0 > >> Element 1 descriptor: ArrayDevice00 > >> Element 2 descriptor: ArrayDevice01 > >> Element 3 descriptor: ArrayDevice02 > >> Element 4 descriptor: ArrayDevice03 > >> Element 5 descriptor: ArrayDevice03 > >> Element 6 descriptor: ArrayDevice03 > >> Element 7 descriptor: ArrayDevice03 > >> Element 8 descriptor: ArrayDevice03 > >> Element 9 descriptor: ArrayDevice03 > >> Element 10 descriptor: ArrayDevice03 > >> Element 11 descriptor: ArrayDevice03 > >> Element 12 descriptor: ArrayDevice03 > > > > What is the external visible labelling of this topology? It's > > completely weird that the enclosure would burn in non-unique names > > unless there's some reason for it. > > I'm not sure what you mean by "external visible labelling". The system has a > SAS expanded backplane. I don't have access to the hardware now, but p7 looked > like this: The element descriptors usually correspond with labelling on the case of whatever it is the enclosure is embedded in. Their job is to describe the location of the device, so they usually get burned in with whatever the default labelling scheme for the physical device is. So what I want to know is what is the labelling scheme in this case? James ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] ses: Handle non-unique element descriptors 2011-07-04 23:00 ` James Bottomley @ 2011-07-11 10:24 ` Ankit Jain 0 siblings, 0 replies; 5+ messages in thread From: Ankit Jain @ 2011-07-11 10:24 UTC (permalink / raw) To: James Bottomley; +Cc: linux-scsi, Hannes Reinecke On 07/05/2011 04:30 AM, James Bottomley wrote: > On Tue, 2011-07-05 at 04:29 +0530, Ankit Jain wrote: >> On 07/04/2011 09:05 PM, James Bottomley wrote: >>> On Mon, 2011-07-04 at 20:46 +0530, Ankit Jain wrote: >>>> Some SES devices give non-unique Element Descriptors as part of the >>>> Element Descriptor diag page. Since we use these for creating sysfs >>>> entries, they need to be unique. >>>> >>>> Eg: >>>> $ sg_ses -p 7 /dev/sg0 >>>> FTS CORP TXS6_SAS20BPX12 0500 >>>> enclosure services device >>>> Element descriptor In diagnostic page: >>>> generation code: 0x0 >>>> element descriptor by type list >>>> Element type: Array device, subenclosure id: 0 >>>> Overall descriptor: ArrayDevicesInSubEnclsr0 >>>> Element 1 descriptor: ArrayDevice00 >>>> Element 2 descriptor: ArrayDevice01 >>>> Element 3 descriptor: ArrayDevice02 >>>> Element 4 descriptor: ArrayDevice03 >>>> Element 5 descriptor: ArrayDevice03 >>>> Element 6 descriptor: ArrayDevice03 >>>> Element 7 descriptor: ArrayDevice03 >>>> Element 8 descriptor: ArrayDevice03 >>>> Element 9 descriptor: ArrayDevice03 >>>> Element 10 descriptor: ArrayDevice03 >>>> Element 11 descriptor: ArrayDevice03 >>>> Element 12 descriptor: ArrayDevice03 >>> >>> What is the external visible labelling of this topology? It's >>> completely weird that the enclosure would burn in non-unique names >>> unless there's some reason for it. >> >> I'm not sure what you mean by "external visible labelling". The system has a >> SAS expanded backplane. I don't have access to the hardware now, but p7 looked >> like this: > > The element descriptors usually correspond with labelling on the case of > whatever it is the enclosure is embedded in. Their job is to describe > the location of the device, so they usually get burned in with whatever > the default labelling scheme for the physical device is. So what I want > to know is what is the labelling scheme in this case? I don't have direct access to the box, so I asked the person who does, and he says: "the labelling is just 1-12 unless I'm mistaken.". The duplicate descriptors is a firmware bug, but since the specification doesn't say that these have to be unique, we should handle these, IMHO. -- Ankit Jain SUSE Labs ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2011-07-11 10:22 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-07-04 15:16 [PATCH] ses: Handle non-unique element descriptors Ankit Jain 2011-07-04 15:35 ` James Bottomley 2011-07-04 22:59 ` Ankit Jain 2011-07-04 23:00 ` James Bottomley 2011-07-11 10:24 ` Ankit Jain
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox