From: Alex Shi <alex.shi@intel.com>
To: Jan Beulich <JBeulich@suse.com>
Cc: jeremy@goop.org,
"asit.k.mallick@intel.com" <asit.k.mallick@intel.com>,
"x86@kernel.org" <x86@kernel.org>,
tglx@linutronix.de, Andi Kleen <ak@linux.intel.com>,
"mingo@redhat.com" <mingo@redhat.com>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"hpa@zytor.com" <hpa@zytor.com>
Subject: Re: [RFC patch] cmpxchg_double: remove local variables to get better performance
Date: Fri, 02 Mar 2012 23:12:54 +0800 [thread overview]
Message-ID: <4F50E376.3030000@intel.com> (raw)
In-Reply-To: <4F509CE30200007800075F84@nat28.tlf.novell.com>
On 03/02/2012 05:11 PM, Jan Beulich wrote:
>>>> On 02.03.12 at 10:00, Alex Shi <alex.shi@intel.com> wrote:
>> Yes, we can use cast for intermediate data. And actually, current kernel
>> has live mis-used case on cmpxchg(), that I plan to point out too.
>>
>> -- a/virt/kvm/kvm_main.c
>> +++ b/virt/kvm/kvm_main.c
>> @@ -203,12 +203,12 @@ static bool make_all_cpus_request(struct kvm *kvm,
>> unsigned int req)
>>
>> void kvm_flush_remote_tlbs(struct kvm *kvm)
>> {
>> - int dirty_count = kvm->tlbs_dirty;
>> + long dirty_count = kvm->tlbs_dirty;
>>
>> smp_mb();
>> if (make_all_cpus_request(kvm, KVM_REQ_TLB_FLUSH))
>> ++kvm->stat.remote_tlb_flush;
>> - cmpxchg(&kvm->tlbs_dirty, dirty_count, 0);
>> + cmpxchg(&kvm->tlbs_dirty, dirty_count, 0L);
>
> Indeed - the cmpxchg would fail if the value doesn't fit. But this is not
> to say that in certain cases it isn't valid to pass an int for the second
> and/or third argument. (And quite likely the issue here is theoretical
> only anyway.)
It may cause potential issue, if it is tlbs_dirty mis-used here, not
dirty_count. If so, it may cause data damage.
>
> In particular, requiring an L suffix here on literals should be avoided.
Even the each macro may save 0x40 bytes text, and bring more 10%
execution speed?
>
> Jan
>
>> }
>
>
next prev parent reply other threads:[~2012-03-02 15:12 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-03-02 8:31 [RFC patch] cmpxchg_double: remove local variables to get better performance Alex Shi
2012-03-02 8:54 ` Jan Beulich
2012-03-02 9:00 ` Alex Shi
2012-03-02 9:11 ` Jan Beulich
2012-03-02 15:12 ` Alex Shi [this message]
2012-03-02 15:30 ` Jan Beulich
2012-03-03 6:03 ` Alex Shi
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=4F50E376.3030000@intel.com \
--to=alex.shi@intel.com \
--cc=JBeulich@suse.com \
--cc=ak@linux.intel.com \
--cc=asit.k.mallick@intel.com \
--cc=hpa@zytor.com \
--cc=jeremy@goop.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=tglx@linutronix.de \
--cc=x86@kernel.org \
/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.