From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id C07DEC2BB41 for ; Tue, 16 Aug 2022 08:13:15 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232518AbiHPINK (ORCPT ); Tue, 16 Aug 2022 04:13:10 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:59022 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231515AbiHPIMK (ORCPT ); Tue, 16 Aug 2022 04:12:10 -0400 Received: from szxga02-in.huawei.com (szxga02-in.huawei.com [45.249.212.188]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 4B857D5998; Mon, 15 Aug 2022 23:26:54 -0700 (PDT) Received: from canpemm500009.china.huawei.com (unknown [172.30.72.53]) by szxga02-in.huawei.com (SkyGuard) with ESMTP id 4M6Lfn3YgtzXdjl; Tue, 16 Aug 2022 14:22:41 +0800 (CST) Received: from [10.67.102.169] (10.67.102.169) by canpemm500009.china.huawei.com (7.192.105.203) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2375.24; Tue, 16 Aug 2022 14:26:52 +0800 CC: , , , , , , , , , , , , , , , , , , Subject: Re: [PATCH] drivers/perf: Change WARN_ON() to dev_err() on irq_set_affinity() failure To: Greg KH References: <20220815092815.11597-1-yangyicong@huawei.com> From: Yicong Yang Message-ID: Date: Tue, 16 Aug 2022 14:26:51 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101 Thunderbird/78.5.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit X-Originating-IP: [10.67.102.169] X-ClientProxiedBy: dggems702-chm.china.huawei.com (10.3.19.179) To canpemm500009.china.huawei.com (7.192.105.203) X-CFilter-Loop: Reflected Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2022/8/15 18:47, Greg KH wrote: > On Mon, Aug 15, 2022 at 05:28:15PM +0800, Yicong Yang wrote: >> From: Yicong Yang >> >> 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. > Got it. Will update with a git.kernel.org link. Thanks for point it out! >> >> Suggested-by: Greg KH >> [https://lore.kernel.org/lkml/YuOi3i0XHV++z1YI@kroah.com/] >> Signed-off-by: Yicong Yang >> --- >> 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. > This patch intends no functional change but to switch the way on the error notification to avoid crash the box. So just keep the current handling behaviour. Will mention this in the commit in v2. I think whether we should actually reponse to the error should be according to the driver. Thanks.