public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Andrew Morton <akpm@osdl.org>
To: Greg KH <gregkh@suse.de>
Cc: Venkatesh Pallipadi <venkatesh.pallipadi@intel.com>,
	Arjan <arjan@linux.intel.com>,
	linux-kernel <linux-kernel@vger.kernel.org>
Subject: Re: kref refcnt and false positives
Date: Wed, 13 Dec 2006 16:41:59 -0800	[thread overview]
Message-ID: <20061213164159.f93cde95.akpm@osdl.org> (raw)
In-Reply-To: <20061214001246.GA10056@suse.de>

On Wed, 13 Dec 2006 16:12:46 -0800
Greg KH <gregkh@suse.de> wrote:

> > Original comment seemed to indicate that this conditional thing was
> > performance related. Is it really? If not, we should consider the below patch.
> 
> Yes, it's a performance gain and I don't see how this patch would change
> the above warning.

I suspect it's a false optimisation.

int kref_put(struct kref *kref, void (*release)(struct kref *kref))
{
	WARN_ON(release == NULL);
	WARN_ON(release == (void (*)(struct kref *))kfree);

	/*
	 * if current count is one, we are the last user and can release object
	 * right now, avoiding an atomic operation on 'refcount'
	 */
 	if ((atomic_read(&kref->refcount) == 1) ||
	    (atomic_dec_and_test(&kref->refcount))) {
		release(kref);
		return 1;
	}
	return 0;
}

The only time we avoid the atomic_dec_and_test() is when the object is
about to be freed.  ie: once in its entire lifetime.  And freeing the
object is part of an expensive (and rare) operation anyway.

otoh, we've gone and added a test-n-branch to the common case: those cases
where the object will not be freed.


  parent reply	other threads:[~2006-12-14  0:42 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-12-13 23:34 kref refcnt and false positives Venkatesh Pallipadi
2006-12-14  0:12 ` Greg KH
2006-12-14  0:08   ` Venkatesh Pallipadi
2006-12-14  0:41   ` Andrew Morton [this message]
2006-12-14  7:56     ` Eric Dumazet
  -- strict thread matches above, loose matches on Subject: below --
2006-12-14 23:51 Pallipadi, Venkatesh
2006-12-15  0:19 ` Eric W. Biederman
2006-12-15  0:35   ` Andrew Morton
2006-12-15  0:53     ` Eric W. Biederman
     [not found] <200612210901.kBL91MwR027509@hera.kernel.org>
2006-12-31 15:16 ` David Woodhouse
2007-01-01 20:13   ` Benjamin Herrenschmidt

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=20061213164159.f93cde95.akpm@osdl.org \
    --to=akpm@osdl.org \
    --cc=arjan@linux.intel.com \
    --cc=gregkh@suse.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=venkatesh.pallipadi@intel.com \
    /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