linux-kernel - Re: [cpuops cmpxchg double V2 1/4] Generic support for this_cpu_cmpxchg

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 for Android: free password hash cracker in your pocket

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

Message-ID: <1da7491b-6e7d-4453-9c6f-0e7767421ec4@email.android.com>
Date:	Fri, 21 Jan 2011 09:13:41 -0800
From:	"H. Peter Anvin" <hpa@...or.com>
To:	Tejun Heo <tj@...nel.org>,
	Mathieu Desnoyers <mathieu.desnoyers@...icios.com>
CC:	Pekka Enberg <penberg@...helsinki.fi>,
	Christoph Lameter <cl@...ux.com>, akpm@...ux-foundation.org,
	linux-kernel@...r.kernel.org, Eric Dumazet <eric.dumazet@...il.com>
Subject: Re: [cpuops cmpxchg double V2 1/4] Generic support for this_cpu_cmpxchg_double

We could do cmpxchg with a structure... the problem with a lon int type is that Cristoph ran into bugs with __int128 on 64 bits.

"Tejun Heo" <tj@...nel.org> wrote:

>Hello,
>
>On Fri, Jan 21, 2011 at 11:54:25AM -0500, Mathieu Desnoyers wrote:
>> "The single large 128 bit scalar does not work. Having to define an
>> additional structure it also a bit clumsy. I think its best to get
>> another patchset out that also duplicates the first parameter and
>> makes the percpu variable specs conform to the other this_cpu ops."
>>
>> I'm again probably missing something, but what is "clumsy" about
>> defining a structure like the following to ensure proper alignment
>> of the target pointer (instead of adding a runtime test) ?
>> 
>> struct cmpxchg_double {
>> #if __BYTE_ORDER == __LITTLE_ENDIAN
>>         unsigned long low, high;
>> #else
>>         unsigned long high, low;
>> #endif
>> } __attribute__((packed, aligned(2 * sizeof(unsigned long))));
>>
>> (note: packed here along with "aligned" does _not_ generate ugly
>> bytewise read/write memory ops like "packed" alone. The use of
>> "packed" is to let the compiler down-align the structure to the
>> value requested, instead of uselessly aligning it on 32-byte if it
>> chooses to.)
>
>Yeah, good point.  :-)
>
>> The prototype could then look like:
>> 
>> bool __this_cpu_generic_cmpxchg_double(pcp, oval_low, oval_high,
>nval_low, nval_high);
>> 
>> With:
>>   struct cmpxchg_double *pcp
>> 
>> I think Christoph's point is that he wants to alias this with a
>pointer. Well,
>> this can be done cleanly with:
>> 
>> union {
>>         struct cmpxchg_double casdbl;
>>         struct {
>>                 void *ptr;
>>                 unsigned long cpuid_tid;
>>         } t;
>> }
>> 
>> So by keeping distinct variables for the oval/nal arguments, we let
>> the compiler use registers (instead of the mandatory stack use that
>> would be required if we pass union or structures as oval/nval
>> arguments), but we ensure proper alignment (and drop the unneeded
>> second pointer, as well as the runtime pointer alignment checks) by
>> passing one single pcp pointer of a fixed type with a known
>> alignment.
>
>Actually, we might be able to work around the stack usage regardless
>of the passed type.  We should be able to decompose the structure or
>ulonglong using macro and then pass the decomposed parameters to a
>function if necessary.
>
>Hmm, if we do ulonglong, we don't even need a separate interface and
>can simply use cmpxchg().  If someone can come up with something which
>uses ulonglong that way, it would be great; then, we wouldn't need to
>worry about alignment either.  Am I missing something?
>
>Thanks.
>
>-- 
>tejun

-- 
Sent from my mobile phone.  Please pardon any lack of formatting.
--
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/