On Fri, Mar 30, 2007 at 02:33:44PM +0800, Shaohua Li wrote:
> On Thu, 2007-03-29 at 22:18 -0700, Greg KH wrote:
> > 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.
> We don't use the kobject to track the reference count.
Then what do you use it for?
And it doesn't matter if you use it or not, the reference count _is_
used by the kobject and sysfs code, so you have to be aware of it.
> But anyway, below patch should make you happy.
I'm still confused as to why you are creating these "extra" kobjects.
What are they used for?
But yes, it is "better" in that you are abiding by the reference count
rules properly, although your completion handling seems like it might
get into a deadlock, but I'll trust you that it works properly :)
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
