Re: [PATCH 085/196] kset: convert s390 ipl.c to use kset_create

Greg KH
2008-01-26T05:36:48Z

On Sat, Jan 26, 2008 at 12:11:33AM +0100, Heiko Carstens wrote:
> On Fri, Jan 25, 2008 at 09:48:58AM -0800, Greg KH wrote:
> > On Fri, Jan 25, 2008 at 01:20:53PM +0100, Heiko Carstens wrote:
> > > On Thu, Jan 24, 2008 at 11:31:54PM -0800, Greg Kroah-Hartman wrote:
> > > > Dynamically create the kset instead of declaring it statically.
> > > > This makes the kobject attributes now work properly that I broke in the
> > > > previous patch.
> > >
> > > Could you please merge this and the previous patch before it goes
> > > upstream? Having an intermediate state where things are broken
> > > will cause pain and additional work in case of bisecting.
> >
> > It will not cause a build error (see the previous patch for details.)
> > The sysfs files will not properly show the correct data, that is all.
> >
> > The odds that you will hit this in a 'git bisect' is VERY low, and the
> > previous patch states that the files are now broken, so there should not
> > be any confusion regarding any user that might run across this.
>
> The odds are very low, as long as not more patch sets come up which
> introduce intermediate broken kernels.
> What exactly is the advantage of breaking the kernel with patch 1 and
> then fix it again with patch 2 instead of doing the straight forward
> conversions all with one patch?

I was trying to do one logical thing at a time with this driver as I did
not have the hardware to test, and I could not even build the code at
the time.

In looking more closer, I think the 084 patch might still work properly,
but I can't guarantee it as the the default kobject parent might not be
pointing to the correct attribute at the time. I know 085 fixes this to
be sure that it will work properly.

It helped in reviewing this code by the other s390 developers to have
this in at least 2 pieces, to try to untangle the mess of sysfs files,
ksets, and other attrocities that you all have grown into over the
years.

So again, I'm sorry if this happens to break your run-time tests when
doing a 'git bisect', but as I explicitly stated it did in the patch, I
think everyone is properly forwarned :)

This core rework was tough to do, there was a reason no one had done it
before. Now it is cleaner, smaller, able to be understood by at least
one active kernel developer, if not more, and it's documented, with
working examples. If the downside of this effort is only this one thing
(note that others are finally finding real bugs...) I'll be very happy.

thanks,

greg k-h