LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: "Jörn Engel" <joern@lazybastard.org>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: Matt Mackall <mpm@selenic.com>,
	Christoph Lameter <clameter@sgi.com>,
	Pekka J Enberg <penberg@cs.helsinki.fi>,
	ast@domdv.de, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] slab: deal with NULL pointers passed to kmem_cache_free
Date: Mon, 19 Mar 2007 23:04:14 +0100	[thread overview]
Message-ID: <20070319220413.GA27302@lazybastard.org> (raw)
In-Reply-To: <20070319141038.212d4ac9.akpm@linux-foundation.org>

On Mon, 19 March 2007 14:10:38 -0700, Andrew Morton wrote:
> 
> Would prefer to do:
> 
> static inline void kmem_cache_free_if_not_null(struct kmem_cache *cachep,
> 						void *objp)
> {
> 	if (objp)
> 		kmem_cache_free(cachep, objp);
> }
> 
> so that we don't add extra overhead to all the thousands of existing,
> well-behaved callsites.

In principle, this would work.  But two things need changing, imho:
1. Don't inline the function.  kmem_cache_free() has only about 34 NULL
   callers, if my grep is reliable, so this case is arguable.  But in
   general, out-of-line functions are better than many extra
   conditionals pulled in through the inline one.
2. Switch the names.  According to Rusty's benchmark, the easiest way to
   use and interface should be the correct one.  Every new driver
   written by a rookie will call kmem_cache_free(), simply because the
   name seems simpler.

void kmem_cache_free_fast(struct kmem_cache *cachep, void *objp)
{
	/* old kmem_cache_free() */
}

void kmem_cache_free(struct kmem_cache *cachep, void *objp)
{
	if (likely(objp))
		kmem_cache_free_fast(cachep, objp);
}

Jörn

-- 
Correctness comes second.
Features come third.
Performance comes last.
Maintainability is easily forgotten.

  parent reply	other threads:[~2007-03-19 22:09 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-03-19  8:27 [PATCH] slab: deal with NULL pointers passed to kmem_cache_free Pekka J Enberg
2007-03-19 11:31 ` Andrew Morton
2007-03-19 11:40   ` Pekka J Enberg
2007-03-19 17:08 ` Christoph Lameter
2007-03-19 17:31   ` Pekka J Enberg
2007-03-19 20:49   ` Matt Mackall
2007-03-19 21:10     ` Andrew Morton
2007-03-19 21:25       ` Pekka Enberg
2007-03-19 21:41         ` Andrew Morton
2007-03-19 21:49           ` Matt Mackall
2007-03-20  7:06           ` Pekka Enberg
2007-03-20 18:41             ` Christoph Lameter
2007-03-21 11:42               ` Pekka J Enberg
2007-03-20  7:14           ` Pekka Enberg
2007-03-20  7:39             ` Eric Dumazet
2007-03-20  7:47               ` Pekka J Enberg
2007-03-20  7:56                 ` Eric Dumazet
2007-03-21 10:11                 ` Jarek Poplawski
2007-03-21 12:13                   ` Pekka Enberg
2007-03-21 13:31                     ` Jarek Poplawski
2007-03-21 13:36                       ` Pekka J Enberg
2007-03-21 14:11                         ` Rafael J. Wysocki
2007-03-21 14:41                           ` Pekka Enberg
2007-03-21 16:30                             ` Andrew Morton
2007-03-21 17:54                               ` Jörn Engel
2007-03-21 18:32                               ` Pekka Enberg
2007-03-21 14:45                         ` Jarek Poplawski
2007-03-19 22:04       ` Jörn Engel [this message]
2007-03-19 21:16     ` Christoph Lameter
2007-03-19 21:44       ` Matt Mackall
2007-03-19 23:32 ` Andreas Steinmetz

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20070319220413.GA27302@lazybastard.org \
    --to=joern@lazybastard.org \
    --cc=akpm@linux-foundation.org \
    --cc=ast@domdv.de \
    --cc=clameter@sgi.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mpm@selenic.com \
    --cc=penberg@cs.helsinki.fi \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).