Thread (16 messages) flat view 16 messages, 3 authors, 2021-09-21

Re: [PATCH v5 5/5] ARM: smp: Enable THREAD_INFO_IN_TASK

From: Amit Kachhap <hidden>
Date: 2021-09-19 13:40:19


On 9/18/21 2:14 PM, Ard Biesheuvel wrote:
quoted hunk ↗ jump to hunk
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 <redacted>
Signed-off-by: Keith Packard <redacted>
Signed-off-by: Ard Biesheuvel <ardb@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
quoted hunk ↗ jump to hunk
+#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);
  
  	/*
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help