2021-09-30 13:23:29

by Ard Biesheuvel

[permalink] [raw]
Subject: [PATCH v2 0/7] Move task_struct::cpu back into thread_info

Commit c65eacbe290b ("sched/core: Allow putting thread_info into
task_struct") mentions that, along with moving thread_info into
task_struct, the cpu field is moved out of the former into the latter,
as at that point, it was the intention to ultimately get rid of
thread_info entirely, but this never happened.

While collaborating with Keith on adding THREAD_INFO_IN_TASK support to
ARM, we noticed that keeping CPU in task_struct is problematic for
architectures that define raw_smp_processor_id() in terms of this field,
as it requires linux/sched.h to be included, which causes a lot of pain
in terms of circular dependencies (or 'header soup', as the original
commit refers to it).

For examples of how existing architectures work around this, please
refer to patches #6 or #7. In the former case, it uses an awful
asm-offsets hack to index thread_info/current without using its type
definition. The latter approach simply keeps a copy of the task_struct
CPU field in thread_info, and keeps it in sync at context switch time.

We also discussed introducing yet another Kconfig symbol to indicate
that the arch has THREAD_INFO_IN_TASK enabled but still prefers to keep
its CPU field in thread_info, but simply keeping it in thread_info in
all cases seems to be the cleanest approach here, so that is what this
series implements.

Changes since v1/RFC:
- use macro for task_thread_info() to work around constness of
task_cpu()'s task_struct argument
- add various acks
- drop patch #8 for ARM - it was preliminary in the RFC, and it can be
taken as a fix in the ARM tree once THREAD_INFO_IN_TASK actually
lands.

Code can be found here:
https://git.kernel.org/pub/scm/linux/kernel/git/ardb/linux.git/log/?h=move-task-cpu-to-ti-v2

Cc: Keith Packard <[email protected]>
Cc: Russell King <[email protected]>
Cc: Catalin Marinas <[email protected]>
Cc: Will Deacon <[email protected]>
Cc: Michael Ellerman <[email protected]>
Cc: Benjamin Herrenschmidt <[email protected]>
Cc: Christophe Leroy <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Paul Walmsley <[email protected]>
Cc: Palmer Dabbelt <[email protected]>
Cc: Albert Ou <[email protected]>
Cc: Heiko Carstens <[email protected]>

Ard Biesheuvel (7):
arm64: add CPU field to struct thread_info
x86: add CPU field to struct thread_info
s390: add CPU field to struct thread_info
powerpc: add CPU field to struct thread_info
sched: move CPU field back into thread_info if THREAD_INFO_IN_TASK=y
powerpc: smp: remove hack to obtain offset of task_struct::cpu
riscv: rely on core code to keep thread_info::cpu updated

arch/arm64/include/asm/thread_info.h | 1 +
arch/arm64/kernel/asm-offsets.c | 2 +-
arch/arm64/kernel/head.S | 2 +-
arch/powerpc/Makefile | 11 -----------
arch/powerpc/include/asm/smp.h | 17 +----------------
arch/powerpc/include/asm/thread_info.h | 3 +++
arch/powerpc/kernel/asm-offsets.c | 4 +---
arch/powerpc/kernel/smp.c | 2 +-
arch/riscv/kernel/asm-offsets.c | 1 -
arch/riscv/kernel/entry.S | 5 -----
arch/riscv/kernel/head.S | 1 -
arch/s390/include/asm/thread_info.h | 1 +
arch/x86/include/asm/thread_info.h | 3 +++
include/linux/sched.h | 13 +------------
kernel/sched/sched.h | 4 ----
15 files changed, 14 insertions(+), 56 deletions(-)

--
2.30.2


2021-09-30 13:23:48

by Ard Biesheuvel

[permalink] [raw]
Subject: [PATCH v2 1/7] arm64: add CPU field to struct thread_info

The CPU field will be moved back into thread_info even when
THREAD_INFO_IN_TASK is enabled, so add it back to arm64's definition of
struct thread_info.

Note that arm64 always has CONFIG_SMP=y so there is no point in guarding
the CPU field with an #ifdef.

Signed-off-by: Ard Biesheuvel <[email protected]>
Acked-by: Catalin Marinas <[email protected]>
Acked-by: Mark Rutland <[email protected]>
---
arch/arm64/include/asm/thread_info.h | 1 +
arch/arm64/kernel/asm-offsets.c | 1 +
2 files changed, 2 insertions(+)

diff --git a/arch/arm64/include/asm/thread_info.h b/arch/arm64/include/asm/thread_info.h
index 6623c99f0984..c02bc8c183c3 100644
--- a/arch/arm64/include/asm/thread_info.h
+++ b/arch/arm64/include/asm/thread_info.h
@@ -42,6 +42,7 @@ struct thread_info {
void *scs_base;
void *scs_sp;
#endif
+ u32 cpu;
};

#define thread_saved_pc(tsk) \
diff --git a/arch/arm64/kernel/asm-offsets.c b/arch/arm64/kernel/asm-offsets.c
index 551427ae8cc5..cee9f3e9f906 100644
--- a/arch/arm64/kernel/asm-offsets.c
+++ b/arch/arm64/kernel/asm-offsets.c
@@ -29,6 +29,7 @@ int main(void)
DEFINE(TSK_ACTIVE_MM, offsetof(struct task_struct, active_mm));
DEFINE(TSK_CPU, offsetof(struct task_struct, cpu));
BLANK();
+ DEFINE(TSK_TI_CPU, offsetof(struct task_struct, thread_info.cpu));
DEFINE(TSK_TI_FLAGS, offsetof(struct task_struct, thread_info.flags));
DEFINE(TSK_TI_PREEMPT, offsetof(struct task_struct, thread_info.preempt_count));
#ifdef CONFIG_ARM64_SW_TTBR0_PAN
--
2.30.2

2021-09-30 13:30:41

by Ard Biesheuvel

[permalink] [raw]
Subject: Re: [PATCH v2 1/7] arm64: add CPU field to struct thread_info

On Thu, 30 Sept 2021 at 15:06, Christophe Leroy
<[email protected]> wrote:
>
>
>
> Le 30/09/2021 à 14:58, Ard Biesheuvel a écrit :
> > The CPU field will be moved back into thread_info even when
> > THREAD_INFO_IN_TASK is enabled, so add it back to arm64's definition of
> > struct thread_info.
> >
> > Note that arm64 always has CONFIG_SMP=y so there is no point in guarding
> > the CPU field with an #ifdef.
> >
> > Signed-off-by: Ard Biesheuvel <[email protected]>
> > Acked-by: Catalin Marinas <[email protected]>
> > Acked-by: Mark Rutland <[email protected]>
> > ---
> > arch/arm64/include/asm/thread_info.h | 1 +
> > arch/arm64/kernel/asm-offsets.c | 1 +
> > 2 files changed, 2 insertions(+)
> >
> > diff --git a/arch/arm64/include/asm/thread_info.h b/arch/arm64/include/asm/thread_info.h
> > index 6623c99f0984..c02bc8c183c3 100644
> > --- a/arch/arm64/include/asm/thread_info.h
> > +++ b/arch/arm64/include/asm/thread_info.h
> > @@ -42,6 +42,7 @@ struct thread_info {
> > void *scs_base;
> > void *scs_sp;
> > #endif
> > + u32 cpu;
> > };
> >
> > #define thread_saved_pc(tsk) \
> > diff --git a/arch/arm64/kernel/asm-offsets.c b/arch/arm64/kernel/asm-offsets.c
> > index 551427ae8cc5..cee9f3e9f906 100644
> > --- a/arch/arm64/kernel/asm-offsets.c
> > +++ b/arch/arm64/kernel/asm-offsets.c
> > @@ -29,6 +29,7 @@ int main(void)
> > DEFINE(TSK_ACTIVE_MM, offsetof(struct task_struct, active_mm));
> > DEFINE(TSK_CPU, offsetof(struct task_struct, cpu));
> > BLANK();
> > + DEFINE(TSK_TI_CPU, offsetof(struct task_struct, thread_info.cpu));
>
> Why adding that now ? For powerpc you do the switch in 5.
>


Why not?


> > DEFINE(TSK_TI_FLAGS, offsetof(struct task_struct, thread_info.flags));
> > DEFINE(TSK_TI_PREEMPT, offsetof(struct task_struct, thread_info.preempt_count));
> > #ifdef CONFIG_ARM64_SW_TTBR0_PAN
> >

2021-09-30 13:30:45

by Christophe Leroy

[permalink] [raw]
Subject: Re: [PATCH v2 1/7] arm64: add CPU field to struct thread_info



Le 30/09/2021 à 14:58, Ard Biesheuvel a écrit :
> The CPU field will be moved back into thread_info even when
> THREAD_INFO_IN_TASK is enabled, so add it back to arm64's definition of
> struct thread_info.
>
> Note that arm64 always has CONFIG_SMP=y so there is no point in guarding
> the CPU field with an #ifdef.
>
> Signed-off-by: Ard Biesheuvel <[email protected]>
> Acked-by: Catalin Marinas <[email protected]>
> Acked-by: Mark Rutland <[email protected]>
> ---
> arch/arm64/include/asm/thread_info.h | 1 +
> arch/arm64/kernel/asm-offsets.c | 1 +
> 2 files changed, 2 insertions(+)
>
> diff --git a/arch/arm64/include/asm/thread_info.h b/arch/arm64/include/asm/thread_info.h
> index 6623c99f0984..c02bc8c183c3 100644
> --- a/arch/arm64/include/asm/thread_info.h
> +++ b/arch/arm64/include/asm/thread_info.h
> @@ -42,6 +42,7 @@ struct thread_info {
> void *scs_base;
> void *scs_sp;
> #endif
> + u32 cpu;
> };
>
> #define thread_saved_pc(tsk) \
> diff --git a/arch/arm64/kernel/asm-offsets.c b/arch/arm64/kernel/asm-offsets.c
> index 551427ae8cc5..cee9f3e9f906 100644
> --- a/arch/arm64/kernel/asm-offsets.c
> +++ b/arch/arm64/kernel/asm-offsets.c
> @@ -29,6 +29,7 @@ int main(void)
> DEFINE(TSK_ACTIVE_MM, offsetof(struct task_struct, active_mm));
> DEFINE(TSK_CPU, offsetof(struct task_struct, cpu));
> BLANK();
> + DEFINE(TSK_TI_CPU, offsetof(struct task_struct, thread_info.cpu));

Why adding that now ? For powerpc you do the switch in 5.

> DEFINE(TSK_TI_FLAGS, offsetof(struct task_struct, thread_info.flags));
> DEFINE(TSK_TI_PREEMPT, offsetof(struct task_struct, thread_info.preempt_count));
> #ifdef CONFIG_ARM64_SW_TTBR0_PAN
>

2021-09-30 13:35:10

by Christophe Leroy

[permalink] [raw]
Subject: Re: [PATCH v2 1/7] arm64: add CPU field to struct thread_info



Le 30/09/2021 à 15:07, Ard Biesheuvel a écrit :
> On Thu, 30 Sept 2021 at 15:06, Christophe Leroy
> <[email protected]> wrote:
>>
>>
>>
>> Le 30/09/2021 à 14:58, Ard Biesheuvel a écrit :
>>> The CPU field will be moved back into thread_info even when
>>> THREAD_INFO_IN_TASK is enabled, so add it back to arm64's definition of
>>> struct thread_info.
>>>
>>> Note that arm64 always has CONFIG_SMP=y so there is no point in guarding
>>> the CPU field with an #ifdef.
>>>
>>> Signed-off-by: Ard Biesheuvel <[email protected]>
>>> Acked-by: Catalin Marinas <[email protected]>
>>> Acked-by: Mark Rutland <[email protected]>
>>> ---
>>> arch/arm64/include/asm/thread_info.h | 1 +
>>> arch/arm64/kernel/asm-offsets.c | 1 +
>>> 2 files changed, 2 insertions(+)
>>>
>>> diff --git a/arch/arm64/include/asm/thread_info.h b/arch/arm64/include/asm/thread_info.h
>>> index 6623c99f0984..c02bc8c183c3 100644
>>> --- a/arch/arm64/include/asm/thread_info.h
>>> +++ b/arch/arm64/include/asm/thread_info.h
>>> @@ -42,6 +42,7 @@ struct thread_info {
>>> void *scs_base;
>>> void *scs_sp;
>>> #endif
>>> + u32 cpu;
>>> };
>>>
>>> #define thread_saved_pc(tsk) \
>>> diff --git a/arch/arm64/kernel/asm-offsets.c b/arch/arm64/kernel/asm-offsets.c
>>> index 551427ae8cc5..cee9f3e9f906 100644
>>> --- a/arch/arm64/kernel/asm-offsets.c
>>> +++ b/arch/arm64/kernel/asm-offsets.c
>>> @@ -29,6 +29,7 @@ int main(void)
>>> DEFINE(TSK_ACTIVE_MM, offsetof(struct task_struct, active_mm));
>>> DEFINE(TSK_CPU, offsetof(struct task_struct, cpu));
>>> BLANK();
>>> + DEFINE(TSK_TI_CPU, offsetof(struct task_struct, thread_info.cpu));
>>
>> Why adding that now ? For powerpc you do the switch in 5.
>>
>
>
> Why not?

Maybe to remain consistent between archs ?

>
>
>>> DEFINE(TSK_TI_FLAGS, offsetof(struct task_struct, thread_info.flags));
>>> DEFINE(TSK_TI_PREEMPT, offsetof(struct task_struct, thread_info.preempt_count));
>>> #ifdef CONFIG_ARM64_SW_TTBR0_PAN
>>>

2021-09-30 13:37:24

by Christophe Leroy

[permalink] [raw]
Subject: Re: [PATCH v2 1/7] arm64: add CPU field to struct thread_info



Le 30/09/2021 à 15:20, Ard Biesheuvel a écrit :
> On Thu, 30 Sept 2021 at 15:15, Christophe Leroy
> <[email protected]> wrote:
>>
>>
>>
>> Le 30/09/2021 à 15:07, Ard Biesheuvel a écrit :
>>> On Thu, 30 Sept 2021 at 15:06, Christophe Leroy
>>> <[email protected]> wrote:
>>>>
>>>>
>>>>
>>>> Le 30/09/2021 à 14:58, Ard Biesheuvel a écrit :
>>>>> The CPU field will be moved back into thread_info even when
>>>>> THREAD_INFO_IN_TASK is enabled, so add it back to arm64's definition of
>>>>> struct thread_info.
>>>>>
>>>>> Note that arm64 always has CONFIG_SMP=y so there is no point in guarding
>>>>> the CPU field with an #ifdef.
>>>>>
>>>>> Signed-off-by: Ard Biesheuvel <[email protected]>
>>>>> Acked-by: Catalin Marinas <[email protected]>
>>>>> Acked-by: Mark Rutland <[email protected]>
>>>>> ---
>>>>> arch/arm64/include/asm/thread_info.h | 1 +
>>>>> arch/arm64/kernel/asm-offsets.c | 1 +
>>>>> 2 files changed, 2 insertions(+)
>>>>>
>>>>> diff --git a/arch/arm64/include/asm/thread_info.h b/arch/arm64/include/asm/thread_info.h
>>>>> index 6623c99f0984..c02bc8c183c3 100644
>>>>> --- a/arch/arm64/include/asm/thread_info.h
>>>>> +++ b/arch/arm64/include/asm/thread_info.h
>>>>> @@ -42,6 +42,7 @@ struct thread_info {
>>>>> void *scs_base;
>>>>> void *scs_sp;
>>>>> #endif
>>>>> + u32 cpu;
>>>>> };
>>>>>
>>>>> #define thread_saved_pc(tsk) \
>>>>> diff --git a/arch/arm64/kernel/asm-offsets.c b/arch/arm64/kernel/asm-offsets.c
>>>>> index 551427ae8cc5..cee9f3e9f906 100644
>>>>> --- a/arch/arm64/kernel/asm-offsets.c
>>>>> +++ b/arch/arm64/kernel/asm-offsets.c
>>>>> @@ -29,6 +29,7 @@ int main(void)
>>>>> DEFINE(TSK_ACTIVE_MM, offsetof(struct task_struct, active_mm));
>>>>> DEFINE(TSK_CPU, offsetof(struct task_struct, cpu));
>>>>> BLANK();
>>>>> + DEFINE(TSK_TI_CPU, offsetof(struct task_struct, thread_info.cpu));
>>>>
>>>> Why adding that now ? For powerpc you do the switch in 5.
>>>>
>>>
>>>
>>> Why not?
>>
>> Maybe to remain consistent between archs ?
>>
>
> Does it matter?

Probably not :)

>
>>>
>>>
>>>>> DEFINE(TSK_TI_FLAGS, offsetof(struct task_struct, thread_info.flags));
>>>>> DEFINE(TSK_TI_PREEMPT, offsetof(struct task_struct, thread_info.preempt_count));
>>>>> #ifdef CONFIG_ARM64_SW_TTBR0_PAN
>>>>>

2021-09-30 14:36:49

by Ard Biesheuvel

[permalink] [raw]
Subject: [PATCH v2 2/7] x86: add CPU field to struct thread_info

The CPU field will be moved back into thread_info even when
THREAD_INFO_IN_TASK is enabled, so add it back to x86's definition of
struct thread_info.

Signed-off-by: Ard Biesheuvel <[email protected]>
Acked-by: Borislav Petkov <[email protected]>
Acked-by: Mark Rutland <[email protected]>
---
arch/x86/include/asm/thread_info.h | 3 +++
1 file changed, 3 insertions(+)

diff --git a/arch/x86/include/asm/thread_info.h b/arch/x86/include/asm/thread_info.h
index cf132663c219..ebec69c35e95 100644
--- a/arch/x86/include/asm/thread_info.h
+++ b/arch/x86/include/asm/thread_info.h
@@ -57,6 +57,9 @@ struct thread_info {
unsigned long flags; /* low level flags */
unsigned long syscall_work; /* SYSCALL_WORK_ flags */
u32 status; /* thread synchronous flags */
+#ifdef CONFIG_SMP
+ u32 cpu; /* current CPU */
+#endif
};

#define INIT_THREAD_INFO(tsk) \
--
2.30.2

2021-09-30 14:38:03

by Ard Biesheuvel

[permalink] [raw]
Subject: [PATCH v2 7/7] riscv: rely on core code to keep thread_info::cpu updated

Now that the core code switched back to using thread_info::cpu to keep
a task's CPU number, we no longer need to keep it in sync explicitly. So
just drop the code that does this.

Signed-off-by: Ard Biesheuvel <[email protected]>
Acked-by: Palmer Dabbelt <[email protected]>
Acked-by: Mark Rutland <[email protected]>
---
arch/riscv/kernel/asm-offsets.c | 1 -
arch/riscv/kernel/entry.S | 5 -----
arch/riscv/kernel/head.S | 1 -
3 files changed, 7 deletions(-)

diff --git a/arch/riscv/kernel/asm-offsets.c b/arch/riscv/kernel/asm-offsets.c
index 90f8ce64fa6f..478d9f02dab5 100644
--- a/arch/riscv/kernel/asm-offsets.c
+++ b/arch/riscv/kernel/asm-offsets.c
@@ -33,7 +33,6 @@ void asm_offsets(void)
OFFSET(TASK_TI_PREEMPT_COUNT, task_struct, thread_info.preempt_count);
OFFSET(TASK_TI_KERNEL_SP, task_struct, thread_info.kernel_sp);
OFFSET(TASK_TI_USER_SP, task_struct, thread_info.user_sp);
- OFFSET(TASK_TI_CPU, 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]);
diff --git a/arch/riscv/kernel/entry.S b/arch/riscv/kernel/entry.S
index 98f502654edd..459eb1714353 100644
--- a/arch/riscv/kernel/entry.S
+++ b/arch/riscv/kernel/entry.S
@@ -544,11 +544,6 @@ ENTRY(__switch_to)
REG_L s9, TASK_THREAD_S9_RA(a4)
REG_L s10, TASK_THREAD_S10_RA(a4)
REG_L s11, TASK_THREAD_S11_RA(a4)
- /* Swap the CPU entry around. */
- lw a3, TASK_TI_CPU(a0)
- lw a4, TASK_TI_CPU(a1)
- sw a3, TASK_TI_CPU(a1)
- sw a4, TASK_TI_CPU(a0)
/* The offset of thread_info in task_struct is zero. */
move tp, a1
ret
diff --git a/arch/riscv/kernel/head.S b/arch/riscv/kernel/head.S
index fce5184b22c3..d5ec30ef6f5d 100644
--- a/arch/riscv/kernel/head.S
+++ b/arch/riscv/kernel/head.S
@@ -317,7 +317,6 @@ clear_bss_done:
call setup_trap_vector
/* Restore C environment */
la tp, init_task
- sw zero, TASK_TI_CPU(tp)
la sp, init_thread_union + THREAD_SIZE

#ifdef CONFIG_KASAN
--
2.30.2

2021-09-30 14:38:30

by Ard Biesheuvel

[permalink] [raw]
Subject: [PATCH v2 3/7] s390: add CPU field to struct thread_info

The CPU field will be moved back into thread_info even when
THREAD_INFO_IN_TASK is enabled, so add it back to s390's definition of
struct thread_info.

Note that s390 always has CONFIG_SMP=y so there is no point in guarding
the CPU field with an #ifdef.

Cc: Heiko Carstens <[email protected]>
Cc: Vasily Gorbik <[email protected]>
Cc: Christian Borntraeger <[email protected]>
Signed-off-by: Ard Biesheuvel <[email protected]>
Acked-by: Mark Rutland <[email protected]>
---
arch/s390/include/asm/thread_info.h | 1 +
1 file changed, 1 insertion(+)

diff --git a/arch/s390/include/asm/thread_info.h b/arch/s390/include/asm/thread_info.h
index e6674796aa6f..b2ffcb4fe000 100644
--- a/arch/s390/include/asm/thread_info.h
+++ b/arch/s390/include/asm/thread_info.h
@@ -37,6 +37,7 @@
struct thread_info {
unsigned long flags; /* low level flags */
unsigned long syscall_work; /* SYSCALL_WORK_ flags */
+ unsigned int cpu; /* current CPU */
};

/*
--
2.30.2

2021-09-30 14:39:25

by Ard Biesheuvel

[permalink] [raw]
Subject: [PATCH v2 4/7] powerpc: add CPU field to struct thread_info

The CPU field will be moved back into thread_info even when
THREAD_INFO_IN_TASK is enabled, so add it back to powerpc's definition
of struct thread_info.

Signed-off-by: Ard Biesheuvel <[email protected]>
Acked-by: Mark Rutland <[email protected]>
Acked-by: Michael Ellerman <[email protected]>
---
arch/powerpc/include/asm/thread_info.h | 3 +++
1 file changed, 3 insertions(+)

diff --git a/arch/powerpc/include/asm/thread_info.h b/arch/powerpc/include/asm/thread_info.h
index b4ec6c7dd72e..5725029aaa29 100644
--- a/arch/powerpc/include/asm/thread_info.h
+++ b/arch/powerpc/include/asm/thread_info.h
@@ -47,6 +47,9 @@
struct thread_info {
int preempt_count; /* 0 => preemptable,
<0 => BUG */
+#ifdef CONFIG_SMP
+ unsigned int cpu;
+#endif
unsigned long local_flags; /* private flags for thread */
#ifdef CONFIG_LIVEPATCH
unsigned long *livepatch_sp;
--
2.30.2

2021-09-30 14:39:40

by Ard Biesheuvel

[permalink] [raw]
Subject: [PATCH v2 6/7] powerpc: smp: remove hack to obtain offset of task_struct::cpu

Instead of relying on awful hacks to obtain the offset of the cpu field
in struct task_struct, move it back into struct thread_info, which does
not create the same level of circular dependency hell when trying to
include the header file that defines it.

Signed-off-by: Ard Biesheuvel <[email protected]>
Acked-by: Mark Rutland <[email protected]>
Acked-by: Michael Ellerman <[email protected]>
---
arch/powerpc/Makefile | 11 -----------
arch/powerpc/include/asm/smp.h | 17 +----------------
arch/powerpc/kernel/asm-offsets.c | 2 --
3 files changed, 1 insertion(+), 29 deletions(-)

diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
index aa6808e70647..54cad1faa5d0 100644
--- a/arch/powerpc/Makefile
+++ b/arch/powerpc/Makefile
@@ -446,17 +446,6 @@ else
endif
endif

-ifdef CONFIG_SMP
-ifdef CONFIG_PPC32
-prepare: task_cpu_prepare
-
-PHONY += task_cpu_prepare
-task_cpu_prepare: prepare0
- $(eval KBUILD_CFLAGS += -D_TASK_CPU=$(shell awk '{if ($$2 == "TASK_CPU") print $$3;}' include/generated/asm-offsets.h))
-
-endif # CONFIG_PPC32
-endif # CONFIG_SMP
-
PHONY += checkbin
# Check toolchain versions:
# - gcc-4.6 is the minimum kernel-wide version so nothing required.
diff --git a/arch/powerpc/include/asm/smp.h b/arch/powerpc/include/asm/smp.h
index 7ef1cd8168a0..007332a4a732 100644
--- a/arch/powerpc/include/asm/smp.h
+++ b/arch/powerpc/include/asm/smp.h
@@ -87,22 +87,7 @@ int is_cpu_dead(unsigned int cpu);
/* 32-bit */
extern int smp_hw_index[];

-/*
- * This is particularly ugly: it appears we can't actually get the definition
- * of task_struct here, but we need access to the CPU this task is running on.
- * Instead of using task_struct we're using _TASK_CPU which is extracted from
- * asm-offsets.h by kbuild to get the current processor ID.
- *
- * This also needs to be safeguarded when building asm-offsets.s because at
- * that time _TASK_CPU is not defined yet. It could have been guarded by
- * _TASK_CPU itself, but we want the build to fail if _TASK_CPU is missing
- * when building something else than asm-offsets.s
- */
-#ifdef GENERATING_ASM_OFFSETS
-#define raw_smp_processor_id() (0)
-#else
-#define raw_smp_processor_id() (*(unsigned int *)((void *)current + _TASK_CPU))
-#endif
+#define raw_smp_processor_id() (current_thread_info()->cpu)
#define hard_smp_processor_id() (smp_hw_index[smp_processor_id()])

static inline int get_hard_smp_processor_id(int cpu)
diff --git a/arch/powerpc/kernel/asm-offsets.c b/arch/powerpc/kernel/asm-offsets.c
index e37e4546034e..cc05522f50bf 100644
--- a/arch/powerpc/kernel/asm-offsets.c
+++ b/arch/powerpc/kernel/asm-offsets.c
@@ -9,8 +9,6 @@
* #defines from the assembly-language output.
*/

-#define GENERATING_ASM_OFFSETS /* asm/smp.h */
-
#include <linux/compat.h>
#include <linux/signal.h>
#include <linux/sched.h>
--
2.30.2

2021-09-30 14:41:16

by Ard Biesheuvel

[permalink] [raw]
Subject: Re: [PATCH v2 1/7] arm64: add CPU field to struct thread_info

On Thu, 30 Sept 2021 at 15:15, Christophe Leroy
<[email protected]> wrote:
>
>
>
> Le 30/09/2021 à 15:07, Ard Biesheuvel a écrit :
> > On Thu, 30 Sept 2021 at 15:06, Christophe Leroy
> > <[email protected]> wrote:
> >>
> >>
> >>
> >> Le 30/09/2021 à 14:58, Ard Biesheuvel a écrit :
> >>> The CPU field will be moved back into thread_info even when
> >>> THREAD_INFO_IN_TASK is enabled, so add it back to arm64's definition of
> >>> struct thread_info.
> >>>
> >>> Note that arm64 always has CONFIG_SMP=y so there is no point in guarding
> >>> the CPU field with an #ifdef.
> >>>
> >>> Signed-off-by: Ard Biesheuvel <[email protected]>
> >>> Acked-by: Catalin Marinas <[email protected]>
> >>> Acked-by: Mark Rutland <[email protected]>
> >>> ---
> >>> arch/arm64/include/asm/thread_info.h | 1 +
> >>> arch/arm64/kernel/asm-offsets.c | 1 +
> >>> 2 files changed, 2 insertions(+)
> >>>
> >>> diff --git a/arch/arm64/include/asm/thread_info.h b/arch/arm64/include/asm/thread_info.h
> >>> index 6623c99f0984..c02bc8c183c3 100644
> >>> --- a/arch/arm64/include/asm/thread_info.h
> >>> +++ b/arch/arm64/include/asm/thread_info.h
> >>> @@ -42,6 +42,7 @@ struct thread_info {
> >>> void *scs_base;
> >>> void *scs_sp;
> >>> #endif
> >>> + u32 cpu;
> >>> };
> >>>
> >>> #define thread_saved_pc(tsk) \
> >>> diff --git a/arch/arm64/kernel/asm-offsets.c b/arch/arm64/kernel/asm-offsets.c
> >>> index 551427ae8cc5..cee9f3e9f906 100644
> >>> --- a/arch/arm64/kernel/asm-offsets.c
> >>> +++ b/arch/arm64/kernel/asm-offsets.c
> >>> @@ -29,6 +29,7 @@ int main(void)
> >>> DEFINE(TSK_ACTIVE_MM, offsetof(struct task_struct, active_mm));
> >>> DEFINE(TSK_CPU, offsetof(struct task_struct, cpu));
> >>> BLANK();
> >>> + DEFINE(TSK_TI_CPU, offsetof(struct task_struct, thread_info.cpu));
> >>
> >> Why adding that now ? For powerpc you do the switch in 5.
> >>
> >
> >
> > Why not?
>
> Maybe to remain consistent between archs ?
>

Does it matter?

> >
> >
> >>> DEFINE(TSK_TI_FLAGS, offsetof(struct task_struct, thread_info.flags));
> >>> DEFINE(TSK_TI_PREEMPT, offsetof(struct task_struct, thread_info.preempt_count));
> >>> #ifdef CONFIG_ARM64_SW_TTBR0_PAN
> >>>