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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <20110121170847.GH2832@htj.dyndns.org>
Date:	Fri, 21 Jan 2011 18:08:47 +0100
From:	Tejun Heo <tj@...nel.org>
To:	Mathieu Desnoyers <mathieu.desnoyers@...icios.com>
Cc:	"H. Peter Anvin" <hpa@...or.com>,
	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

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
--
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/