Re: [PATCH] likely cleanup: remove unlikely for kfree(NULL)

[Andreas Mohr <andi@rhlx01.fht-esslingen.de>]

Hi,

On Wed, Apr 26, 2006 at 10:30:17AM +0300, Pekka Enberg wrote:
> On 4/25/06, Hua Zhong <hzhong@gmail.com> wrote:
> > diff --git a/mm/slab.c b/mm/slab.c
> > index e6ef9bd..0fbc854 100644
> > --- a/mm/slab.c
> > +++ b/mm/slab.c
> > @@ -3380,7 +3380,7 @@ void kfree(const void *objp)
> >         struct kmem_cache *c;
> >         unsigned long flags;
> >
> > -       if (unlikely(!objp))
> > +       if (!objp)
> >                 return;
> 
> NAK. Fix the callers instead.

I don't know. Why then did Wine decide in a lengthy discussion to allow
bogus NULL calls to HeapFree() instead of having NULL checks repeated all over
the place, thus leading to some form of code size bloat?

exit:
	if (foobuffer)
		kfree(foobuffer);
	if (frobbledma)
		kfree(frobbledma);
	if (init_sequence)
		kfree(init_sequence);

	return result;

sure sounds worse than

exit:
	kfree(foobuffer);
	kfree(frobbledma);
	kfree(init_sequence);

	return result;
	
	

Perhaps one way to resolve this would be to add an explanatory comment
to the kfree() NULL check that says that the most frequent NULL abusers
should be fixed (e.g. via call trace profiling), and *only those*?

And drop the (un)likely() in that case, of course, since it's a relatively
"average" decision here.

Andreas Mohr
-
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/