All of lore.kernel.org
 help / color / mirror / Atom feed
From: "H. Peter Anvin" <hpa@zytor.com>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Zachary Amsden <zamsden@redhat.com>,
	linux-kernel@vger.kernel.org,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>,
	x86@kernel.org, Avi Kivity <avi@redhat.com>
Subject: Re: [PATCH] x86 rwsem optimization extreme
Date: Wed, 17 Feb 2010 14:29:34 -0800	[thread overview]
Message-ID: <4B7C6DCE.3080609@zytor.com> (raw)
In-Reply-To: <alpine.LFD.2.00.1002171403160.4141@localhost.localdomain>

On 02/17/2010 02:10 PM, Linus Torvalds wrote:
> 
> 
> On Wed, 17 Feb 2010, Zachary Amsden wrote:
>>
>> The x86 instruction set provides the ability to add an additional
>> bit into addition or subtraction by using the carry flag.
>> It also provides instructions to directly set or clear the
>> carry flag.  By forcibly setting the carry flag, we can then
>> represent one particular 64-bit constant, namely
>>
>>    0xffffffff + 1 = 0x100000000
>>
>> using only 32-bit values.  In particular we can optimize the rwsem
>> write lock release by noting it is of exactly this form.
> 
> Don't do this.
> 
> Just shift the constants down by two, and suddenly you don't need any 
> clever tricks, because all the constants fit in 32 bits anyway, 
> regardless of sign issues.
> 

Why bother at all?  I thought it mattered when I saw __downgrade_write()
as an inline, but in fact it is only ever used inside the
downgrade_write() out-of-line function, so we're talking about saving
*five bytes* across the whole kernel in the best case.  I vote for
leaving it the way it is, and get the very slight extra readability.
There is no point in moving bits around, either.

	-hpa

  reply	other threads:[~2010-02-17 22:30 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-02-17 21:58 [PATCH] x86 rwsem optimization extreme Zachary Amsden
2010-02-17 22:10 ` Linus Torvalds
2010-02-17 22:29   ` H. Peter Anvin [this message]
2010-02-17 23:29   ` H. Peter Anvin
2010-02-18  1:03     ` Zachary Amsden
2010-02-18  1:53     ` Linus Torvalds
2010-02-18  1:59       ` H. Peter Anvin
2010-02-18  4:25         ` Zachary Amsden
2010-02-18  8:12           ` Andi Kleen
2010-02-18  8:24             ` Zachary Amsden
2010-02-18  9:29               ` Andi Kleen
2010-02-18 10:55               ` Ingo Molnar

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=4B7C6DCE.3080609@zytor.com \
    --to=hpa@zytor.com \
    --cc=avi@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.org \
    --cc=x86@kernel.org \
    --cc=zamsden@redhat.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.