Thank John for correcting me ;-)
On Thu, Jul 14, 2022 at 5:25 PM John Ogness <john.ogness@xxxxxxxxxxxxx> wrote:
>
> On 2022-07-14, Zhouyi Zhou <zhouzhouyi@xxxxxxxxx> wrote:
> > use raw_smp_processor_id() in arch_touch_nmi_watchdog
> > because when called from watchdog, the cpu is preemptible.
>
> I would expect the correct solution is to make it a non-migration
> section. Something like the below (untested) patch.
I applied your patch (I have made a tiny modification by removing the
return statement after "goto out;") and
passed the test in the ppc VM of Open Source Lab of Oregon State University.
Tested-by: Zhouyi Zhou <zhouzhouyi@xxxxxxxxx>
Many Thanks
Kindly Regards
Zhouyi
>
> John Ogness
>
> diff --git a/arch/powerpc/kernel/watchdog.c b/arch/powerpc/kernel/watchdog.c
> index bfc27496fe7e..9d34aa809241 100644
> --- a/arch/powerpc/kernel/watchdog.c
> +++ b/arch/powerpc/kernel/watchdog.c
> @@ -450,17 +450,23 @@ static enum hrtimer_restart watchdog_timer_fn(struct hrtimer *hrtimer)
> void arch_touch_nmi_watchdog(void)
> {
> unsigned long ticks = tb_ticks_per_usec * wd_timer_period_ms * 1000;
> - int cpu = smp_processor_id();
> + int cpu;
> u64 tb;
>
> - if (!cpumask_test_cpu(cpu, &watchdog_cpumask))
> + cpu = get_cpu();
> +
> + if (!cpumask_test_cpu(cpu, &watchdog_cpumask)) {
> + goto out;
> return;
I think we should remove the return statement here.
> + }
>
> tb = get_tb();
> if (tb - per_cpu(wd_timer_tb, cpu) >= ticks) {
> per_cpu(wd_timer_tb, cpu) = tb;
> wd_smp_clear_cpu_pending(cpu);
> }
> +out:
> + put_cpu();
> }
> EXPORT_SYMBOL(arch_touch_nmi_watchdog);
