On Wed, Mar 28, 2007 at 02:49:12PM +0800, Shaohua Li wrote:
> On Tue, 2007-03-27 at 22:58 -0700, Greg KH wrote:
> > On Wed, Mar 28, 2007 at 01:39:26PM +0800, Shaohua Li wrote:
> > > On Tue, 2007-03-27 at 22:27 -0700, Greg KH wrote:
> > > >
> > > > Putting more than one kobject in the same structure is a broken design.
> > > > How can you control the lifetime rules properly if there are two
> > > > reference counts for the same structure? It doesn't work.
> > > >
> > > > If you really need something like this, then just use a pointer to a
> > > > kobject for one of them instead of embedding it. Why do you need two
> > > > different kobjects here?
> > > Our data structure is something like below:
> > >
> > > struct foo {
> > > kobject kobja;
> > > }
> > >
> > > struct bar {
> > > struct foo foo[];
> >
> > Ick, don't do that...
> why?
> > > kobject kobjb
Because you have multiple kobjects in the same object.
It's just that simple, the lifetime rules for such a thing is almost
impossible to track properly. Don't do it!
> > > }
> > >
> > > kobjb's .release will free struct bar. kobjb is the parent of kobja. if
> > > you have a reference on kobja, then kobjb can't be released too, right?
> > > So we only kobjb provide a .release to free the memory, kobja's .release
> > > isn't required.
> >
> > Why not just use the "normal" parent/child relationship with the
> > kobjects like the rest of the kernel does?
> I still didn't get the reason why we couldn't do this in the way of my
> patch. As I said, there isn't risk to use 'freed memory'. I can make the
> 'struct foo' a pointer, but this will mess the cpuidle driver.
Again, the main point is you can not have more than one reference count
for the same structure. It just does not work at all.
So please, fix the code, it is broken.
And yes, I know of other places in the kernel (scsi stack...) that
violate this, but that only means that they are wrong, not that it is an
excuse for you to do it also.
thanks,
greg k-h
-
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
