Messages in this thread | | | Date | Sat, 28 Feb 2004 12:09:00 -0500 (EST) | From | Alan Stern <> | Subject | Re: Question about (or bug in?) the kobject implementation |
| |
On Sat, 28 Feb 2004, Michael Frank wrote:
> On Fri, 27 Feb 2004 23:02:34 -0500 (EST), Alan Stern <stern@rowland.harvard.edu> wrote: > > > > This really is a programming error. It means that kobject_get() has been > > passed a possibly stale pointer. Ipso facto, the call to kobject_put() > > that decremented the refcount to 0 was made too early, while there were > > still active pointers to the kobject floating around. > > > > It's impossible to prevent people from making programming errors or > > dereferencing stale pointers. It doesn't matter _what_ code you put in > > kobject_get() -- it will crash when given a pointer to a kobject whose > > cleanup routine has already run and deallocated the storage. > > > > The best you can do is call people's attention to such errors and fail the > > operation gracefully whenever possible (i.e., when it doesn't generate an > > addressing error). My personal choice would be to change kobject_get() as > > follows: > > > > struct kobject * kobject_get(struct kobject * kobj) > > { > > if (kobj) { > > if (atomic_read(&kobj->refcount) == 0) { > > WARN_ON(1); > > return NULL; > > } > > atomic_inc(&kobj->refcount); > > } > > return kobj; > > } > > > > I think that's about the best you can do. > > This is too ugly :-(
It's cleaner than your proposal below. It's not so different from the code that's there now. And it does what that code _ought_ to do, namely, return a NULL pointer when the kobject is no longer available.
> > And what's the answer to A'? > > The weakness is really in that the refcount is stored dynamically. > > What about a new struct to hold the pointer to the kobj and it's refcount: > > struct kobjectref { > struct kobject *kobj; > int refcount; > }; > ... > > struct kobjectref rkobj;
Since kobjects are allocated dynamically, you will have to allocate kobjectrefs dynamically as well.
> Using refkobj eliminates all problems as the pointer to the refcount can't > be invalid.
Only until you deallocate the kobjectref. And when you do, you then face exactly the same set of problems: pointers to the kobjectref will become stale. If you don't ever deallocate kobjectrefs then you have a memory leak. So this proposal doesn't solve anything, it just adds an extra layer of indirection.
Alan Stern
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |