Thread (61 messages) 61 messages, 7 authors, 2010-07-28

Re: [PATCH] Driver-core: Fix bluetooth network device rename regression

From: Eric W. Biederman <hidden>
Date: 2010-07-23 01:32:13
Subsystem: driver core, kobjects, debugfs and sysfs, the rest · Maintainers: Greg Kroah-Hartman, "Rafael J. Wysocki", Danilo Krummrich, Linus Torvalds

Greg KH [off-list ref] writes:
On Thu, Jul 22, 2010 at 08:36:41PM +0200, Johannes Berg wrote:
quoted
On Thu, 2010-07-22 at 11:28 -0700, Greg KH wrote:
quoted
It worked only because no one realized that it was broken with the
DEPRECATED option enabled.  When that is enabled, it is broken, right?
I'm pretty sure I always had that enabled, and never had issues. Can't
test right now since I don't have that option back yet in the tree I'm
using.
quoted
Eric's changes to sysfs to add namespace support exposed this breakage.
That's not a reason to paper over the problem, but it should be driving
someone to fix it correctly, as has been pointed out a number of times
already.
I'm just contesting that that someone should be me. I don't think you
get to blame driver developers for doing something that worked and
solved the problem they needed to solve. sysfs is largely opaque to most
of us already, and it now sure feels like Kay decided to change the
rules underneath the code in saying "this was wrong all along".
Well, if it worked before, and it doesn't now, that's due to Eric's
changes, nothing Kay and I did here :)
It mostly worked before, and it works with CONFIG_SYSFS_DEPRECATED=y
(After I my stupid bug is fixed).

With CONFIG_SYSFS_DEPRECATED=y we have in sysfs:
........./net:bnep0 -> /sys/class/net/bnep0
/sys/class/net/bnep0 is a directory.

With CONFIG_SYSFS_DEPRECATED=n we have in sysfs:
........./bnep0
/sys/class/net/bnep0 -> ......./bnep0

Where for a normal network device we have:
With CONFIG_SYSFS_DEPRECATED=y we have in sysfs:
........./net:eth0 -> /sys/class/net/eth0
/sys/class/net/bnep0 is a directory.

With CONFIG_SYSFS_DEPRECATED=n we have in sysfs:
........./net/eth0
/sys/class/net/eth0 -> ......./net/eth0

The new sysfs layout loses the net namespace separator for bluetooth
devices.
But, in looking at it closer, it does seem that the code is doing things
that was not expected to work at all previously, and It's amazing that
it did.  I thought Kay offered to help fix it all up, and provided 2
different ways to do it.  I know they aren't trivial, but then again,
your usage of sysfs is not trivial either...
I don't expect much just the upper levels to work correctly.  The fact
this works on all but a handful of drivers is what I find dist

Does this version of the change look less bleh worthy?  The effect is
the same as my previous patch but the test is more abstract so the
effect is not strictly limited to /sys/class/net.
diff --git a/drivers/base/core.c b/drivers/base/core.c
index 9630fbd..9b9d3bd 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -673,7 +673,7 @@ static struct kobject *get_device_parent(struct device *dev,
 		 */
 		if (parent == NULL)
 			parent_kobj = virtual_device_parent(dev);
-		else if (parent->class)
+		else if (parent->class && !dev->class->ns_type)
 			return &parent->kobj;
 		else
 			parent_kobj = &parent->kobj;

Eric
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help