On 11/4/21 8:25 AM, Helge Deller wrote:
On 11/4/21 09:39, Ard Biesheuvel wrote:
On Thu, 4 Nov 2021 at 09:31, Helge Deller <deller@xxxxxx> wrote:
On 11/4/21 09:26, Ard Biesheuvel wrote:
In commit 2214c0e77259 ("parisc: Move thread_info into task struct")
PA-RISC gained support for THREAD_INFO_IN_TASK while changes were
already underway to keep the CPU field in thread_info rather than move
it into task_struct when THREAD_INFO_IN_TASK is enabled. The result is a
broken build for all PA-RISC configs that enable SMP.
So let's partially revert that commit, and get rid of the ugly hack to
get at the offset of task_struct::cpu without having to include
linux/sched.h, and put the CPU field back where it was before.
Reported-by: Guenter Roeck <linux@xxxxxxxxxxxx>
Fixes: bcf9033e5449 ("sched: move CPU field back into thread_info if THREAD_INFO_IN_TASK=y")
Signed-off-by: Ard Biesheuvel <ardb@xxxxxxxxxx>
Thanks Ard!
I actually had the same patch finished right now too :-)
I was wondering about that :-)
One small nid though. See below...
---
arch/parisc/include/asm/smp.h | 19 ++-----------------
arch/parisc/include/asm/thread_info.h | 2 ++
arch/parisc/kernel/asm-offsets.c | 4 +---
arch/parisc/kernel/smp.c | 2 +-
4 files changed, 6 insertions(+), 21 deletions(-)
diff --git a/arch/parisc/include/asm/smp.h b/arch/parisc/include/asm/smp.h
index 16d41127500e..cba55abf7bac 100644
--- a/arch/parisc/include/asm/smp.h
+++ b/arch/parisc/include/asm/smp.h
@@ -34,23 +34,8 @@ extern void arch_send_call_function_ipi_mask(const struct cpumask *mask);
#endif /* !ASSEMBLY */
-/*
- * 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
-#include <asm/asm-offsets.h>
-#define raw_smp_processor_id() (*(unsigned int *)((void *)current + TASK_CPU))
-#endif
+#define raw_smp_processor_id() (current_thread_info()->cpu)
+
#else /* CONFIG_SMP */
static inline void smp_send_all_nop(void) { return; }
diff --git a/arch/parisc/include/asm/thread_info.h b/arch/parisc/include/asm/thread_info.h
index 75657c2c54e1..d6268834bfa5 100644
--- a/arch/parisc/include/asm/thread_info.h
+++ b/arch/parisc/include/asm/thread_info.h
@@ -8,12 +8,14 @@
struct thread_info {
unsigned long flags; /* thread_info flags (see TIF_*) */
+ __u32 cpu; /* current CPU */
int preempt_count; /* 0=premptable, <0=BUG; will also serve as bh-counter */
};
#define INIT_THREAD_INFO(tsk) \
{ \
.flags = 0, \
+ .cpu = 0, \
.preempt_count = INIT_PREEMPT_COUNT, \
}
diff --git a/arch/parisc/kernel/asm-offsets.c b/arch/parisc/kernel/asm-offsets.c
index e35154035441..629d38e36e22 100644
--- a/arch/parisc/kernel/asm-offsets.c
+++ b/arch/parisc/kernel/asm-offsets.c
@@ -14,8 +14,6 @@
* Copyright (C) 2003 James Bottomley <jejb at parisc-linux.org>
*/
-#define GENERATING_ASM_OFFSETS /* asm/smp.h */
-
#include <linux/types.h>
#include <linux/sched.h>
#include <linux/thread_info.h>
@@ -40,7 +38,7 @@ int main(void)
DEFINE(TASK_TI_FLAGS, offsetof(struct task_struct, thread_info.flags));
DEFINE(TASK_STACK, offsetof(struct task_struct, stack));
#ifdef CONFIG_SMP
- DEFINE(TASK_CPU, offsetof(struct task_struct, cpu));
+ DEFINE(TASK_CPU, offsetof(struct task_struct, thread_info.cpu));
#endif
The TASK_CPU define isn't needed any longer.
So, the whole part inside #ifdef..#endif can go.
OK let's just drop it then.
Ok
BLANK();
DEFINE(TASK_REGS, offsetof(struct task_struct, thread.regs));
diff --git a/arch/parisc/kernel/smp.c b/arch/parisc/kernel/smp.c
index 171925285f3e..0cd97fa004c5 100644
--- a/arch/parisc/kernel/smp.c
+++ b/arch/parisc/kernel/smp.c
@@ -339,7 +339,7 @@ int smp_boot_one_cpu(int cpuid, struct task_struct *idle)
const struct cpuinfo_parisc *p = &per_cpu(cpu_data, cpuid);
long timeout;
- idle->cpu = cpuid;
+ task_thread_info(idle)->cpu = cpuid;
I was wondering if this could simply be dropped altogether? I am
pretty sure it is redundant, as the core code sets this field as well,
but I don't have access to a SMP PA-RISC machine to test it.
Good point.
I tested and it can be dropped.
/* Let _start know what logical CPU we're booting
** (offset into init_tasks[],cpu_data[])
If it's Ok for you I'll clean up your patch and submit
all with the next pull request to Linus later today.
Whatever works for you is fine with me.
Ok. I've cleaned it up, applied it to my for-next tree and will push later today to Linus
with other patches. Here is the current version:
https://git.kernel.org/pub/scm/linux/kernel/git/deller/parisc-linux.git/commit/?h=for-next
Works for me.
Thanks,
Guenter
