Re: [cpuops cmpxchg double V2 1/4] Generic support for this_cpu_cmpxchg_double

[Tejun Heo <tj@kernel.org>]

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