Re: [PATCH v2] driver core: Fix device link device name collision

2021-01-07 Thread Greg Kroah-Hartman
On Fri, Jan 08, 2021 at 02:10:51AM +0100, Michael Walle wrote:
> Am 8. Januar 2021 02:00:32 MEZ schrieb Saravana Kannan :
> >On Thu, Jan 7, 2021 at 4:43 PM Saravana Kannan 
> >wrote:
> >Nevermind, I see it now. Also, in the future, if you can dump the logs
> >in some kind of pastebin site, that'd be nice. Avoid the emails
> >becoming unwieldy and also avoids the log lines from wrapping.
> 
> I thought about a pastebin. But then decided against it because they might be 
> deleted in the future. So if anyone looking at the mail archives he might 
> only get dead links. So.. 
> 
> dunno how this is handled on the LKML, tbh. 

Full logs are good, you did it right, Saravana can deal with long emails :)


Re: [PATCH v2] driver core: Fix device link device name collision

2021-01-07 Thread Michael Walle
Am 8. Januar 2021 02:00:32 MEZ schrieb Saravana Kannan :
>On Thu, Jan 7, 2021 at 4:43 PM Saravana Kannan 
>wrote:
>Nevermind, I see it now. Also, in the future, if you can dump the logs
>in some kind of pastebin site, that'd be nice. Avoid the emails
>becoming unwieldy and also avoids the log lines from wrapping.

I thought about a pastebin. But then decided against it because they might be 
deleted in the future. So if anyone looking at the mail archives he might only 
get dead links. So.. 

dunno how this is handled on the LKML, tbh. 

-michael 



Re: [PATCH v2] driver core: Fix device link device name collision

2021-01-07 Thread Saravana Kannan
On Thu, Jan 7, 2021 at 4:43 PM Saravana Kannan  wrote:
>
> On Thu, Jan 7, 2021 at 4:14 PM Michael Walle  wrote:
> >
> > Am 2021-01-08 00:41, schrieb Saravana Kannan:
> > > The device link device's name was of the form:
> > > --
> > >
> > > This can cause name collision as reported here [1] as device names are
> > > not globally unique. Since device names have to be unique within the
> > > bus/class, add the bus/class name as a prefix to the device names used
> > > to
> > > construct the device link device name.
> > >
> > > So the devuce link device's name will be of the form:
> > > :--:
> > >
> > > [1] -
> > > https://lore.kernel.org/lkml/20201229033440.32142-1-mich...@walle.cc/
> > >
> > > Cc: sta...@vger.kernel.org
> > > Fixes: 287905e68dd2 ("driver core: Expose device link details in
> > > sysfs")
> > > Reported-by: Michael Walle 
> > > Signed-off-by: Saravana Kannan 
> > > ---
> >
> > This makes it even worse. Please see below for a full bootlog with the
> > dev_dbg() converted to dev_info() and initcall_debug enabled.
>
> Sorry if I'm missing something obvious (been a long day), but how is
> it worse? I don't see any warnings in this log. I'll reply to your
> other emails separately.
>

Nevermind, I see it now. Also, in the future, if you can dump the logs
in some kind of pastebin site, that'd be nice. Avoid the emails
becoming unwieldy and also avoids the log lines from wrapping.

-Saravana


Re: [PATCH v2] driver core: Fix device link device name collision

2021-01-07 Thread Michael Walle

Am 2021-01-08 01:14, schrieb Michael Walle:

Am 2021-01-08 00:41, schrieb Saravana Kannan:

The device link device's name was of the form:
--

This can cause name collision as reported here [1] as device names are
not globally unique. Since device names have to be unique within the
bus/class, add the bus/class name as a prefix to the device names used 
to

construct the device link device name.

So the devuce link device's name will be of the form:
:--:

[1] - 
https://lore.kernel.org/lkml/20201229033440.32142-1-mich...@walle.cc/


Cc: sta...@vger.kernel.org
Fixes: 287905e68dd2 ("driver core: Expose device link details in 
sysfs")

Reported-by: Michael Walle 
Signed-off-by: Saravana Kannan 
---


This makes it even worse.


Mhh, actually there are now other errors (the ones before might be 
fixed). Seems

to be truncated filenames and for the iommu ones, I don't know.

--
-michael


[PATCH v2] driver core: Fix device link device name collision

2021-01-07 Thread Saravana Kannan
The device link device's name was of the form:
--

This can cause name collision as reported here [1] as device names are
not globally unique. Since device names have to be unique within the
bus/class, add the bus/class name as a prefix to the device names used to
construct the device link device name.

So the devuce link device's name will be of the form:
:--:

[1] - https://lore.kernel.org/lkml/20201229033440.32142-1-mich...@walle.cc/

Cc: sta...@vger.kernel.org
Fixes: 287905e68dd2 ("driver core: Expose device link details in sysfs")
Reported-by: Michael Walle 
Signed-off-by: Saravana Kannan 
---
 Documentation/ABI/testing/sysfs-class-devlink|  4 ++--
 Documentation/ABI/testing/sysfs-devices-consumer |  5 +++--
 Documentation/ABI/testing/sysfs-devices-supplier |  5 +++--
 drivers/base/core.c  | 13 ++---
 include/linux/device.h   | 12 
 5 files changed, 26 insertions(+), 13 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-class-devlink 
b/Documentation/ABI/testing/sysfs-class-devlink
index b662f747c83e..8a21ce515f61 100644
--- a/Documentation/ABI/testing/sysfs-class-devlink
+++ b/Documentation/ABI/testing/sysfs-class-devlink
@@ -5,8 +5,8 @@ Description:
Provide a place in sysfs for the device link objects in the
kernel at any given time.  The name of a device link directory,
denoted as ... above, is of the form --
-   where  is the supplier device name and  is
-   the consumer device name.
+   where  is the supplier bus:device name and 
+   is the consumer bus:device name.
 
 What:  /sys/class/devlink/.../auto_remove_on
 Date:  May 2020
diff --git a/Documentation/ABI/testing/sysfs-devices-consumer 
b/Documentation/ABI/testing/sysfs-devices-consumer
index 1f06d74d1c3c..0809fda092e6 100644
--- a/Documentation/ABI/testing/sysfs-devices-consumer
+++ b/Documentation/ABI/testing/sysfs-devices-consumer
@@ -4,5 +4,6 @@ Contact:Saravana Kannan 
 Description:
The /sys/devices/.../consumer: are symlinks to device
links where this device is the supplier.  denotes the
-   name of the consumer in that device link. There can be zero or
-   more of these symlinks for a given device.
+   name of the consumer in that device link and is of the form
+   bus:device name. There can be zero or more of these symlinks
+   for a given device.
diff --git a/Documentation/ABI/testing/sysfs-devices-supplier 
b/Documentation/ABI/testing/sysfs-devices-supplier
index a919e0db5e90..207f5972e98d 100644
--- a/Documentation/ABI/testing/sysfs-devices-supplier
+++ b/Documentation/ABI/testing/sysfs-devices-supplier
@@ -4,5 +4,6 @@ Contact:Saravana Kannan 
 Description:
The /sys/devices/.../supplier: are symlinks to device
links where this device is the consumer.  denotes the
-   name of the supplier in that device link. There can be zero or
-   more of these symlinks for a given device.
+   name of the supplier in that device link and is of the form
+   bus:device name. There can be zero or more of these symlinks
+   for a given device.
diff --git a/drivers/base/core.c b/drivers/base/core.c
index 25e08e5f40bd..1927e70eb71a 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -470,12 +470,12 @@ static int devlink_add_symlinks(struct device *dev,
if (ret)
goto err_con;
 
-   snprintf(buf, len, "consumer:%s", dev_name(con));
+   snprintf(buf, len, "consumer:%s:%s", dev_bus_name(con), dev_name(con));
ret = sysfs_create_link(>kobj, >link_dev.kobj, buf);
if (ret)
goto err_con_dev;
 
-   snprintf(buf, len, "supplier:%s", dev_name(sup));
+   snprintf(buf, len, "supplier:%s:%s", dev_bus_name(sup), dev_name(sup));
ret = sysfs_create_link(>kobj, >link_dev.kobj, buf);
if (ret)
goto err_sup_dev;
@@ -737,8 +737,9 @@ struct device_link *device_link_add(struct device *consumer,
 
link->link_dev.class = _class;
device_set_pm_not_required(>link_dev);
-   dev_set_name(>link_dev, "%s--%s",
-dev_name(supplier), dev_name(consumer));
+   dev_set_name(>link_dev, "%s:%s--%s:%s",
+dev_bus_name(supplier), dev_name(supplier),
+dev_bus_name(consumer), dev_name(consumer));
if (device_register(>link_dev)) {
put_device(consumer);
put_device(supplier);
@@ -1808,9 +1809,7 @@ const char *dev_driver_string(const struct device *dev)
 * never change once they are set, so they don't need special care.
 */
drv = READ_ONCE(dev->driver);
-   return drv ? drv->name :
-   (dev->bus ? dev->bus->name :
-