| 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
| ||
|
Date: Sat, 30 Apr 2016 01:20:31 +0100
From: James Hogan <james.hogan@...tec.com>
To: Peter Zijlstra <peterz@...radead.org>
CC: <torvalds@...ux-foundation.org>, <mingo@...nel.org>,
<tglx@...utronix.de>, <will.deacon@....com>,
<paulmck@...ux.vnet.ibm.com>, <boqun.feng@...il.com>,
<waiman.long@....com>, <fweisbec@...il.com>,
<linux-kernel@...r.kernel.org>, <linux-arch@...r.kernel.org>,
<rth@...ddle.net>, <vgupta@...opsys.com>, <linux@....linux.org.uk>,
<egtvedt@...fundet.no>, <realmz6@...il.com>,
<ysato@...rs.sourceforge.jp>, <rkuo@...eaurora.org>,
<tony.luck@...el.com>, <geert@...ux-m68k.org>,
<ralf@...ux-mips.org>, <dhowells@...hat.com>,
<jejb@...isc-linux.org>, <mpe@...erman.id.au>,
<schwidefsky@...ibm.com>, <dalias@...c.org>, <davem@...emloft.net>,
<cmetcalf@...lanox.com>, <jcmvbkbc@...il.com>, <arnd@...db.de>,
<dbueso@...e.de>, <fengguang.wu@...el.com>
Subject: Re: [RFC][PATCH 14/31] locking,metag: Implement
atomic_fetch_{add,sub,and,or,xor}()
Hi Peter,
On Fri, Apr 22, 2016 at 11:04:27AM +0200, Peter Zijlstra wrote:
> Implement FETCH-OP atomic primitives, these are very similar to the
> existing OP-RETURN primitives we already have, except they return the
> value of the atomic variable _before_ modification.
>
> This is especially useful for irreversible operations -- such as
> bitops (because it becomes impossible to reconstruct the state prior
> to modification).
>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@...radead.org>
> ---
> arch/metag/include/asm/atomic.h | 2 +
> arch/metag/include/asm/atomic_lnkget.h | 36 +++++++++++++++++++++++++++++----
> arch/metag/include/asm/atomic_lock1.h | 33 ++++++++++++++++++++++++++----
> 3 files changed, 63 insertions(+), 8 deletions(-)
>
> --- a/arch/metag/include/asm/atomic.h
> +++ b/arch/metag/include/asm/atomic.h
> @@ -17,6 +17,8 @@
> #include <asm/atomic_lnkget.h>
> #endif
>
> +#define atomic_fetch_or atomic_fetch_or
> +
> #define atomic_add_negative(a, v) (atomic_add_return((a), (v)) < 0)
>
> #define atomic_dec_return(v) atomic_sub_return(1, (v))
> --- a/arch/metag/include/asm/atomic_lnkget.h
> +++ b/arch/metag/include/asm/atomic_lnkget.h
> @@ -69,16 +69,44 @@ static inline int atomic_##op##_return(i
> return result; \
> }
>
> -#define ATOMIC_OPS(op) ATOMIC_OP(op) ATOMIC_OP_RETURN(op)
> +#define ATOMIC_FETCH_OP(op) \
> +static inline int atomic_fetch_##op(int i, atomic_t *v) \
> +{ \
> + int result, temp; \
> + \
> + smp_mb(); \
> + \
> + asm volatile ( \
> + "1: LNKGETD %1, [%2]\n" \
> + " " #op " %0, %1, %3\n" \
i was hoping never to have to think about meta asm constraints again :-P
and/or/xor are only available in the data units, as determined by %1 in
this case, so the constraint for result shouldn't have "a" in it.
diff --git a/arch/metag/include/asm/atomic_lnkget.h b/arch/metag/include/asm/atomic_lnkget.h
index 50ad05050947..def2c642f053 100644
--- a/arch/metag/include/asm/atomic_lnkget.h
+++ b/arch/metag/include/asm/atomic_lnkget.h
@@ -84,7 +84,7 @@ static inline int atomic_fetch_##op(int i, atomic_t *v) \
" ANDT %0, %0, #HI(0x3f000000)\n" \
" CMPT %0, #HI(0x02000000)\n" \
" BNZ 1b\n" \
- : "=&d" (temp), "=&da" (result) \
+ : "=&d" (temp), "=&d" (result) \
: "da" (&v->counter), "bd" (i) \
: "cc"); \
That also ensures the "bd" constraint for %3 (meaning "an op2 register
where op1 [%1 in this case] is a data unit register and the instruction
supports O2R") is consistent.
So with that change this patch looks good to me:
Acked-by: James Hogan <james.hogan@...tec.com>
Note that for the ATOMIC_OP_RETURN() case (add/sub only) either address
or data units can be used (hence the "da" for %1), but then the "bd"
constraint on %3 is wrong as op1 [%1] may not be in data unit (sorry I
didn't spot that at the time). I'll queue a fix, something like below
probably ("br" means "An Op2 register and the instruction supports O2R",
i.e. op1/%1 doesn't have to be a data unit register):
diff --git a/arch/metag/include/asm/atomic_lnkget.h b/arch/metag/include/asm/atomic_lnkget.h
index 50ad05050947..def2c642f053 100644
--- a/arch/metag/include/asm/atomic_lnkget.h
+++ b/arch/metag/include/asm/atomic_lnkget.h
@@ -61,7 +61,7 @@ static inline int atomic_##op##_return(int i, atomic_t *v) \
" CMPT %0, #HI(0x02000000)\n" \
" BNZ 1b\n" \
: "=&d" (temp), "=&da" (result) \
- : "da" (&v->counter), "bd" (i) \
+ : "da" (&v->counter), "br" (i) \
: "cc"); \
\
smp_mb(); \
\ \
Thanks
James
> + " LNKSETD [%2], %0\n" \
> + " DEFR %0, TXSTAT\n" \
> + " ANDT %0, %0, #HI(0x3f000000)\n" \
> + " CMPT %0, #HI(0x02000000)\n" \
> + " BNZ 1b\n" \
> + : "=&d" (temp), "=&da" (result) \
> + : "da" (&v->counter), "bd" (i) \
> + : "cc"); \
> + \
> + smp_mb(); \
> + \
> + return result; \
> +}
> +
> +#define ATOMIC_OPS(op) ATOMIC_OP(op) ATOMIC_OP_RETURN(op) ATOMIC_FETCH_OP(op)
>
> ATOMIC_OPS(add)
> ATOMIC_OPS(sub)
>
> -ATOMIC_OP(and)
> -ATOMIC_OP(or)
> -ATOMIC_OP(xor)
> +#undef ATOMIC_OPS
> +#define ATOMIC_OPS(op) ATOMIC_OP(op) ATOMIC_FETCH_OP(op)
> +
> +ATOMIC_OPS(and)
> +ATOMIC_OPS(or)
> +ATOMIC_OPS(xor)
>
> #undef ATOMIC_OPS
> +#undef ATOMIC_FETCH_OP
> #undef ATOMIC_OP_RETURN
> #undef ATOMIC_OP
>
> --- a/arch/metag/include/asm/atomic_lock1.h
> +++ b/arch/metag/include/asm/atomic_lock1.h
> @@ -64,15 +64,40 @@ static inline int atomic_##op##_return(i
> return result; \
> }
>
> -#define ATOMIC_OPS(op, c_op) ATOMIC_OP(op, c_op) ATOMIC_OP_RETURN(op, c_op)
> +#define ATOMIC_FETCH_OP(op, c_op) \
> +static inline int atomic_fetch_##op(int i, atomic_t *v) \
> +{ \
> + unsigned long result; \
> + unsigned long flags; \
> + \
> + __global_lock1(flags); \
> + result = v->counter; \
> + fence(); \
> + v->counter c_op i; \
> + __global_unlock1(flags); \
> + \
> + return result; \
> +}
> +
> +#define ATOMIC_OPS(op, c_op) \
> + ATOMIC_OP(op, c_op) \
> + ATOMIC_OP_RETURN(op, c_op) \
> + ATOMIC_FETCH_OP(op, c_op)
>
> ATOMIC_OPS(add, +=)
> ATOMIC_OPS(sub, -=)
> -ATOMIC_OP(and, &=)
> -ATOMIC_OP(or, |=)
> -ATOMIC_OP(xor, ^=)
>
> #undef ATOMIC_OPS
> +#define ATOMIC_OPS(op, c_op) \
> + ATOMIC_OP(op, c_op) \
> + ATOMIC_FETCH_OP(op, c_op)
> +
> +ATOMIC_OPS(and, &=)
> +ATOMIC_OPS(or, |=)
> +ATOMIC_OPS(xor, ^=)
> +
> +#undef ATOMIC_OPS
> +#undef ATOMIC_FETCH_OP
> #undef ATOMIC_OP_RETURN
> #undef ATOMIC_OP
>
>
>
Download attachment "signature.asc" of type "application/pgp-signature" (820 bytes)
Powered by blists - more mailing lists