From: Waiman Long <waiman.long@hp.com>
To: Peter Zijlstra <peterz@infradead.org>
Cc: linux-arch@vger.kernel.org, Rik van Riel <riel@redhat.com>,
Raghavendra K T <raghavendra.kt@linux.vnet.ibm.com>,
Oleg Nesterov <oleg@redhat.com>,
kvm@vger.kernel.org,
Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>,
Daniel J Blueman <daniel@numascale.com>,
x86@kernel.org, Paolo Bonzini <paolo.bonzini@gmail.com>,
linux-kernel@vger.kernel.org,
virtualization@lists.linux-foundation.org,
Scott J Norton <scott.norton@hp.com>,
Ingo Molnar <mingo@redhat.com>,
David Vrabel <david.vrabel@citrix.com>,
"H. Peter Anvin" <hpa@zytor.com>,
xen-devel@lists.xenproject.org,
Thomas Gleixner <tglx@linutronix.de>,
"Paul E. McKenney" <paulmck@linux.vnet.ibm.com>,
Linus Torvalds <torvalds@linux-foundation.org>,
Boris Ostrovsky <boris.ostrovsky@oracle.com>,
Douglas Hatch <doug.hatch@hp.com>
Subject: Re: [PATCH v16 13/14] pvqspinlock: Improve slowpath performance by avoiding cmpxchg
Date: Thu, 30 Apr 2015 14:49:26 -0400 [thread overview]
Message-ID: <55427936.4080900@hp.com> (raw)
In-Reply-To: <20150429181112.GI23123@twins.programming.kicks-ass.net>
On 04/29/2015 02:11 PM, Peter Zijlstra wrote:
> On Fri, Apr 24, 2015 at 02:56:42PM -0400, Waiman Long wrote:
>> In the pv_scan_next() function, the slow cmpxchg atomic operation is
>> performed even if the other CPU is not even close to being halted. This
>> extra cmpxchg can harm slowpath performance.
>>
>> This patch introduces the new mayhalt flag to indicate if the other
>> spinning CPU is close to being halted or not. The current threshold
>> for x86 is 2k cpu_relax() calls. If this flag is not set, the other
>> spinning CPU will have at least 2k more cpu_relax() calls before
>> it can enter the halt state. This should give enough time for the
>> setting of the locked flag in struct mcs_spinlock to propagate to
>> that CPU without using atomic op.
> Yuck! I'm not at all sure you can make assumptions like that. And the
> worst part is, if it goes wrong the borkage is subtle and painful.
I do think the code is OK. However, you are right that if my reasoning
is incorrect, the resulting bug will be really subtle. So I am going to
withdraw this particular patch as it has no functional impact to the
overall patch series. Please let me know if you have any other comments
on other parts of the series and I will send send out a new series
without this particular patch.
Cheers,
Longman
next prev parent reply other threads:[~2015-04-30 18:49 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-04-24 18:56 [PATCH v16 00/14] qspinlock: a 4-byte queue spinlock with PV support Waiman Long
2015-04-24 18:56 ` [PATCH v16 01/14] qspinlock: A simple generic 4-byte queue spinlock Waiman Long
2015-04-24 18:56 ` [PATCH v16 02/14] qspinlock, x86: Enable x86-64 to use " Waiman Long
2015-04-24 18:56 ` [PATCH v16 03/14] qspinlock: Add pending bit Waiman Long
2015-04-24 18:56 ` [PATCH v16 04/14] qspinlock: Extract out code snippets for the next patch Waiman Long
2015-04-24 18:56 ` [PATCH v16 05/14] qspinlock: Optimize for smaller NR_CPUS Waiman Long
2015-04-24 18:56 ` [PATCH v16 06/14] qspinlock: Use a simple write to grab the lock Waiman Long
2015-04-24 18:56 ` [PATCH v16 07/14] qspinlock: Revert to test-and-set on hypervisors Waiman Long
2015-04-24 18:56 ` [PATCH v16 08/14] pvqspinlock: Implement simple paravirt support for the qspinlock Waiman Long
2015-05-04 14:20 ` Peter Zijlstra
2015-05-04 17:15 ` Waiman Long
2015-04-24 18:56 ` [PATCH v16 09/14] pvqspinlock, x86: Implement the paravirt qspinlock call patching Waiman Long
2015-04-24 18:56 ` [PATCH v16 10/14] pvqspinlock, x86: Enable PV qspinlock for KVM Waiman Long
2015-04-24 18:56 ` [PATCH v16 11/14] pvqspinlock, x86: Enable PV qspinlock for Xen Waiman Long
2015-04-24 18:56 ` [PATCH v16 12/14] pvqspinlock: Only kick CPU at unlock time Waiman Long
2015-04-24 18:56 ` [PATCH v16 13/14] pvqspinlock: Improve slowpath performance by avoiding cmpxchg Waiman Long
2015-04-29 18:11 ` Peter Zijlstra
2015-04-29 18:27 ` Linus Torvalds
2015-04-30 18:56 ` Waiman Long
2015-04-30 18:49 ` Waiman Long [this message]
2015-05-04 14:05 ` Peter Zijlstra
2015-05-04 17:18 ` Waiman Long
2015-04-24 18:56 ` [PATCH v16 14/14] pvqspinlock: Collect slowpath lock statistics Waiman Long
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=55427936.4080900@hp.com \
--to=waiman.long@hp.com \
--cc=boris.ostrovsky@oracle.com \
--cc=daniel@numascale.com \
--cc=david.vrabel@citrix.com \
--cc=doug.hatch@hp.com \
--cc=hpa@zytor.com \
--cc=konrad.wilk@oracle.com \
--cc=kvm@vger.kernel.org \
--cc=linux-arch@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=oleg@redhat.com \
--cc=paolo.bonzini@gmail.com \
--cc=paulmck@linux.vnet.ibm.com \
--cc=peterz@infradead.org \
--cc=raghavendra.kt@linux.vnet.ibm.com \
--cc=riel@redhat.com \
--cc=scott.norton@hp.com \
--cc=tglx@linutronix.de \
--cc=torvalds@linux-foundation.org \
--cc=virtualization@lists.linux-foundation.org \
--cc=x86@kernel.org \
--cc=xen-devel@lists.xenproject.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 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).