[PATCH 0/3] riscv: minor thread_info.cpu improvements

[PATCH 0/3] riscv: minor thread_info.cpu improvements

[Radim Krčmář]

Hello,

I noticed that we give thread_info.cpu two different names in the
assembly, load it as a wider type, and store it suboptimally.

The patches are split for easy review instead of easy backporting:
I could split [2/3] into two patches, and move them before [1/3].
Do we expect someone to actually backport the fixes?

Thanks.

Radim Krčmář (3):
  riscv: use TASK_TI_CPU instead of TASK_TI_CPU_NUM
  riscv: use lw instead of REG_L when reading int cpu
  riscv: pack rv64 thread_info better

 arch/riscv/include/asm/asm.h         | 2 +-
 arch/riscv/include/asm/thread_info.h | 2 +-
 arch/riscv/kernel/asm-offsets.c      | 1 -
 arch/riscv/kernel/entry.S            | 2 +-
 4 files changed, 3 insertions(+), 4 deletions(-)

-- 
2.50.0

[PATCH 1/3] riscv: use TASK_TI_CPU instead of TASK_TI_CPU_NUM

[Radim Krčmář]

The offsets of TASK_TI_CPU and TASK_TI_CPU_NUM are identical, and
TASK_TI_CPU is a better name for thread_info.cpu.

Signed-off-by: Radim Krčmář <rkrcmar@ventanamicro.com>
---
 arch/riscv/include/asm/asm.h    | 2 +-
 arch/riscv/kernel/asm-offsets.c | 1 -
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/riscv/include/asm/asm.h b/arch/riscv/include/asm/asm.h
index a8a2af6dfe9d..b3022bc224ec 100644
--- a/arch/riscv/include/asm/asm.h
+++ b/arch/riscv/include/asm/asm.h
@@ -91,7 +91,7 @@
 #endif
 
 .macro asm_per_cpu dst sym tmp
-	REG_L \tmp, TASK_TI_CPU_NUM(tp)
+	REG_L \tmp, TASK_TI_CPU(tp)
 	slli  \tmp, \tmp, PER_CPU_OFFSET_SHIFT
 	la    \dst, __per_cpu_offset
 	add   \dst, \dst, \tmp
diff --git a/arch/riscv/kernel/asm-offsets.c b/arch/riscv/kernel/asm-offsets.c
index 6e8c0d6feae9..49cf2c347485 100644
--- a/arch/riscv/kernel/asm-offsets.c
+++ b/arch/riscv/kernel/asm-offsets.c
@@ -49,7 +49,6 @@ void asm_offsets(void)
 	OFFSET(TASK_TI_A2, task_struct, thread_info.a2);
 #endif
 
-	OFFSET(TASK_TI_CPU_NUM, task_struct, thread_info.cpu);
 	OFFSET(TASK_THREAD_F0,  task_struct, thread.fstate.f[0]);
 	OFFSET(TASK_THREAD_F1,  task_struct, thread.fstate.f[1]);
 	OFFSET(TASK_THREAD_F2,  task_struct, thread.fstate.f[2]);
-- 
2.50.0

[PATCH 2/3] riscv: use lw instead of REG_L when reading int cpu

[Radim Krčmář]

REG_L is wrong, because thread_info.cpu is 32-bit, not xlen-bit wide.
The struct currently has a hole after cpu, so little endian accesses
seemed fine.

Fixes: be97d0db5f44 ("riscv: VMAP_STACK overflow detection thread-safe")
Fixes: 503638e0babf ("riscv: Stop emitting preventive sfence.vma for new vmalloc mappings")
Signed-off-by: Radim Krčmář <rkrcmar@ventanamicro.com>
---
 arch/riscv/include/asm/asm.h | 2 +-
 arch/riscv/kernel/entry.S    | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/riscv/include/asm/asm.h b/arch/riscv/include/asm/asm.h
index b3022bc224ec..93b1e4ce34d1 100644
--- a/arch/riscv/include/asm/asm.h
+++ b/arch/riscv/include/asm/asm.h
@@ -91,7 +91,7 @@
 #endif
 
 .macro asm_per_cpu dst sym tmp
-	REG_L \tmp, TASK_TI_CPU(tp)
+	lw    \tmp, TASK_TI_CPU(tp)
 	slli  \tmp, \tmp, PER_CPU_OFFSET_SHIFT
 	la    \dst, __per_cpu_offset
 	add   \dst, \dst, \tmp
diff --git a/arch/riscv/kernel/entry.S b/arch/riscv/kernel/entry.S
index 75656afa2d6b..4fdf187a62bf 100644
--- a/arch/riscv/kernel/entry.S
+++ b/arch/riscv/kernel/entry.S
@@ -46,7 +46,7 @@
 	 * a0 = &new_vmalloc[BIT_WORD(cpu)]
 	 * a1 = BIT_MASK(cpu)
 	 */
-	REG_L 	a2, TASK_TI_CPU(tp)
+	lw	a2, TASK_TI_CPU(tp)
 	/*
 	 * Compute the new_vmalloc element position:
 	 * (cpu / 64) * 8 = (cpu >> 6) << 3
-- 
2.50.0

[PATCH 3/3] riscv: pack rv64 thread_info better

[Radim Krčmář]

On rv64, preempt_count and cpu were both 32-bit followed by 64-bit, so
placing one in the hole saves 8 bytes in the struct.

Signed-off-by: Radim Krčmář <rkrcmar@ventanamicro.com>
---
 arch/riscv/include/asm/thread_info.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/riscv/include/asm/thread_info.h b/arch/riscv/include/asm/thread_info.h
index f5916a70879a..c267d6bd838e 100644
--- a/arch/riscv/include/asm/thread_info.h
+++ b/arch/riscv/include/asm/thread_info.h
@@ -53,6 +53,7 @@
 struct thread_info {
 	unsigned long		flags;		/* low level flags */
 	int                     preempt_count;  /* 0=>preemptible, <0=>BUG */
+	int			cpu;
 	/*
 	 * These stack pointers are overwritten on every system call or
 	 * exception.  SP is also saved to the stack it can be recovered when
@@ -60,7 +61,6 @@ struct thread_info {
 	 */
 	long			kernel_sp;	/* Kernel stack pointer */
 	long			user_sp;	/* User stack pointer */
-	int			cpu;
 	unsigned long		syscall_work;	/* SYSCALL_WORK_ flags */
 #ifdef CONFIG_SHADOW_CALL_STACK
 	void			*scs_base;
-- 
2.50.0

Re: [PATCH 1/3] riscv: use TASK_TI_CPU instead of TASK_TI_CPU_NUM

[Alexandre Ghiti]

Hi Radim,

On 7/22/25 18:05, Radim Krčmář wrote:
> The offsets of TASK_TI_CPU and TASK_TI_CPU_NUM are identical, and
> TASK_TI_CPU is a better name for thread_info.cpu.
>
> Signed-off-by: Radim Krčmář <rkrcmar@ventanamicro.com>
> ---
>   arch/riscv/include/asm/asm.h    | 2 +-
>   arch/riscv/kernel/asm-offsets.c | 1 -
>   2 files changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/arch/riscv/include/asm/asm.h b/arch/riscv/include/asm/asm.h
> index a8a2af6dfe9d..b3022bc224ec 100644
> --- a/arch/riscv/include/asm/asm.h
> +++ b/arch/riscv/include/asm/asm.h
> @@ -91,7 +91,7 @@
>   #endif
>   
>   .macro asm_per_cpu dst sym tmp
> -	REG_L \tmp, TASK_TI_CPU_NUM(tp)
> +	REG_L \tmp, TASK_TI_CPU(tp)
>   	slli  \tmp, \tmp, PER_CPU_OFFSET_SHIFT
>   	la    \dst, __per_cpu_offset
>   	add   \dst, \dst, \tmp
> diff --git a/arch/riscv/kernel/asm-offsets.c b/arch/riscv/kernel/asm-offsets.c
> index 6e8c0d6feae9..49cf2c347485 100644
> --- a/arch/riscv/kernel/asm-offsets.c
> +++ b/arch/riscv/kernel/asm-offsets.c
> @@ -49,7 +49,6 @@ void asm_offsets(void)
>   	OFFSET(TASK_TI_A2, task_struct, thread_info.a2);
>   #endif
>   
> -	OFFSET(TASK_TI_CPU_NUM, task_struct, thread_info.cpu);
>   	OFFSET(TASK_THREAD_F0,  task_struct, thread.fstate.f[0]);
>   	OFFSET(TASK_THREAD_F1,  task_struct, thread.fstate.f[1]);
>   	OFFSET(TASK_THREAD_F2,  task_struct, thread.fstate.f[2]);


Reviewed-by: Alexandre Ghiti <alexghiti@rivosinc.com>

Thanks,

Alex

Re: [PATCH 2/3] riscv: use lw instead of REG_L when reading int cpu

[Alexandre Ghiti]

On 7/22/25 18:05, Radim Krčmář wrote:
> REG_L is wrong, because thread_info.cpu is 32-bit, not xlen-bit wide.
> The struct currently has a hole after cpu, so little endian accesses
> seemed fine.
>
> Fixes: be97d0db5f44 ("riscv: VMAP_STACK overflow detection thread-safe")
> Fixes: 503638e0babf ("riscv: Stop emitting preventive sfence.vma for new vmalloc mappings")
> Signed-off-by: Radim Krčmář <rkrcmar@ventanamicro.com>
> ---
>   arch/riscv/include/asm/asm.h | 2 +-
>   arch/riscv/kernel/entry.S    | 2 +-
>   2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/riscv/include/asm/asm.h b/arch/riscv/include/asm/asm.h
> index b3022bc224ec..93b1e4ce34d1 100644
> --- a/arch/riscv/include/asm/asm.h
> +++ b/arch/riscv/include/asm/asm.h
> @@ -91,7 +91,7 @@
>   #endif
>   
>   .macro asm_per_cpu dst sym tmp
> -	REG_L \tmp, TASK_TI_CPU(tp)
> +	lw    \tmp, TASK_TI_CPU(tp)
>   	slli  \tmp, \tmp, PER_CPU_OFFSET_SHIFT
>   	la    \dst, __per_cpu_offset
>   	add   \dst, \dst, \tmp
> diff --git a/arch/riscv/kernel/entry.S b/arch/riscv/kernel/entry.S
> index 75656afa2d6b..4fdf187a62bf 100644
> --- a/arch/riscv/kernel/entry.S
> +++ b/arch/riscv/kernel/entry.S
> @@ -46,7 +46,7 @@
>   	 * a0 = &new_vmalloc[BIT_WORD(cpu)]
>   	 * a1 = BIT_MASK(cpu)
>   	 */
> -	REG_L 	a2, TASK_TI_CPU(tp)
> +	lw	a2, TASK_TI_CPU(tp)
>   	/*
>   	 * Compute the new_vmalloc element position:
>   	 * (cpu / 64) * 8 = (cpu >> 6) << 3


I went across the same kind of bug a week ago, thanks for catching this one.

You can add:

Reviewed-by: Alexandre Ghiti <alexghiti@rivosinc.com>

Though, to get backported to stable kernels, it will be easier if you 
split this patch into 2 independent patches as it will fail to apply on 
older releases. And it should not be dependent on the renaming of 
TASK_TI_CPU_NUM.

Please also add:

Cc: <stable@vger.kernel.org>

So that they will get backported automatically.

Thanks again for the catch,

Alex

Re: [PATCH 3/3] riscv: pack rv64 thread_info better

[Alexandre Ghiti]

On 7/22/25 18:05, Radim Krčmář wrote:
> On rv64, preempt_count and cpu were both 32-bit followed by 64-bit, so
> placing one in the hole saves 8 bytes in the struct.
>
> Signed-off-by: Radim Krčmář <rkrcmar@ventanamicro.com>
> ---
>   arch/riscv/include/asm/thread_info.h | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/riscv/include/asm/thread_info.h b/arch/riscv/include/asm/thread_info.h
> index f5916a70879a..c267d6bd838e 100644
> --- a/arch/riscv/include/asm/thread_info.h
> +++ b/arch/riscv/include/asm/thread_info.h
> @@ -53,6 +53,7 @@
>   struct thread_info {
>   	unsigned long		flags;		/* low level flags */
>   	int                     preempt_count;  /* 0=>preemptible, <0=>BUG */
> +	int			cpu;
>   	/*
>   	 * These stack pointers are overwritten on every system call or
>   	 * exception.  SP is also saved to the stack it can be recovered when
> @@ -60,7 +61,6 @@ struct thread_info {
>   	 */
>   	long			kernel_sp;	/* Kernel stack pointer */
>   	long			user_sp;	/* User stack pointer */
> -	int			cpu;
>   	unsigned long		syscall_work;	/* SYSCALL_WORK_ flags */
>   #ifdef CONFIG_SHADOW_CALL_STACK
>   	void			*scs_base;


Great, that now fits into a cacheline. I guess other structures would 
deserve the same attention.

You can add:

Tested-by: Alexandre Ghiti <alexghiti@rivosinc.com>
Reviewed-by: Alexandre Ghiti <alexghiti@rivosinc.com>

Thanks,

Alex