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: <C2D7FE5348E1B147BCA15975FBA23075665B0A44@IN01WEMBXB.internal.synopsys.com>
Date:	Mon, 3 Aug 2015 13:50:01 +0000
From:	Vineet Gupta <Vineet.Gupta1@...opsys.com>
To:	Peter Zijlstra <peterz@...radead.org>
CC:	lkml <linux-kernel@...r.kernel.org>,
	"arc-linux-dev@...opsys.com" <arc-linux-dev@...opsys.com>
Subject: Re: [PATCH 5/6] ARCv2: spinlock/rwlock/atomics: Delayed retry of
 failed SCOND with exponential backoff

On Monday 03 August 2015 06:31 PM, Vineet Gupta wrote:
> On Monday 03 August 2015 05:11 PM, Peter Zijlstra wrote:
>> > On Mon, Aug 03, 2015 at 03:33:07PM +0530, Vineet Gupta wrote:
>>> >> +#define SCOND_FAIL_RETRY_VAR_DEF						\
>>> >> +	unsigned int delay = 1, tmp;						\
>>> >> +
>>> >> +#define SCOND_FAIL_RETRY_ASM							\
>>> >> +	"	bz	4f			\n"				\
>>> >> +	"   ; --- scond fail delay ---		\n"				\
>>> >> +	"	mov	%[tmp], %[delay]	\n"	/* tmp = delay */	\
>>> >> +	"2: 	brne.d	%[tmp], 0, 2b		\n"	/* while (tmp != 0) */	\
>>> >> +	"	sub	%[tmp], %[tmp], 1	\n"	/* tmp-- */		\
>>> >> +	"	asl	%[delay], %[delay], 1	\n"	/* delay *= 2 */	\
>>> >> +	"	b	1b			\n"	/* start over */	\
>>> >> +	"4: ; --- success ---			\n"				\
>>> >> +
>>> >> +#define SCOND_FAIL_RETRY_VARS							\
>>> >> +	  ,[delay] "+&r" (delay),[tmp] "=&r"	(tmp)				\
>>> >> +
>>> >> +#define ATOMIC_OP(op, c_op, asm_op)					\
>>> >> +static inline void atomic_##op(int i, atomic_t *v)			\
>>> >> +{									\
>>> >> +	unsigned int val, delay = 1, tmp;				\
>> > Maybe use your SCOND_FAIL_RETRY_VAR_DEF ?
> Right - not sure how I missed that !
>
>> >
>>> >> +									\
>>> >> +	__asm__ __volatile__(						\
>>> >> +	"1:	llock   %[val], [%[ctr]]		\n"		\
>>> >> +	"	" #asm_op " %[val], %[val], %[i]	\n"		\
>>> >> +	"	scond   %[val], [%[ctr]]		\n"		\
>>> >> +	"						\n"		\
>>> >> +	SCOND_FAIL_RETRY_ASM						\
>>> >> +									\
>>> >> +	: [val]	"=&r"	(val) /* Early clobber to prevent reg reuse */	\
>>> >> +	  SCOND_FAIL_RETRY_VARS						\
>>> >> +	: [ctr]	"r"	(&v->counter), /* Not "m": llock only supports reg direct addr mode */	\
>>> >> +	  [i]	"ir"	(i)						\
>>> >> +	: "cc");							\
>>> >> +}									\
>>> >> +
>>> >> +#define ATOMIC_OP_RETURN(op, c_op, asm_op)				\
>>> >> +static inline int atomic_##op##_return(int i, atomic_t *v)		\
>>> >> +{									\
>>> >> +	unsigned int val, delay = 1, tmp;				\
>> > Idem.
> OK !
>
>>> >> +									\
>>> >> +	/*								\
>>> >> +	 * Explicit full memory barrier needed before/after as		\
>>> >> +	 * LLOCK/SCOND thmeselves don't provide any such semantics	\
>>> >> +	 */								\
>>> >> +	smp_mb();							\
>>> >> +									\
>>> >> +	__asm__ __volatile__(						\
>>> >> +	"1:	llock   %[val], [%[ctr]]		\n"		\
>>> >> +	"	" #asm_op " %[val], %[val], %[i]	\n"		\
>>> >> +	"	scond   %[val], [%[ctr]]		\n"		\
>>> >> +	"						\n"		\
>>> >> +	SCOND_FAIL_RETRY_ASM						\
>>> >> +									\
>>> >> +	: [val]	"=&r"	(val)						\
>>> >> +	  SCOND_FAIL_RETRY_VARS						\
>>> >> +	: [ctr]	"r"	(&v->counter),					\
>>> >> +	  [i]	"ir"	(i)						\
>>> >> +	: "cc");							\
>>> >> +									\
>>> >> +	smp_mb();							\
>>> >> +									\
>>> >> +	return val;							\
>>> >> +}
>>> >> +#define SCOND_FAIL_RETRY_VAR_DEF						\
>>> >> +	unsigned int delay, tmp;						\
>>> >> +
>>> >> +#define SCOND_FAIL_RETRY_ASM							\
>>> >> +	"   ; --- scond fail delay ---		\n"				\
>>> >> +	"	mov	%[tmp], %[delay]	\n"	/* tmp = delay */	\
>>> >> +	"2: 	brne.d	%[tmp], 0, 2b		\n"	/* while (tmp != 0) */	\
>>> >> +	"	sub	%[tmp], %[tmp], 1	\n"	/* tmp-- */		\
>>> >> +	"	asl	%[delay], %[delay], 1	\n"	/* delay *= 2 */	\
>>> >> +	"	b	1b			\n"	/* start over */	\
>>> >> +	"					\n"				\
>>> >> +	"4: ; --- done ---			\n"				\
>>> >> +
>>> >> +#define SCOND_FAIL_RETRY_VARS							\
>>> >> +	  ,[delay] "=&r" (delay), [tmp] "=&r"	(tmp)				\
>> > This is looking remarkably similar to the previous ones, why not a
>> > shared header?

On second thoughts, the duplication of atomic generator macros seems to be
superflous ...

Below is much more readable and shorter.

------------>
diff --git a/arch/arc/include/asm/atomic.h b/arch/arc/include/asm/atomic.h
index 3dd36c1efee1..c2e012ca4560 100644
--- a/arch/arc/include/asm/atomic.h
+++ b/arch/arc/include/asm/atomic.h
@@ -23,17 +23,50 @@
 
 #define atomic_set(v, i) (((v)->counter) = (i))
 
+#ifdef CONFIG_ARC_STAR_9000923308
+
+#define SCOND_FAIL_RETRY_VAR_DEF                        \
+    unsigned int delay = 1, tmp;                        \
+
+#define SCOND_FAIL_RETRY_ASM                            \
+    "    bz    4f            \n"                \
+    "   ; --- scond fail delay ---        \n"                \
+    "    mov    %[tmp], %[delay]    \n"    /* tmp = delay */    \
+    "2:     brne.d    %[tmp], 0, 2b        \n"    /* while (tmp != 0) */    \
+    "    sub    %[tmp], %[tmp], 1    \n"    /* tmp-- */        \
+    "    asl    %[delay], %[delay], 1    \n"    /* delay *= 2 */    \
+    "    b    1b            \n"    /* start over */    \
+    "4: ; --- success ---            \n"                \
+
+#define SCOND_FAIL_RETRY_VARS                            \
+      ,[delay] "+&r" (delay),[tmp] "=&r"    (tmp)                \
+
+#else    /* !CONFIG_ARC_STAR_9000923308 */
+
+#define SCOND_FAIL_RETRY_VAR_DEF
+
+#define SCOND_FAIL_RETRY_ASM                            \
+    "    bnz     1b            \n"                \
+
+#define SCOND_FAIL_RETRY_VARS
+
+#endif
+
 #define ATOMIC_OP(op, c_op, asm_op)                    \
 static inline void atomic_##op(int i, atomic_t *v)            \
 {                                    \
-    unsigned int val;                        \
+    unsigned int val;                                \
+    SCOND_FAIL_RETRY_VAR_DEF                                        \
                                     \
     __asm__ __volatile__(                        \
     "1:    llock   %[val], [%[ctr]]        \n"        \
     "    " #asm_op " %[val], %[val], %[i]    \n"        \
     "    scond   %[val], [%[ctr]]        \n"        \
-    "    bnz     1b                \n"        \
+    "                        \n"        \
+    SCOND_FAIL_RETRY_ASM                        \
+                                    \
     : [val]    "=&r"    (val) /* Early clobber to prevent reg reuse */    \
+      SCOND_FAIL_RETRY_VARS                        \
     : [ctr]    "r"    (&v->counter), /* Not "m": llock only supports reg direct
addr mode */    \
       [i]    "ir"    (i)                        \
     : "cc");                            \
@@ -42,7 +75,8 @@ static inline void atomic_##op(int i, atomic_t *v)            \
 #define ATOMIC_OP_RETURN(op, c_op, asm_op)                \
 static inline int atomic_##op##_return(int i, atomic_t *v)        \
 {                                    \
-    unsigned int val;                        \
+    unsigned int val;                                \
+    SCOND_FAIL_RETRY_VAR_DEF                                        \
                                     \
     /*                                \
      * Explicit full memory barrier needed before/after as        \
@@ -54,8 +88,11 @@ static inline int atomic_##op##_return(int i, atomic_t *v)        \
     "1:    llock   %[val], [%[ctr]]        \n"        \
     "    " #asm_op " %[val], %[val], %[i]    \n"        \
     "    scond   %[val], [%[ctr]]        \n"        \
-    "    bnz     1b                \n"        \
+    "                        \n"        \
+    SCOND_FAIL_RETRY_ASM                        \
+                                    \
     : [val]    "=&r"    (val)                        \
+      SCOND_FAIL_RETRY_VARS                        \
     : [ctr]    "r"    (&v->counter),                    \
       [i]    "ir"    (i)                        \
     : "cc");                            \
@@ -142,6 +179,9 @@ ATOMIC_OP(and, &=, and)
 #undef ATOMIC_OPS
 #undef ATOMIC_OP_RETURN
 #undef ATOMIC_OP
+#undef SCOND_FAIL_RETRY_VAR_DEF
+#undef SCOND_FAIL_RETRY_ASM
+#undef SCOND_FAIL_RETRY_VARS
 
 /**
  * __atomic_add_unless - add unless the number is a given value





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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ