From: Peter Zijlstra <peterz@infradead.org>
To: Waiman Long <waiman.long@hp.com>
Cc: x86@kernel.org, Gleb Natapov <gleb@redhat.com>,
Aswin Chandramouleeswaran <aswin@hp.com>,
Marcos Matsunaga <Marcos.Matsunaga@oracle.com>,
"H. Peter Anvin" <hpa@zytor.com>,
linux-arch@vger.kernel.org, kvm@vger.kernel.org,
Raghavendra K T <raghavendra.kt@linux.vnet.ibm.com>,
Ingo Molnar <mingo@redhat.com>,
xen-devel@lists.xenproject.org,
"Paul E. McKenney" <paulmck@linux.vnet.ibm.com>,
Rik van Riel <riel@redhat.com>,
Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>,
Scott J Norton <scott.norton@hp.com>,
Paolo Bonzini <paolo.bonzini@gmail.com>,
Oleg Nesterov <oleg@redhat.com>,
Thomas Gleixner <tglx@linutronix.de>,
virtualization@lists.linux-foundation.org,
Chegu Vinod <chegu_vinod@hp.com>,
linux-kernel@vger.kernel.org,
David Vrabel <david.vrabel@citrix.com>,
Linus Torvalds <torvalds@linux-foundation.org>
Subject: Re: [PATCH v8 01/10] qspinlock: A generic 4-byte queue spinlock implementation
Date: Mon, 7 Apr 2014 16:09:53 +0200 [thread overview]
Message-ID: <20140407140953.GK10526@twins.programming.kicks-ass.net> (raw)
In-Reply-To: <533EE700.4080408@hp.com>
On Fri, Apr 04, 2014 at 01:08:16PM -0400, Waiman Long wrote:
> Peter's patch is a rewrite of my patches 1-4, there is no PV or unfair lock
> support in there.
Yes, because your patches were unreadable and entirely non obvious.
And while I appreciate that its not entirely your fault; the subject is
hard, you didn't even try to make it better and explain things in a
normal gradual fashion.
So what I did was start with a 'simple' correct implementation (although
I could have started simpler still I suppose and added 3-4 more patches)
and then added each optimization on top and explained the what and why
for them.
The result is similar code, but the path is ever so much easier to
understand and review.
And no, it doesn't have PV support; I spend the whole week trying to
reverse engineer your patch 1; whereas if you'd presented it in the form
I posted I might have agreed in a day or so.
I still have to look at the PV bits; but seeing how you have shown no
interest in writing coherent and understandable patch sets I'm less
inclined to go stare at them for another few weeks and rewrite that code
too.
Also; theres a few subtle but important differences between the patch
sets. Your series only makes x86_64 use the qspinlocks; the very last we
want is i386 and x86_64 to use different spinlock implementations; we
want less differences between them, not more.
You stuff a whole lot of code into arch/x86 for no reason what so ever.
Prior to that; I also rewrote your benchmark thing; using jiffies for
timing is just vile. Not to mention you require a full kernel build and
reboot (which is just stupid slow on big machines) to test anything.
next prev parent reply other threads:[~2014-04-07 14:09 UTC|newest]
Thread overview: 41+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-04-02 13:27 [PATCH v8 00/10] qspinlock: a 4-byte queue spinlock with PV support Waiman Long
2014-04-02 13:27 ` [PATCH v8 01/10] qspinlock: A generic 4-byte queue spinlock implementation Waiman Long
2014-04-04 13:00 ` Peter Zijlstra
2014-04-04 14:59 ` Waiman Long
2014-04-04 17:53 ` Ingo Molnar
2014-04-07 14:16 ` Peter Zijlstra
2014-04-04 16:57 ` Konrad Rzeszutek Wilk
2014-04-04 17:08 ` Waiman Long
2014-04-04 17:54 ` Ingo Molnar
2014-04-07 14:09 ` Peter Zijlstra [this message]
2014-04-07 16:59 ` Waiman Long
2014-04-07 14:12 ` Peter Zijlstra
2014-04-07 14:33 ` Konrad Rzeszutek Wilk
2014-04-02 13:27 ` [PATCH v8 02/10] qspinlock, x86: Enable x86-64 to use queue spinlock Waiman Long
2014-04-02 13:27 ` [PATCH v8 03/10] qspinlock: More optimized code for smaller NR_CPUS Waiman Long
2014-04-02 13:27 ` [PATCH v8 04/10] qspinlock: Optimized code path for 2 contending tasks Waiman Long
2014-04-02 13:27 ` [PATCH v8 05/10] pvqspinlock, x86: Allow unfair spinlock in a PV guest Waiman Long
2014-04-02 13:27 ` [PATCH v8 06/10] pvqspinlock: Enable lock stealing in queue lock waiters Waiman Long
2014-04-02 13:27 ` [PATCH v8 07/10] pvqspinlock, x86: Rename paravirt_ticketlocks_enabled Waiman Long
2014-04-02 13:27 ` [PATCH v8 08/10] pvqspinlock, x86: Add qspinlock para-virtualization support Waiman Long
2014-04-02 13:27 ` [PATCH v8 09/10] pvqspinlock, x86: Enable qspinlock PV support for KVM Waiman Long
2014-04-02 13:27 ` [PATCH v8 10/10] pvqspinlock, x86: Enable qspinlock PV support for XEN Waiman Long
2014-04-02 14:39 ` Konrad Rzeszutek Wilk
2014-04-02 20:38 ` Waiman Long
2014-04-02 14:32 ` [PATCH v8 00/10] qspinlock: a 4-byte queue spinlock with PV support Konrad Rzeszutek Wilk
2014-04-02 20:35 ` Waiman Long
2014-04-03 2:10 ` Waiman Long
2014-04-03 17:23 ` Konrad Rzeszutek Wilk
2014-04-04 2:57 ` Waiman Long
2014-04-04 16:55 ` Konrad Rzeszutek Wilk
2014-04-04 17:13 ` Waiman Long
2014-04-04 17:58 ` Konrad Rzeszutek Wilk
2014-04-04 18:33 ` Konrad Rzeszutek Wilk
2014-04-04 18:14 ` Marcos E. Matsunaga
2014-04-04 15:25 ` Konrad Rzeszutek Wilk
2014-04-07 6:14 ` Raghavendra K T
2014-04-07 16:38 ` Waiman Long
2014-04-07 17:51 ` Raghavendra K T
2014-04-08 19:15 ` Waiman Long
2014-04-09 12:08 ` Raghavendra K T
-- strict thread matches above, loose matches on Subject: below --
2014-04-01 20:47 Waiman Long
2014-04-01 20:47 ` [PATCH v8 01/10] qspinlock: A generic 4-byte queue spinlock implementation 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=20140407140953.GK10526@twins.programming.kicks-ass.net \
--to=peterz@infradead.org \
--cc=Marcos.Matsunaga@oracle.com \
--cc=aswin@hp.com \
--cc=chegu_vinod@hp.com \
--cc=david.vrabel@citrix.com \
--cc=gleb@redhat.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=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=waiman.long@hp.com \
--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).