Align the scratch allocation to 64 bytes

PATCH New Chao-ying Fu 4 participants 10 messages Jan 10, 2025 – Jan 24, 2025

Share & tools

Chao-ying Fu

author

2025-01-10 00:34 UTC #0 lore raw

PATCH 1/1 Align the scratch allocation to 64 bytes

We increase the scratch allocation alignment to 64 bytes as the cache line size, to avoid two atomic variables from the same cache line that may cause livelock on some platforms.

---
 lib/sbi/sbi_scratch.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

lib/sbi/sbi_scratch.c +9 -2

index ccbbc68..6b161bc 100644
@@ -14,6 +14,9 @@
 #include <sbi/sbi_scratch.h>
 #include <sbi/sbi_string.h>
 
+/* Minimum size and alignment of scratch allocations */
+#define SCRATCH_ALLOC_ALIGN 64
+
 u32 last_hartindex_having_scratch = 0;
 u32 hartindex_to_hartid_table[SBI_HARTMASK_MAX_BITS + 1] = { -1U };
 struct sbi_scratch *hartindex_to_scratch_table[SBI_HARTMASK_MAX_BITS + 1] = { 0 };

@@ -70,8 +73,12 @@ unsigned long sbi_scratch_alloc_offset(unsigned long size)
 	if (!size)
 		return 0;
 
-	size += __SIZEOF_POINTER__ - 1;
-	size &= ~((unsigned long)__SIZEOF_POINTER__ - 1);
+	/*
+	 * We let the allocation align to 64 bytes to avoid livelock on
+	 * certain platforms due to atomic variables from the same cache line.
+	 */
+	size += SCRATCH_ALLOC_ALIGN - 1;
+	size &= ~((unsigned long)SCRATCH_ALLOC_ALIGN - 1);
 
 	spin_lock(&extra_lock);

Signature · 1 line

2.47.1

Xiang W

2025-01-10 04:15 UTC #1 lore raw

PATCH 1/1 Align the scratch allocation to 64 bytes

? 2025-01-09?? 16:34 -0800?Chao-ying Fu???

Quoted 15 lines

We increase the scratch allocation alignment to 64 bytes as the cache line size, to avoid two atomic variables from the same cache line that may cause livelock on some platforms.
---
?lib/sbi/sbi_scratch.c | 11 +++++++++--
?1 file changed, 9 insertions(+), 2 deletions(-)
lib/sbi/sbi_scratch.c +0 -0
index ccbbc68..6b161bc 100644
@@ -14,6 +14,9 @@
?#include <sbi/sbi_scratch.h>
?#include <sbi/sbi_string.h>
?

lib/sbi/sbi_scratch.c line 18

Quoted 2 lines

lib/sbi/sbi_scratch.c +2 -0

+/* Minimum size and alignment of scratch allocations */
+#define SCRATCH_ALLOC_ALIGN 64

This appears to be a platform-specific need and should not affect all platforms. Can be like this:

#ifndef SCRATCH_ALLOC_ALIGN
#define SCRATCH_ALLOC_ALIGN __SIZEOF_POINTER__
#endif

Regards,
Xiang W

Quoted 23 lines

+
?u32 last_hartindex_having_scratch = 0;
?u32 hartindex_to_hartid_table[SBI_HARTMASK_MAX_BITS + 1] = { -1U }; ?struct sbi_scratch *hartindex_to_scratch_table[SBI_HARTMASK_MAX_BITS + 1] = { 0 };
lib/sbi/sbi_scratch.c +0 -0
@@ -70,8 +73,12 @@ unsigned long sbi_scratch_alloc_offset(unsigned long size)
 ?	if (!size)
 ?		return 0;
?
lib/sbi/sbi_scratch.c +6 -2
-	size += __SIZEOF_POINTER__ - 1;
-	size &= ~((unsigned long)__SIZEOF_POINTER__ - 1);
+	/*
+	 * We let the allocation align to 64 bytes to avoid livelock on
+	 * certain platforms due to atomic variables from the same cache line.
+	 */
+	size += SCRATCH_ALLOC_ALIGN - 1;
+	size &= ~((unsigned long)SCRATCH_ALLOC_ALIGN - 1);
?
? spin_lock(&extra_lock);
?
Signature · 1 line
2.47.1

Chao-ying Fu

author

2025-01-10 02:36 UTC #2 lore raw

PATCH 1/1 Align the scratch allocation to 64 bytes

Quoted 22 lines · On Thu, Jan 9, 2025 at 8:15?PM Xiang W <[email protected]> wrote:

? 2025-01-09?? 16:34 -0800?Chao-ying Fu???
Quoted 12 lines
We increase the scratch allocation alignment to 64 bytes as the cache line size, to avoid two atomic variables from the same cache line that may cause livelock on some platforms.
---
 lib/sbi/sbi_scratch.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)
lib/sbi/sbi_scratch.c +0 -0
index ccbbc68..6b161bc 100644
lib/sbi/sbi_scratch.c line 18
Quoted 5 lines
lib/sbi/sbi_scratch.c +2 -0
@@ -14,6 +14,9 @@
 #include <sbi/sbi_scratch.h>
 #include <sbi/sbi_string.h>
 
+/* Minimum size and alignment of scratch allocations */
+#define SCRATCH_ALLOC_ALIGN 64
This appears to be a platform-specific need and should not affect all platforms. Can be like this:

Quoted 6 lines

#ifndef SCRATCH_ALLOC_ALIGN
#define SCRATCH_ALLOC_ALIGN __SIZEOF_POINTER__
#endif
Regards,
Xiang W

Yes, I will revise the patch to use the original alignment, if SCRATCH_ALLOC_ALIGN is not defined.
Thanks a lot!

Regards,
Chao-ying

Raj Vishwanathan

2025-01-19 20:04 UTC #3 lore raw

PATCH v2 1/1 We add a new configuration CONFIG_SBI_SCRATCH_ALLOC_ALIGNMENT as an int

In reply to Chao-ying Fu

If it is set to 0 or not defined, we will continue with the previous defintion of allocating pointer size chunks. Otherwise we will use the chacheline size of 64.
We have the option of increasing the scratch allocation alignment to 64 bytes as the cache line size, to avoid two atomic variables from
the same cache line that may cause livelock on some platforms.

---
 lib/sbi/Kconfig       |  8 ++++++++
 lib/sbi/sbi_scratch.c | 18 ++++++++++++++++--
 2 files changed, 24 insertions(+), 2 deletions(-)

lib/sbi/Kconfig +8 -0

index c6cc04b..1ffa170 100644
@@ -69,4 +69,12 @@ config SBI_ECALL_SSE
 config SBI_ECALL_MPXY
 	bool "MPXY extension"
 	default y
+config SBI_SCRATCH_ALLOC_ALIGNMENT
+	int  "Scratch allocation alignment"
+	default 0
+	help
+	  We provide the option for allocation alignment to 64 bytes to avoid 
+	  livelock on certain platforms due to atomic variables from the same 
+	  cache line.  Leave it 0 for default behaviour.
+
 endmenu

lib/sbi/sbi_scratch.c +16 -2

index ccbbc68..88ea3c7 100644
@@ -14,6 +14,13 @@
 #include <sbi/sbi_scratch.h>
 #include <sbi/sbi_string.h>
 
+#if !defined(CONFIG_SBI_SCRATCH_ALLOC_ALIGNMENT) || (CONFIG_SBI_SCRATCH_ALLOC_ALIGNMENT==0)
+#define SCRATCH_ALLOC_ALIGNMENT __SIZEOF_POINTER__
+#else
+#define SCRATCH_ALLOC_ALIGNMENT CONFIG_SBI_SCRATCH_ALLOC_ALIGNMENT
+#endif
+
+
 u32 last_hartindex_having_scratch = 0;
 u32 hartindex_to_hartid_table[SBI_HARTMASK_MAX_BITS + 1] = { -1U };
 struct sbi_scratch *hartindex_to_scratch_table[SBI_HARTMASK_MAX_BITS + 1] = { 0 };

@@ -70,8 +77,15 @@ unsigned long sbi_scratch_alloc_offset(unsigned long size)
 	if (!size)
 		return 0;
 
-	size += __SIZEOF_POINTER__ - 1;
-	size &= ~((unsigned long)__SIZEOF_POINTER__ - 1);
+	/*
+	 * We let the allocation align to the cache line size, so
+	 * certain platforms due to atomic variables from the same cache line.
+	 * This ensures that the LR/SC variables are in a separate cache line 
+	 * to avoid live lock.
+	 */
+
+	size += SCRATCH_ALLOC_ALIGNMENT- 1;
+	size &= ~((unsigned long)SCRATCH_ALLOC_ALIGNMENT - 1);
 
 	spin_lock(&extra_lock);

Signature · 1 line

2.43.0

Xiang W

2025-01-20 03:58 UTC #4 lore raw

PATCH v2 1/1 We add a new configuration CONFIG_SBI_SCRATCH_ALLOC_ALIGNMENT as an int

? 2025-01-19?? 12:04 -0800?Raj Vishwanathan???

Quoted 19 lines

If it is set to 0 or not defined, we will continue with the previous defintion of allocating pointer size chunks. Otherwise we will use the chacheline size of 64.
We have the option of increasing the scratch allocation alignment to 64 bytes as the cache line size, to avoid two atomic variables from
the same cache line that may cause livelock on some platforms.
---
?lib/sbi/Kconfig?????? |? 8 ++++++++
?lib/sbi/sbi_scratch.c | 18 ++++++++++++++++--
?2 files changed, 24 insertions(+), 2 deletions(-)
lib/sbi/Kconfig +0 -0
index c6cc04b..1ffa170 100644
@@ -69,4 +69,12 @@ config SBI_ECALL_SSE
?config SBI_ECALL_MPXY
? bool "MPXY extension"
? default y

lib/sbi/Kconfig

Quoted 7 lines

lib/sbi/Kconfig +7 -0

+config SBI_SCRATCH_ALLOC_ALIGNMENT
+	int? "Scratch allocation alignment"
+	default 0
+	help
+	? We provide the option for allocation alignment to 64 bytes to avoid 
+	? livelock on certain platforms due to atomic variables from the same 
+	? cache line.? Leave it 0 for default behaviour.

Don't stress 64 bytes specifically

Customise the size of the alignment to allocate from the extra space in sbi_scratch. Leave it 0 for default behaviour

Regards,
Xiang W

Quoted 42 lines

+
?endmenu
lib/sbi/sbi_scratch.c +0 -0
index ccbbc68..88ea3c7 100644
@@ -14,6 +14,13 @@
?#include <sbi/sbi_scratch.h>
?#include <sbi/sbi_string.h>
?
lib/sbi/sbi_scratch.c +7 -0
+#if !defined(CONFIG_SBI_SCRATCH_ALLOC_ALIGNMENT) || (CONFIG_SBI_SCRATCH_ALLOC_ALIGNMENT==0)
+#define SCRATCH_ALLOC_ALIGNMENT __SIZEOF_POINTER__
+#else
+#define SCRATCH_ALLOC_ALIGNMENT CONFIG_SBI_SCRATCH_ALLOC_ALIGNMENT
+#endif
+
+
?u32 last_hartindex_having_scratch = 0;
?u32 hartindex_to_hartid_table[SBI_HARTMASK_MAX_BITS + 1] = { -1U }; ?struct sbi_scratch *hartindex_to_scratch_table[SBI_HARTMASK_MAX_BITS + 1] = { 0 };
lib/sbi/sbi_scratch.c +0 -0
@@ -70,8 +77,15 @@ unsigned long sbi_scratch_alloc_offset(unsigned long size)
 ?	if (!size)
 ?		return 0;
?
lib/sbi/sbi_scratch.c +9 -2
-	size += __SIZEOF_POINTER__ - 1;
-	size &= ~((unsigned long)__SIZEOF_POINTER__ - 1);
+	/*
+	 * We let the allocation align to the cache line size, so
+	 * certain platforms due to atomic variables from the same cache line.
+	 * This ensures that the LR/SC variables are in a separate cache line 
+	 * to avoid live lock.
+	 */
+
+	size += SCRATCH_ALLOC_ALIGNMENT- 1;
+	size &= ~((unsigned long)SCRATCH_ALLOC_ALIGNMENT - 1);
?
? spin_lock(&extra_lock);
?
Signature · 1 line
2.43.0

Raj Vishwanathan

2025-01-20 18:56 UTC #5 lore raw

[EXTERNAL]Re: [PATCH v2] We add a new configuration CONFIG_SBI_SCRATCH_ALLOC_ALIGNMENT as an int

Xiang W.
Sounds good. Will update it the text in Kconfig. The reason for 64 bytes was to align it with the?cacheline size.

Raj
-----Original Message-----
From: Xiang W <[email protected]>
Sent: Sunday, January 19, 2025 7:58 PM
To: Raj Vishwanathan <[email protected]>; opensbi at lists.infradead.org
Cc: Raj Vishwanathan <[email protected]>
Subject: [EXTERNAL]Re: [PATCH v2] We add a new configuration CONFIG_SBI_SCRATCH_ALLOC_ALIGNMENT as an int

[You don't often get email from wxjstz at 126.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]

? 2025-01-19?? 12:04 -0800?Raj Vishwanathan???

Quoted 16 lines

If it is set to 0 or not defined, we will continue with the previous defintion of allocating pointer size chunks. Otherwise we will use the chacheline size of 64.
We have the option of increasing the scratch allocation alignment to 64 bytes as the cache line size, to avoid two atomic variables from the same cache line that may cause livelock on some platforms.
---
lib/sbi/Kconfig |? 8 ++++++++
 lib/sbi/sbi_scratch.c | 18 ++++++++++++++++--
 2 files changed, 24 insertions(+), 2 deletions(-)
lib/sbi/Kconfig index c6cc04b..1ffa170 renamed from lib/sbi/Kconfig +0 -0
100644

lib/sbi/Kconfig config SBI_ECALL_SSE config SBI_ECALL_MPXY · line 77

Quoted 9 lines

lib/sbi/Kconfig +7 -0

@@ -69,4 +69,12 @@ config SBI_ECALL_SSE  config SBI_ECALL_MPXY
      bool "MPXY extension"
      default y
+config SBI_SCRATCH_ALLOC_ALIGNMENT
+     int  "Scratch allocation alignment"
+     default 0
+     help
+       We provide the option for allocation alignment to 64 bytes to avoid
+       livelock on certain platforms due to atomic variables from the same
+       cache line.  Leave it 0 for default behaviour.

Don't stress 64 bytes specifically

Customise the size of the alignment to allocate from the extra space in sbi_scratch. Leave it 0 for default behaviour

Regards,
Xiang W

Quoted 42 lines

lib/sbi/Kconfig +1 -0

+
 endmenu

lib/sbi/sbi_scratch.c index renamed from lib/sbi/sbi_scratch.c +0 -0

ccbbc68..88ea3c7 100644

lib/sbi/sbi_scratch.c +7 -0

@@ -14,6 +14,13 @@
 #include <sbi/sbi_scratch.h>
 #include <sbi/sbi_string.h>
 
+#if !defined(CONFIG_SBI_SCRATCH_ALLOC_ALIGNMENT) || 
+(CONFIG_SBI_SCRATCH_ALLOC_ALIGNMENT==0)
+#define SCRATCH_ALLOC_ALIGNMENT __SIZEOF_POINTER__ #else #define 
+SCRATCH_ALLOC_ALIGNMENT CONFIG_SBI_SCRATCH_ALLOC_ALIGNMENT
+#endif
+
+
 u32 last_hartindex_having_scratch = 0;
 u32 hartindex_to_hartid_table[SBI_HARTMASK_MAX_BITS + 1] = { -1U };

struct sbi_scratch *hartindex_to_scratch_table[SBI_HARTMASK_MAX_BITS +
1] = { 0 }; @@ -70,8 +77,15 @@ unsigned long sbi_scratch_alloc_offset(unsigned long size)

      if (!size)
              return 0;

lib/sbi/sbi_scratch.c +9 -3

-     size += __SIZEOF_POINTER__ - 1;
-     size &= ~((unsigned long)__SIZEOF_POINTER__ - 1);
+     /*
+      * We let the allocation align to the cache line size, so
+      * certain platforms due to atomic variables from the same cache line.
+      * This ensures that the LR/SC variables are in a separate cache line
+      * to avoid live lock.
+      */
+
+     size += SCRATCH_ALLOC_ALIGNMENT- 1;
+     size &= ~((unsigned long)SCRATCH_ALLOC_ALIGNMENT - 1);
 
      spin_lock(&extra_lock);
 
--

2.43.0

Raj Vishwanathan

2025-01-21 01:46 UTC #6 lore raw

PATCH v3 1/1 New configuration CONFIG_SBI_SCRATCH_ALLOC_ALIGNMENT

In reply to Raj Vishwanathan

We add a new configuration CONFIG_SBI_SCRATCH_ALLOC_ALIGNMENT as an int If it is set to 0 or not defined, we will continue with the previous defintion of allocating pointer size chunks. Otherwise we will use the chacheline size of 64.
We have the option of increasing the scratch allocation alignment to 64 bytes as the cache line size, to avoid two atomic variables from
the same cache line that may cause livelock on some platforms.

Update: Agreeing with the reviewer's comment about not stressing 64 bytes.

---
 lib/sbi/Kconfig       |  7 +++++++
 lib/sbi/sbi_scratch.c | 18 ++++++++++++++++--
 2 files changed, 23 insertions(+), 2 deletions(-)

lib/sbi/Kconfig +7 -0

index c6cc04b..5f7eb70 100644
@@ -69,4 +69,11 @@ config SBI_ECALL_SSE
 config SBI_ECALL_MPXY
 	bool "MPXY extension"
 	default y
+config SBI_SCRATCH_ALLOC_ALIGNMENT
+	int  "Scratch allocation alignment"
+	default 0
+	help
+	  We provide the option to customize the alignment to allocate from
+	  the extra space in sbi_scratch. Leave it 0 for default behaviour.
+
 endmenu

lib/sbi/sbi_scratch.c +16 -2

index ccbbc68..88ea3c7 100644
@@ -14,6 +14,13 @@
 #include <sbi/sbi_scratch.h>
 #include <sbi/sbi_string.h>
 
+#if !defined(CONFIG_SBI_SCRATCH_ALLOC_ALIGNMENT) || (CONFIG_SBI_SCRATCH_ALLOC_ALIGNMENT==0)
+#define SCRATCH_ALLOC_ALIGNMENT __SIZEOF_POINTER__
+#else
+#define SCRATCH_ALLOC_ALIGNMENT CONFIG_SBI_SCRATCH_ALLOC_ALIGNMENT
+#endif
+
+
 u32 last_hartindex_having_scratch = 0;
 u32 hartindex_to_hartid_table[SBI_HARTMASK_MAX_BITS + 1] = { -1U };
 struct sbi_scratch *hartindex_to_scratch_table[SBI_HARTMASK_MAX_BITS + 1] = { 0 };

@@ -70,8 +77,15 @@ unsigned long sbi_scratch_alloc_offset(unsigned long size)
 	if (!size)
 		return 0;
 
-	size += __SIZEOF_POINTER__ - 1;
-	size &= ~((unsigned long)__SIZEOF_POINTER__ - 1);
+	/*
+	 * We let the allocation align to the cache line size, so
+	 * certain platforms due to atomic variables from the same cache line.
+	 * This ensures that the LR/SC variables are in a separate cache line 
+	 * to avoid live lock.
+	 */
+
+	size += SCRATCH_ALLOC_ALIGNMENT- 1;
+	size &= ~((unsigned long)SCRATCH_ALLOC_ALIGNMENT - 1);
 
 	spin_lock(&extra_lock);

Signature · 1 line

2.43.0

Xiang W

2025-01-21 03:21 UTC #7 lore raw

PATCH v3 1/1 New configuration CONFIG_SBI_SCRATCH_ALLOC_ALIGNMENT

? 2025-01-20?? 17:46 -0800?Raj Vishwanathan???

Quoted 8 lines

We add a new configuration CONFIG_SBI_SCRATCH_ALLOC_ALIGNMENT as an int If it is set to 0 or not defined, we will continue with the previous defintion of allocating pointer size chunks. Otherwise we will use the chacheline size of 64.
We have the option of increasing the scratch allocation alignment to 64 bytes as the cache line size, to avoid two atomic variables from
the same cache line that may cause livelock on some platforms.

missing your Signed-off-by

Quoted 1 line

Update: Agreeing with the reviewer's comment about not stressing 64 bytes.

The previous line need move after '---'

Quoted 43 lines

---
?lib/sbi/Kconfig?????? |? 7 +++++++ ?lib/sbi/sbi_scratch.c | 18 ++++++++++++++++-- ?2 files changed, 23 insertions(+), 2 deletions(-)
lib/sbi/Kconfig +0 -0
index c6cc04b..5f7eb70 100644
@@ -69,4 +69,11 @@ config SBI_ECALL_SSE
?config SBI_ECALL_MPXY
? bool "MPXY extension"
? default y
lib/sbi/Kconfig +7 -0
+config SBI_SCRATCH_ALLOC_ALIGNMENT
+	int? "Scratch allocation alignment"
+	default 0
+	help
+	? We provide the option to customize the alignment to allocate from
+	? the extra space in sbi_scratch. Leave it 0 for default behaviour.
+
?endmenu
lib/sbi/sbi_scratch.c +0 -0
index ccbbc68..88ea3c7 100644
@@ -14,6 +14,13 @@
?#include <sbi/sbi_scratch.h>
?#include <sbi/sbi_string.h>
?
lib/sbi/sbi_scratch.c +7 -0
+#if !defined(CONFIG_SBI_SCRATCH_ALLOC_ALIGNMENT) || (CONFIG_SBI_SCRATCH_ALLOC_ALIGNMENT==0)
+#define SCRATCH_ALLOC_ALIGNMENT __SIZEOF_POINTER__
+#else
+#define SCRATCH_ALLOC_ALIGNMENT CONFIG_SBI_SCRATCH_ALLOC_ALIGNMENT
+#endif
+
+
?u32 last_hartindex_having_scratch = 0;
?u32 hartindex_to_hartid_table[SBI_HARTMASK_MAX_BITS + 1] = { -1U }; ?struct sbi_scratch *hartindex_to_scratch_table[SBI_HARTMASK_MAX_BITS + 1] = { 0 };
lib/sbi/sbi_scratch.c +0 -0
@@ -70,8 +77,15 @@ unsigned long sbi_scratch_alloc_offset(unsigned long size)
 ?	if (!size)
 ?		return 0;
?

lib/sbi/sbi_scratch.c unsigned long sbi_scratch_alloc_offset(unsigned long size) · line 85

Quoted 8 lines

lib/sbi/sbi_scratch.c +6 -2

-	size += __SIZEOF_POINTER__ - 1;
-	size &= ~((unsigned long)__SIZEOF_POINTER__ - 1);
+	/*
+	 * We let the allocation align to the cache line size, so
+	 * certain platforms due to atomic variables from the same cache line.
+	 * This ensures that the LR/SC variables are in a separate cache line 
+	 * to avoid live lock.
+	 */

These comments are platform-specific. Better change it.

Regards,
Xiang W

Quoted 10 lines

lib/sbi/sbi_scratch.c +3 -0
+
+	size += SCRATCH_ALLOC_ALIGNMENT- 1;
+	size &= ~((unsigned long)SCRATCH_ALLOC_ALIGNMENT - 1);
?
? spin_lock(&extra_lock);
?
Signature · 1 line
2.43.0

Raj Vishwanathan

2025-01-21 22:55 UTC #8 lore raw

PATCH v3 1/1 New configuration CONFIG_SBI_SCRATCH_ALLOC_ALIGNMENT

In reply to Raj Vishwanathan

---
Update: Agreeing with the reviewer's comment about not stressing 64 bytes.

Signed-off-by Raj Vishwanathan <[email protected]>

---
 lib/sbi/Kconfig       |  7 +++++++
 lib/sbi/sbi_scratch.c | 18 ++++++++++++++++--
 2 files changed, 23 insertions(+), 2 deletions(-)

lib/sbi/Kconfig +7 -0

index c6cc04b..5f7eb70 100644
@@ -69,4 +69,11 @@ config SBI_ECALL_SSE
 config SBI_ECALL_MPXY
 	bool "MPXY extension"
 	default y
+config SBI_SCRATCH_ALLOC_ALIGNMENT
+	int  "Scratch allocation alignment"
+	default 0
+	help
+	  We provide the option to customize the alignment to allocate from
+	  the extra space in sbi_scratch. Leave it 0 for default behaviour.
+
 endmenu

lib/sbi/sbi_scratch.c +16 -2

index ccbbc68..3fd7b4b 100644
@@ -14,6 +14,13 @@
 #include <sbi/sbi_scratch.h>
 #include <sbi/sbi_string.h>
 
+#if !defined(CONFIG_SBI_SCRATCH_ALLOC_ALIGNMENT) || (CONFIG_SBI_SCRATCH_ALLOC_ALIGNMENT==0)
+#define SCRATCH_ALLOC_ALIGNMENT __SIZEOF_POINTER__
+#else
+#define SCRATCH_ALLOC_ALIGNMENT CONFIG_SBI_SCRATCH_ALLOC_ALIGNMENT
+#endif
+
+
 u32 last_hartindex_having_scratch = 0;
 u32 hartindex_to_hartid_table[SBI_HARTMASK_MAX_BITS + 1] = { -1U };
 struct sbi_scratch *hartindex_to_scratch_table[SBI_HARTMASK_MAX_BITS + 1] = { 0 };

@@ -70,8 +77,15 @@ unsigned long sbi_scratch_alloc_offset(unsigned long size)
 	if (!size)
 		return 0;
 
-	size += __SIZEOF_POINTER__ - 1;
-	size &= ~((unsigned long)__SIZEOF_POINTER__ - 1);
+	/*
+	 * We do the scratch allocation on the basis of a user configuration.
+	 * We have the option of increasing the scratch allocation alignment
+	 * to the cache line size (64 bytes) to avoid two atomic variables
+	 * from the same cache line that may cause livelock on some platforms.
+	 */
+
+	size += SCRATCH_ALLOC_ALIGNMENT- 1;
+	size &= ~((unsigned long)SCRATCH_ALLOC_ALIGNMENT - 1);
 
 	spin_lock(&extra_lock);

Signature · 1 line

2.43.0

Raj Vishwanathan

2025-01-24 23:26 UTC #9 lore raw

PATCH v3 1/1 New configuration CONFIG_SBI_SCRATCH_ALLOC_ALIGNMENT

In reply to Raj Vishwanathan

We add a new configuration CONFIG_SBI_SCRATCH_ALLOC_ALIGNMENT as an int If it is set to 0 or not defined, we will continue with the previous defintion of allocating pointer size chunks. Otherwise we will use the user specified size.
We use the scratch allocation alignment to 64 bytes or cacheline size, to avoid two atomic variables from the same cache line causing livelock on some platforms.

Changes in V2:
Remove platform specific references to 64 bytes in the configuration

Changes in V3:
Remove platform specific references in 64 bytes in the comments

Signed-off-by Raj Vishwanathan <[email protected]>

---
 lib/sbi/Kconfig       |  7 +++++++
 lib/sbi/sbi_scratch.c | 14 ++++++++++++--
 2 files changed, 19 insertions(+), 2 deletions(-)

lib/sbi/Kconfig +7 -0

index c6cc04b..5f7eb70 100644
@@ -69,4 +69,11 @@ config SBI_ECALL_SSE
 config SBI_ECALL_MPXY
 	bool "MPXY extension"
 	default y
+config SBI_SCRATCH_ALLOC_ALIGNMENT
+	int  "Scratch allocation alignment"
+	default 0
+	help
+	  We provide the option to customize the alignment to allocate from
+	  the extra space in sbi_scratch. Leave it 0 for default behaviour.
+
 endmenu

lib/sbi/sbi_scratch.c +12 -2

index ccbbc68..0f1be58 100644
@@ -14,6 +14,16 @@
 #include <sbi/sbi_scratch.h>
 #include <sbi/sbi_string.h>
 
+/*
+ * We do the scratch allocation on the basis of a user configuration.
+ */
+#if !defined(CONFIG_SBI_SCRATCH_ALLOC_ALIGNMENT) || (CONFIG_SBI_SCRATCH_ALLOC_ALIGNMENT==0)
+#define SCRATCH_ALLOC_ALIGNMENT __SIZEOF_POINTER__
+#else
+#define SCRATCH_ALLOC_ALIGNMENT CONFIG_SBI_SCRATCH_ALLOC_ALIGNMENT
+#endif
+
+
 u32 last_hartindex_having_scratch = 0;
 u32 hartindex_to_hartid_table[SBI_HARTMASK_MAX_BITS + 1] = { -1U };
 struct sbi_scratch *hartindex_to_scratch_table[SBI_HARTMASK_MAX_BITS + 1] = { 0 };

@@ -70,8 +80,8 @@ unsigned long sbi_scratch_alloc_offset(unsigned long size)
 	if (!size)
 		return 0;
 
-	size += __SIZEOF_POINTER__ - 1;
-	size &= ~((unsigned long)__SIZEOF_POINTER__ - 1);
+	size += SCRATCH_ALLOC_ALIGNMENT- 1;
+	size &= ~((unsigned long)SCRATCH_ALLOC_ALIGNMENT - 1);
 
 	spin_lock(&extra_lock);

Signature · 1 line

2.43.0