Messages in this thread | | | Subject | Re: [PATCH] MIPS: Fix using smp_processor_id() in preemptible in show_cpuinfo() | From | Tiezhu Yang <> | Date | Thu, 25 Nov 2021 19:32:39 +0800 |
| |
On 11/25/2021 05:34 PM, Thomas Bogendoerfer wrote: > On Mon, Nov 22, 2021 at 06:17:37PM +0800, Tiezhu Yang wrote: >> There exists the following issue under DEBUG_PREEMPT: >> >> BUG: using smp_processor_id() in preemptible [00000000] code: systemd/1 >> caller is show_cpuinfo+0x460/0xea0 >> ... >> Call Trace: >> [<ffffffff8020f0dc>] show_stack+0x94/0x128 >> [<ffffffff80e6cab4>] dump_stack_lvl+0x94/0xd8 >> [<ffffffff80e74c5c>] check_preemption_disabled+0x104/0x110 >> [<ffffffff802209c8>] show_cpuinfo+0x460/0xea0 >> [<ffffffff80539d54>] seq_read_iter+0xfc/0x4f8 >> [<ffffffff804fcc10>] new_sync_read+0x110/0x1b8 >> [<ffffffff804ff57c>] vfs_read+0x1b4/0x1d0 >> [<ffffffff804ffb18>] ksys_read+0xd0/0x110 >> [<ffffffff8021c090>] syscall_common+0x34/0x58 >> >> We can see the following call trace: >> show_cpuinfo() >> cpu_has_fpu >> current_cpu_data >> smp_processor_id() >> >> $ addr2line -f -e vmlinux 0xffffffff802209c8 >> show_cpuinfo >> arch/mips/kernel/proc.c:188 >> >> $ head -188 arch/mips/kernel/proc.c | tail -1 >> if (cpu_has_fpu) >> >> arch/mips/include/asm/cpu-features.h >> # define cpu_has_fpu (current_cpu_data.options & MIPS_CPU_FPU) >> >> arch/mips/include/asm/cpu-info.h >> #define current_cpu_data cpu_data[smp_processor_id()] >> >> Based on the above analysis, fix the issue by disabling preemption >> around cpu_has_fpu in show_cpuinfo(). >> >> Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn> > > missing > > Fixes: 626bfa037299 ("MIPS: kernel: proc: add CPU option reporting")
Thank you, will add in v2.
> >> --- >> arch/mips/kernel/proc.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/arch/mips/kernel/proc.c b/arch/mips/kernel/proc.c >> index 376a6e2..c6c2661 100644 >> --- a/arch/mips/kernel/proc.c >> +++ b/arch/mips/kernel/proc.c >> @@ -185,8 +185,10 @@ static int show_cpuinfo(struct seq_file *m, void *v) >> seq_puts(m, " tx39_cache"); >> if (cpu_has_octeon_cache) >> seq_puts(m, " octeon_cache"); >> + preempt_disable(); >> if (cpu_has_fpu) >> seq_puts(m, " fpu"); >> + preempt_enable(); > > what about using raw_cpu_has_fpu() instead ?
OK, no problem, using raw_cpu_has_fpu can also fix the issue and looks better, I will send v2 soon.
Thanks, Tiezhu
> > Thomas. >
| |