diff mbox series

[RFC] s390/uaccess: Limit number of retries for cmpxchg_user_key

Message ID 20221117100745.3253896-1-scgl@linux.ibm.com
State New
Headers
Series [RFC] s390/uaccess: Limit number of retries for cmpxchg_user_key |

Commit Message

Janis Schoetterl-Glausch Nov. 17, 2022, 10:07 a.m. UTC 
  cmpxchg_user_key for byte and short values is implemented via a one word
cmpxchg loop. Give up trying to perform the cmpxchg if it fails too
often because of contention on the cache line. This ensures that the
thread cannot become stuck in the kernel.

Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com>
---


128 might seem like a small number, but it actually seems to be plenty.
I could not get it to return EAGAIN with MAX_LOOP being 8 while 248
vcpus/threads are hammering the same word.
This could mean that we don't actually need to limit the number of
retries, but then, I didn't simulate the absolute worst case, where
the competing threads are running on dedicated cpus.


 arch/s390/include/asm/uaccess.h | 35 +++++++++++++++++++++++----------
 1 file changed, 25 insertions(+), 10 deletions(-)


base-commit: b23ddf9d5a30f64a1a51a85f0d9e2553210b21a2
prerequisite-patch-id: c5cdc3ce7cdffc18c5e56abfb657c84141fb623a

Comments

Heiko Carstens Nov. 17, 2022, 6:20 p.m. UTC | #1 
On Thu, Nov 17, 2022 at 11:07:45AM +0100, Janis Schoetterl-Glausch wrote:
> cmpxchg_user_key for byte and short values is implemented via a one word
> cmpxchg loop. Give up trying to perform the cmpxchg if it fails too
> often because of contention on the cache line. This ensures that the
> thread cannot become stuck in the kernel.
> 
> Signed-off-by: Janis Schoetterl-Glausch <scgl@linux.ibm.com>
> ---
> 
> 
> 128 might seem like a small number, but it actually seems to be plenty.
> I could not get it to return EAGAIN with MAX_LOOP being 8 while 248
> vcpus/threads are hammering the same word.
> This could mean that we don't actually need to limit the number of
> retries, but then, I didn't simulate the absolute worst case, where
> the competing threads are running on dedicated cpus.
> 
> 
>  arch/s390/include/asm/uaccess.h | 35 +++++++++++++++++++++++----------
>  1 file changed, 25 insertions(+), 10 deletions(-)

Looks good, also applied to wip/cmpxchg_user_branch.

Thanks!
diff mbox series

Patch

diff --git a/arch/s390/include/asm/uaccess.h b/arch/s390/include/asm/uaccess.h
index d028ee59e941..f2d3a4e27963 100644
--- a/arch/s390/include/asm/uaccess.h
+++ b/arch/s390/include/asm/uaccess.h
@@ -392,6 +392,8 @@  do {									\
 
 void __cmpxchg_user_key_called_with_bad_pointer(void);
 
+#define CMPXCHG_USER_KEY_MAX_LOOPS 128
+
 static __always_inline int __cmpxchg_user_key(unsigned long address, void *uval,
 					      __uint128_t old, __uint128_t new,
 					      unsigned long key, int size)
@@ -400,7 +402,7 @@  static __always_inline int __cmpxchg_user_key(unsigned long address, void *uval,
 
 	switch (size) {
 	case 1: {
-		unsigned int prev, shift, mask, _old, _new;
+		unsigned int prev, shift, mask, _old, _new, count;
 
 		shift = (3 ^ (address & 3)) << 3;
 		address ^= address & 3;
@@ -410,6 +412,7 @@  static __always_inline int __cmpxchg_user_key(unsigned long address, void *uval,
 		asm volatile(
 			"	spka	0(%[key])\n"
 			"	sacf	256\n"
+			"	llill	%[count],%[max_loops]\n"
 			"0:	l	%[prev],%[address]\n"
 			"1:	nr	%[prev],%[mask]\n"
 			"	xilf	%[mask],0xffffffff\n"
@@ -421,7 +424,8 @@  static __always_inline int __cmpxchg_user_key(unsigned long address, void *uval,
 			"	xr	%[tmp],%[prev]\n"
 			"	xr	%[new],%[tmp]\n"
 			"	nr	%[tmp],%[mask]\n"
-			"	jz	2b\n"
+			"	jnz	5f\n"
+			"	brct	%[count],2b\n"
 			"5:	sacf	768\n"
 			"	spka	%[default_key]\n"
 			EX_TABLE_UA_LOAD_REG(0b, 5b, %[rc], %[prev])
@@ -433,15 +437,19 @@  static __always_inline int __cmpxchg_user_key(unsigned long address, void *uval,
 			  [address] "+Q" (*(int *)address),
 			  [tmp] "+&d" (_old),
 			  [new] "+&d" (_new),
-			  [mask] "+&d" (mask)
-			: [key] "a" (key << 4),
-			  [default_key] "J" (PAGE_DEFAULT_KEY)
+			  [mask] "+&d" (mask),
+			  [count] "=a" (count)
+			: [key] "%[count]" (key << 4),
+			  [default_key] "J" (PAGE_DEFAULT_KEY),
+			  [max_loops] "J" (CMPXCHG_USER_KEY_MAX_LOOPS)
 			: "memory", "cc");
 		*(unsigned char *)uval = prev >> shift;
+		if (!count)
+			rc = -EAGAIN;
 		return rc;
 	}
 	case 2: {
-		unsigned int prev, shift, mask, _old, _new;
+		unsigned int prev, shift, mask, _old, _new, count;
 
 		shift = (2 ^ (address & 2)) << 3;
 		address ^= address & 2;
@@ -451,6 +459,7 @@  static __always_inline int __cmpxchg_user_key(unsigned long address, void *uval,
 		asm volatile(
 			"	spka	0(%[key])\n"
 			"	sacf	256\n"
+			"	llill	%[count],%[max_loops]\n"
 			"0:	l	%[prev],%[address]\n"
 			"1:	nr	%[prev],%[mask]\n"
 			"	xilf	%[mask],0xffffffff\n"
@@ -462,7 +471,8 @@  static __always_inline int __cmpxchg_user_key(unsigned long address, void *uval,
 			"	xr	%[tmp],%[prev]\n"
 			"	xr	%[new],%[tmp]\n"
 			"	nr	%[tmp],%[mask]\n"
-			"	jz	2b\n"
+			"	jnz	5f\n"
+			"	brct	%[count],2b\n"
 			"5:	sacf	768\n"
 			"	spka	%[default_key]\n"
 			EX_TABLE_UA_LOAD_REG(0b, 5b, %[rc], %[prev])
@@ -474,11 +484,15 @@  static __always_inline int __cmpxchg_user_key(unsigned long address, void *uval,
 			  [address] "+Q" (*(int *)address),
 			  [tmp] "+&d" (_old),
 			  [new] "+&d" (_new),
-			  [mask] "+&d" (mask)
-			: [key] "a" (key << 4),
-			  [default_key] "J" (PAGE_DEFAULT_KEY)
+			  [mask] "+&d" (mask),
+			  [count] "=a" (count)
+			: [key] "%[count]" (key << 4),
+			  [default_key] "J" (PAGE_DEFAULT_KEY),
+			  [max_loops] "J" (CMPXCHG_USER_KEY_MAX_LOOPS)
 			: "memory", "cc");
 		*(unsigned short *)uval = prev >> shift;
+		if (!count)
+			rc = -EAGAIN;
 		return rc;
 	}
 	case 4:	{
@@ -568,6 +582,7 @@  static __always_inline int __cmpxchg_user_key(unsigned long address, void *uval,
  *
  * Return:     0: cmpxchg executed
  *	       -EFAULT: an exception happened when trying to access *@ptr
+ *             -EAGAIN: maxed out number of retries (byte and short only)
  */
 #define cmpxchg_user_key(ptr, uval, old, new, key)			\
 ({									\