Re: [PATCH 8/19]: SCST SYSFS interface implementation

From: Greg KH
Date: Fri Oct 22 2010 - 15:25:17 EST


On Fri, Oct 22, 2010 at 10:40:34PM +0400, Vladislav Bolkhovitin wrote:
> Greg KH, on 10/22/2010 09:56 PM wrote:
> > On Fri, Oct 22, 2010 at 09:30:53PM +0400, Vladislav Bolkhovitin wrote:
> >> + unsigned int tgt_kobj_initialized:1;
> >
> > It's the middle of the merge window, and I'm about to go on vacation, so
> > I didn't read this patch after this line.
> >
> > It's obvious that this patch is wrong, you shouldn't need to worry about
> > this. And even if you did, you don't need this flag.
> >
> > Why are you trying to do something that no one else needs? Why make
> > things harder than they have to be.
>
> I tried to explain that to you in http://lkml.org/lkml/2010/10/14/291
> and mentioned there the need to create this flag to track
> half-initialized kobjects. You agreed
> (http://lkml.org/lkml/2010/10/14/299) that don't return half-initialized
> objects is a regular kernel practice, but then requested to strictly
> bound the larger object freeing to its kobject release(), which means
> that all SYSFS creating functions now have to return half-initialized
> SYSFS hierarchy in case of any error. Hence the flag to track it.

I agreed that you needed to do something about it, not that this is the
correct way to do it.

Think for a second as to why your code path looks different from EVERY
other kobject user in the kernel. Perhaps it is incorrect? You don't
need all this completion mess, in fact, it's wrong.

Just do what everyone else does please, as that is the simpler, and
correct, way to do it.

Oh, and why are you using a kobject at all anyway? Shouldn't you be
using a 'struct device'?

Anyway, I don't have time for this anymore for the next 2 weeks, good
luck.

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/