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

[Valdis.Kletnieks@vt.edu]

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: pgpSBGDPqBvN9.pgp
Description: PGP signature