What is the purpose behind these patches?
I can see the /dis/advantages:
a) Seems to hide bugs - it's now not clear whether
a code patch is meant to be run through when 'foo'
is NULL or not. Whilst I appreciate kfree() checks
foo for NULL anyway, and it's going to be hard to
turn that into a BUG(), it doesn't mean we should
make more of a mess.
b) I expect kmalloc() calls and kfree() calls to balance,
and so would any kmalloc/kfree leak debugger.
c) When 'foo' was NULL before, there was a fast path
where no kfree() function call would be made, and no
write 'foo=NULL;' would subsequently be performed.
Whilst I appreciate not all these are on a critical
path, is there any reason why we now want to do a
function call and a write when previously we avoided
it?
If what you are worried about is performance loss through
checking the argument to kfree() against NULL twice, wouldn't
we be better doing something like this:
--- linux.clean/kernel/slab.c Sat Jan 27 20:05:11 2001
+++ linux/kernel/slab.c Sat Dec 1 17:31:38 2001
@@ -1577,7 +1577,7 @@
kmem_cache_t *c;
unsigned long flags;
- if (!objp)
+ if (unlikely(!objp))
return;
local_irq_save(flags);
CHECK_PAGE(virt_to_page(objp));
And then go through all the ones in your patch, and
rather than deleting them, inserting likely() / unlikely()
as appropriate in the ones that have any chance of actually
affecting performance.
Or even better, add in slab.h
static inline void kfree(const void * objp)
{
if (likely(objp)) __kfree(objp);
/* perhaps it should 'else BUG() here' but
* can't do that today
*/
}
&
--- linux.clean/kernel/slab.c Sat Jan 27 20:05:11 2001
+++ linux/kernel/slab.c Sat Dec 1 17:35:35 2001
@@ -1572,13 +1572,11 @@
* Don't free memory not originally allocated by kmalloc()
* or you will run into trouble.
*/
-void kfree (const void *objp)
+void __kfree (const void *objp)
{
kmem_cache_t *c;
unsigned long flags;
- if (!objp)
- return;
local_irq_save(flags);
CHECK_PAGE(virt_to_page(objp));
c = GET_PAGE_CACHE(virt_to_page(objp));
And on a good day gcc may optimize out all the double tests
anyhow.
-- Alex Bligh - 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/
This archive was generated by hypermail 2b29 : Fri Dec 07 2001 - 21:00:16 EST