Re: [PATCH 063/196] kset: convert /sys/devices to use kset_create

Re: [PATCH 063/196] kset: convert /sys/devices to use kset_create

[Olof Johansson]

On Thu, Jan 24, 2008 at 11:10:01PM -0800, Greg Kroah-Hartman wrote:
> Dynamically create the kset instead of declaring it statically.  We also
> rename devices_subsys to devices_kset to catch all users of the
> variable.

Guess what, you broke powerpc again!

olof@quad:~/work/linux/k.org $ git grep devices_subsys
arch/powerpc/kernel/vio.c:extern struct kset devices_subsys; /* needed for vio_find_name() */
arch/powerpc/kernel/vio.c:      found = kset_find_obj(&devices_subsys, kobj_name);

Obviously causes build failues, even of ppc64_defconfig.

(I can unfortunately not boot test, since I lack hardware that uses vio)


Signed-off-by: Olof Johansson <olof@lixom.net>

diff --git a/arch/powerpc/kernel/vio.c b/arch/powerpc/kernel/vio.c
index 19a5656..ee752ab 100644
--- a/arch/powerpc/kernel/vio.c
+++ b/arch/powerpc/kernel/vio.c
@@ -37,7 +37,7 @@
 #include <asm/iseries/hv_call_xm.h>
 #include <asm/iseries/iommu.h>
 
-extern struct kset devices_subsys; /* needed for vio_find_name() */
+extern struct kset *devices_kset; /* needed for vio_find_name() */
 
 static struct bus_type vio_bus_type;
 
@@ -369,7 +369,7 @@ static struct vio_dev *vio_find_name(const char *kobj_name)
 {
 	struct kobject *found;
 
-	found = kset_find_obj(&devices_subsys, kobj_name);
+	found = kset_find_obj(devices_kset, kobj_name);
 	if (!found)
 		return NULL;
 

Re: [PATCH 063/196] kset: convert /sys/devices to use kset_create

[Greg KH]

On Fri, Jan 25, 2008 at 09:40:55PM -0600, Olof Johansson wrote:
> On Thu, Jan 24, 2008 at 11:10:01PM -0800, Greg Kroah-Hartman wrote:
> > Dynamically create the kset instead of declaring it statically.  We also
> > rename devices_subsys to devices_kset to catch all users of the
> > variable.
> 
> Guess what, you broke powerpc again!

I did this ON PURPOSE!!!

The linux-kernel archives hold the details, and I was told by the PPC64
IBM people that they would fix this properly for 2.6.25, and not to hold
back on my changes.  This has been known for many months now.

> olof@quad:~/work/linux/k.org $ git grep devices_subsys
> arch/powerpc/kernel/vio.c:extern struct kset devices_subsys; /* needed for vio_find_name() */
> arch/powerpc/kernel/vio.c:      found = kset_find_obj(&devices_subsys, kobj_name);
> 
> Obviously causes build failues, even of ppc64_defconfig.
> 
> (I can unfortunately not boot test, since I lack hardware that uses vio)
> 
> 
> Signed-off-by: Olof Johansson <olof@lixom.net>
> 
> diff --git a/arch/powerpc/kernel/vio.c b/arch/powerpc/kernel/vio.c
> index 19a5656..ee752ab 100644
> --- a/arch/powerpc/kernel/vio.c
> +++ b/arch/powerpc/kernel/vio.c
> @@ -37,7 +37,7 @@
>  #include <asm/iseries/hv_call_xm.h>
>  #include <asm/iseries/iommu.h>
>  
> -extern struct kset devices_subsys; /* needed for vio_find_name() */
> +extern struct kset *devices_kset; /* needed for vio_find_name() */

No, this just papers over the real problem here.  For some reason, the
vio code thinks it is acceptable to walk the whole device tree and match
by a name and just assume that they got the correct device.  You call
this "enterprise grade"?  :)

You need to just put your device on a real bus, and then just walk the
bus.  That's the ONLY way you can guarantee the proper name will return
what you want, and you get the pointer that you really think you are
getting.

There is a reason that devices_kset is not exported, don't make me go
and have to name it something like:
	devices_kset_dont_touch_this_or_gregkh_will_make_fun_of_you

Or I'll just mush 3 files in the driver core together and keep the
symbol from being accessible at all.

So no, I'm going to leave the build broken for this code, because that
is what it really is.

Please fix it correctly.

thanks,

greg k-h

Re: [PATCH 063/196] kset: convert /sys/devices to use kset_create

[Olof Johansson]

On Fri, Jan 25, 2008 at 09:24:54PM -0800, Greg KH wrote:
> On Fri, Jan 25, 2008 at 09:40:55PM -0600, Olof Johansson wrote:
> > On Thu, Jan 24, 2008 at 11:10:01PM -0800, Greg Kroah-Hartman wrote:
> > > Dynamically create the kset instead of declaring it statically.  We also
> > > rename devices_subsys to devices_kset to catch all users of the
> > > variable.
> > 
> > Guess what, you broke powerpc again!
> 
> I did this ON PURPOSE!!!
> 
> The linux-kernel archives hold the details, and I was told by the PPC64
> IBM people that they would fix this properly for 2.6.25, and not to hold
> back on my changes.  This has been known for many months now.

Yeah, my bad. :( I thought this was new, but it was just not exposed by
my scripts because of the EHEA build errors (they were actual compile
errors, while this was a link error, so it never go this far). That's
why I didn't see it in -rc8-mm1 and thought it was new.

> > -extern struct kset devices_subsys; /* needed for vio_find_name() */
> > +extern struct kset *devices_kset; /* needed for vio_find_name() */
> 
> No, this just papers over the real problem here.  For some reason, the
> vio code thinks it is acceptable to walk the whole device tree and match
> by a name and just assume that they got the correct device.  You call
> this "enterprise grade"?  :)
>
> You need to just put your device on a real bus, and then just walk the
> bus.  That's the ONLY way you can guarantee the proper name will return
> what you want, and you get the pointer that you really think you are
> getting.

Hmm, they are already on a bus. Odd, must be done like this for legacy
reasons.

[...]

> So no, I'm going to leave the build broken for this code, because that
> is what it really is.
> 
> Please fix it correctly.

Alright, I'll leave that to people who care about vio and can test the
proper fix. After a quick glance it looks like it should be easy to use
bus_find_device() for it instead.


-Olof