Re: [patch 1/4] kref: warn kref_put() with unreferenced kref

From: Valdis . Kletnieks
Date: Tue Apr 25 2006 - 00:25:41 EST


On Mon, 24 Apr 2006 20:51:28 PDT, Greg KH said:
> On Mon, Apr 24, 2006 at 04:33:34PM +0800, Akinobu Mita wrote:

> > --- 2.6-git.orig/lib/kref.c
> > +++ 2.6-git/lib/kref.c
> > @@ -49,6 +49,7 @@ void kref_get(struct kref *kref)
> > */
> > int kref_put(struct kref *kref, void (*release)(struct kref *kref))
> > {
> > + WARN_ON(atomic_read(&kref->refcount) < 1);
>
> How can this ever be true? If the refcount _ever_ goes below 1, the
> object is freed.

Maybe it should BUG_ON instead in that case. ;)

And strictly speaking, as long as the kref.c stuff is the only stuff to
play with ->refcount, that *should* be true. On the other hand, if somebody
has a bad pointer that just did a fandango on core, it would be a nice thing
to know that. Looking at the *next* few lines:

if ((atomic_read(&kref->refcount) == 1) ||
(atomic_dec_and_test(&kref->refcount))) {
release(kref);
return 1;
}
return 0;

If we managed to get a -1 smashed in there, this won't actually trigger
for another 2**32-2 or so kref_put calls - the first test is for "exactly 1",
and the dec_and_test is for "exactly zero"...

Attachment: pgp00000.pgp
Description: PGP signature