Re: [PATCH] perf/riscv: clear snapshot setup state when a CPU goes offline

[Anup Patel]

On Sun, Sep 6, 2026 at 9:58 AM Pengpeng Hou <hppiscas@xxxxxxx> wrote:
>
> pmu_sbi_dying_cpu() disables the calling hart's SBI PMU snapshot shared
> memory but leaves snapshot_set_done set. When the CPU comes back online,
> pmu_sbi_snapshot_setup() returns early without registering the shared
> memory again.
>
> Snapshot operations on that hart can then fail because the firmware no
> longer has a snapshot area, while the driver still follows its snapshot
> paths.
>
> Clear the per-CPU setup flag after snapshot disable succeeds. Retain the
> flag and the existing error return if disable fails, so a failed teardown
> does not claim that the firmware registration has been removed.
>
> Fixes: a8625217a054 ("drivers/perf: riscv: Implement SBI PMU snapshot function")
> Assisted-by: GPT-5
> Signed-off-by: Pengpeng Hou <hppiscas@xxxxxxx>

LGTM.

Reviewed-by: Anup Patel <anup@xxxxxxxxxxxxxx>

Thanks,
Anup

> ---
> The issue was found by our static-analysis tool and manually reviewed.
>
>
> diff --git a/drivers/perf/riscv_pmu_sbi.c b/drivers/perf/riscv_pmu_sbi.c
> index 50220f7b..f38a64c7 100644
> --- a/drivers/perf/riscv_pmu_sbi.c
> +++ b/drivers/perf/riscv_pmu_sbi.c
> @@ -1175,6 +1175,10 @@ static int pmu_sbi_starting_cpu(unsigned int cpu, struct hlist_node *node)
>
>  static int pmu_sbi_dying_cpu(unsigned int cpu, struct hlist_node *node)
>  {
> +       struct riscv_pmu *pmu = hlist_entry_safe(node, struct riscv_pmu, node);
> +       struct cpu_hw_events *cpu_hw_evt = per_cpu_ptr(pmu->hw_events, cpu);
> +       int ret;
> +
>         if (riscv_pmu_use_irq) {
>                 disable_percpu_irq(riscv_pmu_irq);
>         }
> @@ -1182,8 +1186,12 @@ static int pmu_sbi_dying_cpu(unsigned int cpu, struct hlist_node *node)
>         /* Disable all counters access for user mode now */
>         csr_write(CSR_SCOUNTEREN, 0x0);
>
> -       if (sbi_pmu_snapshot_available())
> -               return pmu_sbi_snapshot_disable();
> +       if (sbi_pmu_snapshot_available()) {
> +               ret = pmu_sbi_snapshot_disable();
> +               if (ret)
> +                       return ret;
> +               cpu_hw_evt->snapshot_set_done = false;
> +       }
>
>         return 0;
>  }
>
> base-commit: 4d7d9486c04d917265f64c55bd23b2cc4fe7749c
>