linux-kernel - Re: [tip:core/locking] locking, x86: Slightly shorten __ticket_spin

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]

Message-Id: <4B168E7702000078000230C5@vpn.id2.novell.com>
Date:	Wed, 02 Dec 2009 14:57:43 +0000
From:	"Jan Beulich" <JBeulich@...ell.com>
To:	"Ingo Molnar" <mingo@...e.hu>
Cc:	<a.p.zijlstra@...llo.nl>, "Jeremy Fitzhardinge" <jeremy@...p.org>,
	<tglx@...utronix.de>, <torvalds@...ux-foundation.org>,
	<mingo@...hat.com>, <npiggin@...e.de>,
	<linux-kernel@...r.kernel.org>,
	<linux-tip-commits@...r.kernel.org>, <hpa@...or.com>
Subject: Re: [tip:core/locking] locking, x86: Slightly shorten
	 __ticket_spin_trylock()

>>> Ingo Molnar <mingo@...e.hu> 02.12.09 15:21 >>>
792a99a9 <_raw_spin_lock>:
...
>792a99f3:	89 d8                	mov    %ebx,%eax
>792a99f5:	ff 15 d0 6c f2 79    	call   *0x79f26cd0
>792a99fb:	85 c0                	test   %eax,%eax
...
>792a9a2e:	89 f8                	mov    %edi,%eax
>792a9a30:	ff 15 d0 6c f2 79    	call   *0x79f26cd0
>792a9a36:	85 c0                	test   %eax,%eax

Assuming that these are the calls to __raw_spin_trylock, it is clear that
the generated code isn't what we want: It should be test %al, %al in
both cases.

This isn't a compiler bug, though: ____PVOP_CALL() doesn't properly
deal with rettype being bool. Other than all integer types, casting
to bool isn't just a truncation (or extension), but is an actual
conversion. With the compiler seeing that __eax is declared as
unsigned long, it can optimize the whole operation and avoid the
conversion to bool (and hence test the 32-bit register directly).

While I would think these macros should be fixed accordingly (as it
would be quite easy for others to run into the same trap), for the
case at hand I'm not too certain it would be worthwhile as I believe
the goal is to eliminate at least pv-ops spinlocks anyway due to
their overhead on native (and I have a replacement basically
ready, just that for now it only aims at fully virtualized Xen guests).

In any case, please drop the patch until either paravirt.h gets
fixed (I'll try to find time to look into this), or pv-ops spinlocks gets
eliminated. And I'm sorry for not noticing this before submission.

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/