This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
Home Products Support Community News


[Xen-devel] Re: [PATCH RFC 02/12] x86/ticketlock: convert spin loop to C

To: "H. Peter Anvin" <hpa@xxxxxxxxx>
Subject: [Xen-devel] Re: [PATCH RFC 02/12] x86/ticketlock: convert spin loop to C
From: Jeremy Fitzhardinge <jeremy@xxxxxxxx>
Date: Fri, 06 Aug 2010 15:03:57 -0700
Cc: Nick Piggin <npiggin@xxxxxxx>, Xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxx>, Peter Zijlstra <peterz@xxxxxxxxxxxxx>, Linux Kernel Mailing List <linux-kernel@xxxxxxxxxxxxxxx>, Jan Beulich <JBeulich@xxxxxxxxxx>, Avi Kivity <avi@xxxxxxxxxx>
Delivery-date: Fri, 06 Aug 2010 15:32:16 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <4C5C7A0A.9080107@xxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <cover.1279328276.git.jeremy.fitzhardinge@xxxxxxxxxx> <cf01e01095f802dc5082e82a8e96c84903e929a6.1279328276.git.jeremy.fitzhardinge@xxxxxxxxxx> <1280761639.1923.213.camel@laptop> <4C56E1A1.6020005@xxxxxxxx> <4C5C1F80020000780000EA6D@xxxxxxxxxxxxxxxxxx> <4C5C21E3.1020004@xxxxxxxx> <4C5C6DCE.4020206@xxxxxxxxx> <4C5C71A5.3050500@xxxxxxxx> <4C5C7A0A.9080107@xxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv: Gecko/20100720 Fedora/3.1.1-1.fc13 Lightning/1.0b2pre Thunderbird/3.1.1
 On 08/06/2010 02:09 PM, H. Peter Anvin wrote:
On 08/06/2010 01:33 PM, Jeremy Fitzhardinge wrote:
On 08/06/2010 01:17 PM, H. Peter Anvin wrote:
On 08/06/2010 07:53 AM, Jeremy Fitzhardinge wrote:
On 08/06/2010 05:43 AM, Jan Beulich wrote:
You certainly mean "the compiler currently treats this as being:" - I
don't think there's a guarantee it'll always be doing so.

for (;;) {
if (inc.tickets.head == inc.tickets.tail)
goto out;
out: barrier();

(Which would probably be a reasonable way to clarify the code.)
I therefore think it needs to be written this way.


A call/return to an actual out-of-line function is a barrier (and will
always be a barrier, as it is the fundamental ABI sequence points),
but to an inline function it is not.

Yes. So the goto explicitly puts the barrier into the control flow which
should stop the compiler from doing anything unexpected.

In this particular case, though, I would somewhat expect the more conventional:

while (inc.tickets.head != inc.tickets.tail) {
    inc.tickets.head = ACCESS_ONCE(lock->tickets_head);

Yes, that makes sense for the plain spinlock version. But the full code, including the pv-ticketlock spin timeout, ends up being:

static __always_inline void arch_spin_lock(struct arch_spinlock *lock)
        register struct __raw_tickets inc;

        inc = __ticket_spin_claim(lock);

        for (;;) {
                unsigned count = SPIN_THRESHOLD;

                do {
                        if (inc.head == inc.tail)
                                goto out;
                        inc.head = ACCESS_ONCE(lock->tickets.head);
                } while (--count);
                __ticket_lock_spinning(lock, inc.tail);
out:    barrier();              /* make sure nothing creeps before the lock is 
taken */

So the goto form is closer to the final form. If it weren't for this, I'd also prefer the while() form.

(If you config PARAVIRT_SPINLOCKS=n, then __ticket_lock_spinning() becomes an empty inline, which causes gcc to collapse the whole thing into a simple infinite loop (ie, it eliminates "count" and the inner loop).)


Xen-devel mailing list