linux-kernel - Re: [PATCH 1/4, v2] x86: enlightenment for ticket spin locks

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 30 Jun 2010 10:00:29 +0100
From:	"Jan Beulich" <JBeulich@...ell.com>
To:	"Peter Zijlstra" <peterz@...radead.org>
Cc:	<jeremy.fitzhardinge@...rix.com>, <mingo@...e.hu>,
	<tglx@...utronix.de>, "Ky Srinivasan" <KSrinivasan@...ell.com>,
	<linux-kernel@...r.kernel.org>, <hpa@...or.com>
Subject: Re: [PATCH 1/4, v2] x86: enlightenment for ticket spin locks -
	 base implementation

>>> On 30.06.10 at 10:05, Peter Zijlstra <peterz@...radead.org> wrote:
> On Tue, 2010-06-29 at 15:31 +0100, Jan Beulich wrote:
>> Add optional (alternative instructions based) callout hooks to the
>> contended ticket lock and the ticket unlock paths, to allow hypervisor
>> specific code to be used for reducing/eliminating the bad effects
>> ticket locks have on performance when running virtualized.
> 
> Uhm, I'd much rather see a single alternative implementation, not a
> per-hypervisor lock implementation.

How would you imaging this to work? I can't see how the mechanism
could be hypervisor agnostic. Just look at the Xen implementation
(patch 2) - do you really see room for meaningful abstraction there?
Not the least that not every hypervisor may even have a way to
poll for events (like Xen does), in which case a simple yield may be
needed instead.

>> For the moment, this isn't intended to be used together with pv-ops,
>> but this is just to simplify initial integration. The ultimate goal
>> for this should still be to replace pv-ops spinlocks.
> 
> So why not start by removing that?

Because I wouldn't get around to test it within the time constraints
I have?

>> +config ENLIGHTEN_SPINLOCKS
> 
> Why exactly are these enlightened? I'd say CONFIG_UNFAIR_SPINLOCKS would
> be much better.

The naming certainly isn't significant to me. If consensus can be
reached on any one name, I'll be fine with changing it. I just don't
want to play ping pong here.

>> +#define X86_FEATURE_SPINLOCK_YIELD (3*32+31) /* hypervisor yield interface 
> */
> 
> That name also sucks chunks, yield isn't a lock related term.

Not sure what's wrong with the name (the behavior *is* a yield of
some sort to the underlying scheduler). But again, any name
acceptable to all relevant parties will be fine with me.

>> +#define ALTERNATIVE_TICKET_LOCK \
> 
> But but but, the alternative isn't a ticket lock..!?

??? Of course it is. Or do you mean the macro doesn't
represent the full lock operation? My reading of the name is that
this is the common alternative instruction sequence used in a lock
operation. And just as above - I don't care much about the actual
name, and I'll change any or all of them as long as I'm not going to
be asked to change them back and forth.

Jan

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/