| Message ID | 1358287662-24016-1-git-send-email-dmilburn@redhat.com |
|---|---|
| State | Not Applicable |
| Delegated to: | David Miller |
| Headers | show |
On Tue, Jan 15, 2013 at 11:07 PM, David Milburn <dmilburn@redhat.com> wrote: > As low-level drivers register their host controller(s), keep track > of the number of controllers and export thru /sys in a <host.port> > format so that udev can better match up port numbers with a > specific controller. Hmm, I tried this and I don't think it will solve out problem. But I might have missed something ... This is host 2 with port "7", but it is really port 1: > ./0000:00:1e.0/0000:05:01.0/ata2.7 > ./0000:00:1e.0/0000:05:01.0/ata2.7/link7/dev7.0/ata_device This is host 1 with port "6": > ./0000:00:1f.2/ata1.6/ata_port > ./0000:00:1f.2/ata1.6/ata_port/ata1.6 The host number is meaningless regarding stable identifiers, as it depends on probing/driver binding order. Now, the port number is still *global* across all all controllers, and therefore also not useful. We need *host-local* numbers, which share nothing with other hosts, so the drivers and bus enumeration order can change at any time without affecting any of the device numbers used inside the host. I have not much clue about port-multipliers, never seen them, but if we want to support arbitrary stacking of them we will need to compose a "chain of numbers" in the "stable identifier" out of the individual port numbers they are connected to, very much like USB hub chaining works. This means we will need every instance in the chain to start with their own numbers again. Any global counters used for device naming/enumeration are not useful to identify devices from userspace. Thanks, Kay -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Kay Sievers wrote: > On Tue, Jan 15, 2013 at 11:07 PM, David Milburn <dmilburn@redhat.com> wrote: >> As low-level drivers register their host controller(s), keep track >> of the number of controllers and export thru /sys in a <host.port> >> format so that udev can better match up port numbers with a >> specific controller. > > Hmm, I tried this and I don't think it will solve out problem. But I > might have missed something ... > > This is host 2 with port "7", but it is really port 1: > >> ./0000:00:1e.0/0000:05:01.0/ata2.7 >> ./0000:00:1e.0/0000:05:01.0/ata2.7/link7/dev7.0/ata_device > > This is host 1 with port "6": > >> ./0000:00:1f.2/ata1.6/ata_port >> ./0000:00:1f.2/ata1.6/ata_port/ata1.6 > > The host number is meaningless regarding stable identifiers, as it > depends on probing/driver binding order. > > Now, the port number is still *global* across all all controllers, and > therefore also not useful. We need *host-local* numbers, which share > nothing with other hosts, so the drivers and bus enumeration order can > change at any time without affecting any of the device numbers used > inside the host. > > I have not much clue about port-multipliers, never seen them, but if > we want to support arbitrary stacking of them we will need to compose > a "chain of numbers" in the "stable identifier" out of the individual > port numbers they are connected to, very much like USB hub chaining > works. This means we will need every instance in the chain to start > with their own numbers again. > > Any global counters used for device naming/enumeration are not useful > to identify devices from userspace. Hi Kay, So, if we eliminated the global ata_print_id counter, then we would need to check all the places ap->print_id is used and consider the host controller and a local_print_id. So, if the above changed to 2.1, we would expect ata2.1 to show up in dmesg during error recovery instead of ata7, right? There are other places (non-printk stuff) in libata that check the global counter that would need to change, and libsas increments the global counter when a SATA drive is present on a SAS controller. Thanks, David > > Thanks, > Kay -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jan 16, 2013 at 10:25 PM, David Milburn <dmilburn@redhat.com> wrote: > So, if we eliminated the global ata_print_id counter, then we would > need to check all the places ap->print_id is used and consider the host > controller and a local_print_id. So, if the above changed to 2.1, we > would expect ata2.1 to show up in dmesg during error recovery > instead of ata7, right? > > There are other places (non-printk stuff) in libata that check the global > counter that would need to change, and libsas increments the global > counter when a SATA drive is present on a SAS controller. Well, we just need a way to get some "stable" numbers out of /sys when we start at the block device and walk up the chain of parent devices and see the ata directory. It does not necessarily need to be the name of the top ata%d directory, it could be a sysfs attribute somewhere too, if that's easier. If we we are going to support port by-path/ strings for block devices connected through a port multiplier chain, I guess we will need an arbitrary length string representing the concatenated chain of local port numbers, containing stable port identifier numbers of every device involved. None of the numbers it contains can therefore depend on enumeration or loading order. The "stable chain of port numbers" do not necessarily need to share anything with the current global numbers to make the device name unique. It would be easier to understand, but it's not a requirement. Kay Kay -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c index 46cd3f4..275941b 100644 --- a/drivers/ata/libata-core.c +++ b/drivers/ata/libata-core.c @@ -99,6 +99,7 @@ static void ata_dev_xfermask(struct ata_device *dev); static unsigned long ata_dev_blacklisted(const struct ata_device *dev); atomic_t ata_print_id = ATOMIC_INIT(0); +atomic_t host_print_id = ATOMIC_INIT(0); struct ata_force_param { const char *name; @@ -6097,6 +6098,9 @@ int ata_host_register(struct ata_host *host, struct scsi_host_template *sht) for (i = host->n_ports; host->ports[i]; i++) kfree(host->ports[i]); + /* track host controller */ + host->host_id = atomic_inc_return(&host_print_id); + /* give ports names and add SCSI hosts */ for (i = 0; i < host->n_ports; i++) host->ports[i]->print_id = atomic_inc_return(&ata_print_id); diff --git a/drivers/ata/libata-transport.c b/drivers/ata/libata-transport.c index c04d393..61dca7a 100644 --- a/drivers/ata/libata-transport.c +++ b/drivers/ata/libata-transport.c @@ -284,7 +284,7 @@ int ata_tport_add(struct device *parent, dev->parent = get_device(parent); dev->release = ata_tport_release; - dev_set_name(dev, "ata%d", ap->print_id); + dev_set_name(dev, "ata%d.%d", ap->host->host_id, ap->print_id); transport_setup_device(dev); error = device_add(dev); if (error) { diff --git a/drivers/ata/libata.h b/drivers/ata/libata.h index 7148a58..97776fd 100644 --- a/drivers/ata/libata.h +++ b/drivers/ata/libata.h @@ -54,6 +54,7 @@ enum { }; extern atomic_t ata_print_id; +extern atomic_t host_print_id; extern int atapi_passthru16; extern int libata_fua; extern int libata_noacpi; diff --git a/include/linux/libata.h b/include/linux/libata.h index 649e5f8..7ae207e 100644 --- a/include/linux/libata.h +++ b/include/linux/libata.h @@ -546,6 +546,7 @@ struct ata_host { void *private_data; struct ata_port_operations *ops; unsigned long flags; + unsigned int host_id; /* user visible host ID */ struct mutex eh_mutex; struct task_struct *eh_owner;
As low-level drivers register their host controller(s), keep track of the number of controllers and export thru /sys in a <host.port> format so that udev can better match up port numbers with a specific controller. # pwd /sys/devices/pci0000:00 # find . -name 'ata*' -print (2nd controller with port multiplier attached) ./0000:00:1e.0/0000:05:01.0/ata2.7 ./0000:00:1e.0/0000:05:01.0/ata2.7/link7/dev7.0/ata_device ./0000:00:1e.0/0000:05:01.0/ata2.7/link7/ata_link ./0000:00:1e.0/0000:05:01.0/ata2.7/link7.0/dev7.0.0/ata_device ./0000:00:1e.0/0000:05:01.0/ata2.7/link7.0/ata_link ./0000:00:1e.0/0000:05:01.0/ata2.7/link7.1/dev7.1.0/ata_device ./0000:00:1e.0/0000:05:01.0/ata2.7/link7.1/ata_link ./0000:00:1e.0/0000:05:01.0/ata2.7/link7.2/dev7.2.0/ata_device ./0000:00:1e.0/0000:05:01.0/ata2.7/link7.2/ata_link ./0000:00:1e.0/0000:05:01.0/ata2.7/link7.3/dev7.3.0/ata_device ./0000:00:1e.0/0000:05:01.0/ata2.7/link7.3/ata_link ./0000:00:1e.0/0000:05:01.0/ata2.7/link7.4/dev7.4.0/ata_device ./0000:00:1e.0/0000:05:01.0/ata2.7/link7.4/ata_link ./0000:00:1e.0/0000:05:01.0/ata2.7/link7.5/dev7.5.0/ata_device ./0000:00:1e.0/0000:05:01.0/ata2.7/link7.5/ata_link ./0000:00:1e.0/0000:05:01.0/ata2.7/link7.6/dev7.6.0/ata_device ./0000:00:1e.0/0000:05:01.0/ata2.7/link7.6/ata_link ./0000:00:1e.0/0000:05:01.0/ata2.7/link7.7/dev7.7.0/ata_device ./0000:00:1e.0/0000:05:01.0/ata2.7/link7.7/ata_link ./0000:00:1e.0/0000:05:01.0/ata2.7/link7.8/dev7.8.0/ata_device ./0000:00:1e.0/0000:05:01.0/ata2.7/link7.8/ata_link ./0000:00:1e.0/0000:05:01.0/ata2.7/link7.9/dev7.9.0/ata_device ./0000:00:1e.0/0000:05:01.0/ata2.7/link7.9/ata_link ./0000:00:1e.0/0000:05:01.0/ata2.7/ata_port ./0000:00:1e.0/0000:05:01.0/ata2.7/ata_port/ata2.7 ./0000:00:1e.0/0000:05:01.0/ata2.7/link7.10/dev7.10.0/ata_device ./0000:00:1e.0/0000:05:01.0/ata2.7/link7.10/ata_link ./0000:00:1e.0/0000:05:01.0/ata2.7/link7.11/dev7.11.0/ata_device ./0000:00:1e.0/0000:05:01.0/ata2.7/link7.11/ata_link ./0000:00:1e.0/0000:05:01.0/ata2.7/link7.12/dev7.12.0/ata_device ./0000:00:1e.0/0000:05:01.0/ata2.7/link7.12/ata_link ./0000:00:1e.0/0000:05:01.0/ata2.7/link7.13/dev7.13.0/ata_device ./0000:00:1e.0/0000:05:01.0/ata2.7/link7.13/ata_link ./0000:00:1e.0/0000:05:01.0/ata2.7/link7.14/dev7.14.0/ata_device ./0000:00:1e.0/0000:05:01.0/ata2.7/link7.14/ata_link ./0000:00:1e.0/0000:05:01.0/ata2.8 ./0000:00:1e.0/0000:05:01.0/ata2.8/link8/dev8.0/ata_device ./0000:00:1e.0/0000:05:01.0/ata2.8/link8/ata_link ./0000:00:1e.0/0000:05:01.0/ata2.8/ata_port ./0000:00:1e.0/0000:05:01.0/ata2.8/ata_port/ata2.8 (1st controller) ./0000:00:1f.2/ata1.1 ./0000:00:1f.2/ata1.1/link1/dev1.0/ata_device ./0000:00:1f.2/ata1.1/link1/ata_link ./0000:00:1f.2/ata1.1/ata_port ./0000:00:1f.2/ata1.1/ata_port/ata1.1 ./0000:00:1f.2/ata1.2 ./0000:00:1f.2/ata1.2/link2/dev2.0/ata_device ./0000:00:1f.2/ata1.2/link2/ata_link ./0000:00:1f.2/ata1.2/ata_port ./0000:00:1f.2/ata1.2/ata_port/ata1.2 ./0000:00:1f.2/ata1.3 ./0000:00:1f.2/ata1.3/link3/dev3.0/ata_device ./0000:00:1f.2/ata1.3/link3/ata_link ./0000:00:1f.2/ata1.3/ata_port ./0000:00:1f.2/ata1.3/ata_port/ata1.3 ./0000:00:1f.2/ata1.4 ./0000:00:1f.2/ata1.4/link4/dev4.0/ata_device ./0000:00:1f.2/ata1.4/link4/ata_link ./0000:00:1f.2/ata1.4/ata_port ./0000:00:1f.2/ata1.4/ata_port/ata1.4 ./0000:00:1f.2/ata1.5 ./0000:00:1f.2/ata1.5/link5/dev5.0/ata_device ./0000:00:1f.2/ata1.5/link5/ata_link ./0000:00:1f.2/ata1.5/ata_port ./0000:00:1f.2/ata1.5/ata_port/ata1.5 ./0000:00:1f.2/ata1.6 ./0000:00:1f.2/ata1.6/link6/dev6.0/ata_device ./0000:00:1f.2/ata1.6/link6/ata_link ./0000:00:1f.2/ata1.6/ata_port ./0000:00:1f.2/ata1.6/ata_port/ata1.6 Signed-off-by: David Milburn <dmilburn@redhat.com> --- changes from v1->v2: Fengguang Wu reported a sparse warning for host_print_id, declared as extern in drivers/ata/libata.h drivers/ata/libata-core.c:102:10: sparse: symbol 'host_print_id' was not declared. Should it be static? drivers/ata/libata-core.c | 4 ++++ drivers/ata/libata-transport.c | 2 +- drivers/ata/libata.h | 1 + include/linux/libata.h | 1 + 4 files changed, 7 insertions(+), 1 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html