[PATCH v5 5/5] ARM: smp: Enable THREAD_INFO_IN_TASK
Amit Kachhap
amit.kachhap at arm.com
Sun Sep 19 06:38:07 PDT 2021
On 9/18/21 2:14 PM, Ard Biesheuvel wrote:
> Now that we no longer rely on thread_info living at the base of the task
> stack to be able to access the 'current' pointer, we can wire up the
> generic support for moving thread_info into the task struct itself.
>
> Note that this requires us to update the cpu field in thread_info
> explicitly, now that the core code no longer does so. Ideally, we would
> switch the percpu code to access the cpu field in task_struct instead,
> but this unleashes #include circular dependency hell.
>
> Co-developed-by: Keith Packard <keithpac at amazon.com>
> Signed-off-by: Keith Packard <keithpac at amazon.com>
> Signed-off-by: Ard Biesheuvel <ardb at kernel.org>
> ---
> arch/arm/Kconfig | 1 +
> arch/arm/include/asm/assembler.h | 5 +++++
> arch/arm/include/asm/switch_to.h | 14 ++++++++++++++
> arch/arm/include/asm/thread_info.h | 10 +++++++++-
> arch/arm/kernel/asm-offsets.c | 2 ++
> arch/arm/kernel/entry-armv.S | 2 +-
> arch/arm/kernel/smp.c | 3 +++
> 7 files changed, 35 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index cd195e6f4ea6..4f61c9789e7f 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -125,6 +125,7 @@ config ARM
> select PERF_USE_VMALLOC
> select RTC_LIB
> select SYS_SUPPORTS_APM_EMULATION
> + select THREAD_INFO_IN_TASK if CURRENT_POINTER_IN_TPIDRURO
> select TRACE_IRQFLAGS_SUPPORT if !CPU_V7M
> # Above selects are sorted alphabetically; please add new ones
> # according to that. Thanks.
> diff --git a/arch/arm/include/asm/assembler.h b/arch/arm/include/asm/assembler.h
> index c1551dee28be..7d23d4bb2168 100644
> --- a/arch/arm/include/asm/assembler.h
> +++ b/arch/arm/include/asm/assembler.h
> @@ -227,10 +227,15 @@
> * Get current thread_info.
> */
> .macro get_thread_info, rd
> +#ifdef CONFIG_THREAD_INFO_IN_TASK
> + /* thread_info is the first member of struct task_struct */
> + get_current \rd
I have a minor review comment here,
By looking at the this code it looks like get_thread_info calls
get_current and get_current again calls get_thread_info. Probably
recursion may never happen due to config dependency order. But this can
be simplified by doing something like below.
+#ifdef CONFIG_CURRENT_POINTER_IN_TPIDRURO
+mrc p15, 0, \rd, c13, c0, 3 @ get TPIDRURO register
+#else
Thanks,
Amit Daniel
> +#else
> ARM( mov \rd, sp, lsr #THREAD_SIZE_ORDER + PAGE_SHIFT )
> THUMB( mov \rd, sp )
> THUMB( lsr \rd, \rd, #THREAD_SIZE_ORDER + PAGE_SHIFT )
> mov \rd, \rd, lsl #THREAD_SIZE_ORDER + PAGE_SHIFT
> +#endif
> .endm
>
> /*
> diff --git a/arch/arm/include/asm/switch_to.h b/arch/arm/include/asm/switch_to.h
> index 61e4a3c4ca6e..b55c7b2755e4 100644
> --- a/arch/arm/include/asm/switch_to.h
> +++ b/arch/arm/include/asm/switch_to.h
> @@ -23,9 +23,23 @@
> */
> extern struct task_struct *__switch_to(struct task_struct *, struct thread_info *, struct thread_info *);
>
> +static inline void set_ti_cpu(struct task_struct *p)
> +{
> +#ifdef CONFIG_THREAD_INFO_IN_TASK
> + /*
> + * The core code no longer maintains the thread_info::cpu field once
> + * CONFIG_THREAD_INFO_IN_TASK is in effect, but we rely on it for
> + * raw_smp_processor_id(), which cannot access struct task_struct*
> + * directly for reasons of circular #inclusion hell.
> + */
> + task_thread_info(p)->cpu = task_cpu(p);
> +#endif
> +}
> +
> #define switch_to(prev,next,last) \
> do { \
> __complete_pending_tlbi(); \
> + set_ti_cpu(next); \
> if (IS_ENABLED(CONFIG_CURRENT_POINTER_IN_TPIDRURO)) \
> __this_cpu_write(__entry_task, next); \
> last = __switch_to(prev,task_thread_info(prev), task_thread_info(next)); \
> diff --git a/arch/arm/include/asm/thread_info.h b/arch/arm/include/asm/thread_info.h
> index 76b6fbd5540c..787511396f3f 100644
> --- a/arch/arm/include/asm/thread_info.h
> +++ b/arch/arm/include/asm/thread_info.h
> @@ -54,7 +54,9 @@ struct cpu_context_save {
> struct thread_info {
> unsigned long flags; /* low level flags */
> int preempt_count; /* 0 => preemptable, <0 => bug */
> +#ifndef CONFIG_THREAD_INFO_IN_TASK
> struct task_struct *task; /* main task structure */
> +#endif
> __u32 cpu; /* cpu */
> __u32 cpu_domain; /* cpu domain */
> struct cpu_context_save cpu_context; /* cpu context */
> @@ -70,11 +72,16 @@ struct thread_info {
>
> #define INIT_THREAD_INFO(tsk) \
> { \
> - .task = &tsk, \
> + INIT_THREAD_INFO_TASK(tsk) \
> .flags = 0, \
> .preempt_count = INIT_PREEMPT_COUNT, \
> }
>
> +#ifdef CONFIG_THREAD_INFO_IN_TASK
> +#define INIT_THREAD_INFO_TASK(tsk)
> +#else
> +#define INIT_THREAD_INFO_TASK(tsk) .task = &(tsk),
> +
> /*
> * how to get the thread information struct from C
> */
> @@ -85,6 +92,7 @@ static inline struct thread_info *current_thread_info(void)
> return (struct thread_info *)
> (current_stack_pointer & ~(THREAD_SIZE - 1));
> }
> +#endif
>
> #define thread_saved_pc(tsk) \
> ((unsigned long)(task_thread_info(tsk)->cpu_context.pc))
> diff --git a/arch/arm/kernel/asm-offsets.c b/arch/arm/kernel/asm-offsets.c
> index 9c864ee76107..645845e4982a 100644
> --- a/arch/arm/kernel/asm-offsets.c
> +++ b/arch/arm/kernel/asm-offsets.c
> @@ -43,7 +43,9 @@ int main(void)
> BLANK();
> DEFINE(TI_FLAGS, offsetof(struct thread_info, flags));
> DEFINE(TI_PREEMPT, offsetof(struct thread_info, preempt_count));
> +#ifndef CONFIG_THREAD_INFO_IN_TASK
> DEFINE(TI_TASK, offsetof(struct thread_info, task));
> +#endif
> DEFINE(TI_CPU, offsetof(struct thread_info, cpu));
> DEFINE(TI_CPU_DOMAIN, offsetof(struct thread_info, cpu_domain));
> DEFINE(TI_CPU_SAVE, offsetof(struct thread_info, cpu_context));
> diff --git a/arch/arm/kernel/entry-armv.S b/arch/arm/kernel/entry-armv.S
> index 7263a45abf3d..a54b5044d406 100644
> --- a/arch/arm/kernel/entry-armv.S
> +++ b/arch/arm/kernel/entry-armv.S
> @@ -765,7 +765,7 @@ ENTRY(__switch_to)
> .endif
> ldr r7, [r7, #TSK_STACK_CANARY & IMM12_MASK]
> #elif defined(CONFIG_CURRENT_POINTER_IN_TPIDRURO)
> - ldr r7, [r2, #TI_TASK]
> + mov r7, r2 @ Preserve 'next'
> #endif
> #ifdef CONFIG_CPU_USE_DOMAINS
> mcr p15, 0, r6, c3, c0, 0 @ Set domain register
> diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c
> index 97ee6b1567e9..cde5b6d8bac5 100644
> --- a/arch/arm/kernel/smp.c
> +++ b/arch/arm/kernel/smp.c
> @@ -154,6 +154,9 @@ int __cpu_up(unsigned int cpu, struct task_struct *idle)
> secondary_data.swapper_pg_dir = get_arch_pgd(swapper_pg_dir);
> #endif
> secondary_data.task = idle;
> + if (IS_ENABLED(CONFIG_THREAD_INFO_IN_TASK))
> + task_thread_info(idle)->cpu = cpu;
> +
> sync_cache_w(&secondary_data);
>
> /*
>
More information about the linux-arm-kernel
mailing list