Re: [PATCH] drivers/perf: Change WARN_ON() to dev_err() on irq_set_affinity() failure

[Greg KH]

On Mon, Aug 15, 2022 at 05:28:15PM +0800, Yicong Yang wrote:
> From: Yicong Yang <yangyicong@xxxxxxxxxxxxx>
> 
> The WARN_ON() on irq_set_affinity() failure is misused according to the [1]
> and may crash people's box unintentionally. This may also be redundant since
> in the failure case we may also trigger the WARN and dump the stack in the
> perf core[2] for a second time.
> 
> So change the WARN_ON() to dev_err() to just print the failure message.
> 
> [1] https://github.com/torvalds/linux/blob/master/include/asm-generic/bug.h#L74
> [2] https://github.com/torvalds/linux/blob/master/kernel/events/core.c#L313

Please point to git.kernel.org links, we do not control github.com and
it's random mirrors.

> 
> Suggested-by: Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx>
> [https://lore.kernel.org/lkml/YuOi3i0XHV++z1YI@xxxxxxxxx/]
> Signed-off-by: Yicong Yang <yangyicong@xxxxxxxxxxxxx>
> ---
>  drivers/perf/arm-ccn.c                   | 5 +++--
>  drivers/perf/arm_dmc620_pmu.c            | 3 ++-
>  drivers/perf/arm_smmuv3_pmu.c            | 6 ++++--
>  drivers/perf/fsl_imx8_ddr_perf.c         | 3 ++-
>  drivers/perf/hisilicon/hisi_pcie_pmu.c   | 6 ++++--
>  drivers/perf/hisilicon/hisi_uncore_pmu.c | 6 ++++--
>  drivers/perf/qcom_l2_pmu.c               | 8 ++++++--
>  drivers/perf/xgene_pmu.c                 | 6 ++++--
>  8 files changed, 29 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/perf/arm-ccn.c b/drivers/perf/arm-ccn.c
> index 728d13d8e98a..83abd909ba49 100644
> --- a/drivers/perf/arm-ccn.c
> +++ b/drivers/perf/arm-ccn.c
> @@ -1210,8 +1210,9 @@ static int arm_ccn_pmu_offline_cpu(unsigned int cpu, struct hlist_node *node)
>  		return 0;
>  	perf_pmu_migrate_context(&dt->pmu, cpu, target);
>  	dt->cpu = target;
> -	if (ccn->irq)
> -		WARN_ON(irq_set_affinity(ccn->irq, cpumask_of(dt->cpu)));
> +	if (ccn->irq && irq_set_affinity(ccn->irq, cpumask_of(dt->cpu)))
> +		dev_err(ccn->dev, "Failed to set interrupt affinity\n");
> +
>  	return 0;

Why are you returning with no error, if an error happened?

Same everywhere else, you need to explain this in your changelog text.

thanks,

greg k-h