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 bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 0F182CD1284 for ; Tue, 2 Apr 2024 11:07:18 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=lYrddZ7c8lHWdnsdDuxMpZ+E6QaPayPI5s5b0uabiZA=; b=odVTud2/2t8Er+ cJxbkbbWnEHKrYx+iXkmN39FYLZWMt6AjYmWJoIcTPsZ3ULcSWQn+HcZnHcB+37DOsKvzCXLnF1OD VBrjzzf1p7ZE+CnzbTa8toawgAdbtTg4WuMnEWPqN0HxG6Es+nQ0OIG5Vc5h76xE0bh+KYdA7XjcT X1Ze69DLa7uhh7M6L3uATMnoakWfI+0sqbs+cGnIq7bzJIu1hp78DX05abOBUe9Bi4iHhCajnXy88 KJHzkDH6GM2fhCJWhx2v8yym2h/vRSF9E9eArsdAOBzSuo9F8BrY/NNtJv6ttKG8pJxPKUHZ0m1Ki 6Efl4XHDp31h/k5UxTZQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1rrbys-0000000AnzD-1seQ; Tue, 02 Apr 2024 11:07:06 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1rrbyp-0000000Any1-0osa for linux-arm-kernel@lists.infradead.org; Tue, 02 Apr 2024 11:07:05 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id F3879FEC; Tue, 2 Apr 2024 04:07:31 -0700 (PDT) Received: from FVFF77S0Q05N (unknown [10.57.18.33]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 26E383F766; Tue, 2 Apr 2024 04:06:58 -0700 (PDT) Date: Tue, 2 Apr 2024 12:06:52 +0100 From: Mark Rutland To: Dawei Li Cc: will@kernel.org, xueshuai@linux.alibaba.com, renyu.zj@linux.alibaba.com, yangyicong@hisilicon.com, jonathan.cameron@huawei.com, andersson@kernel.org, konrad.dybcio@linaro.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org Subject: Re: [PATCH 1/9] perf/alibaba_uncore_drw: Avoid explicit cpumask var allocation from stack Message-ID: References: <20240402105610.1695644-1-dawei.li@shingroup.cn> <20240402105610.1695644-2-dawei.li@shingroup.cn> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20240402105610.1695644-2-dawei.li@shingroup.cn> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240402_040703_310620_7EF57C9F X-CRM114-Status: GOOD ( 23.35 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Tue, Apr 02, 2024 at 06:56:02PM +0800, Dawei Li wrote: > For CONFIG_CPUMASK_OFFSTACK=y kernel, explicit allocation of cpumask > variable on stack is not recommended since it can cause potential stack > overflow. > > Instead, kernel code should always use *cpumask_var API(s) to allocate > cpumask var in config- neutral way, leaving allocation strategy to > CONFIG_CPUMASK_OFFSTACK. > > Use *cpumask_var API(s) to address it. > > Signed-off-by: Dawei Li > --- > drivers/perf/alibaba_uncore_drw_pmu.c | 13 +++++++++---- > 1 file changed, 9 insertions(+), 4 deletions(-) > > diff --git a/drivers/perf/alibaba_uncore_drw_pmu.c b/drivers/perf/alibaba_uncore_drw_pmu.c > index a9277dcf90ce..251f0a2dee84 100644 > --- a/drivers/perf/alibaba_uncore_drw_pmu.c > +++ b/drivers/perf/alibaba_uncore_drw_pmu.c > @@ -743,25 +743,28 @@ static void ali_drw_pmu_remove(struct platform_device *pdev) > > static int ali_drw_pmu_offline_cpu(unsigned int cpu, struct hlist_node *node) > { > + cpumask_var_t node_online_cpus; > struct ali_drw_pmu_irq *irq; > struct ali_drw_pmu *drw_pmu; > unsigned int target; > int ret; > - cpumask_t node_online_cpus; > > irq = hlist_entry_safe(node, struct ali_drw_pmu_irq, node); > if (cpu != irq->cpu) > return 0; > > - ret = cpumask_and(&node_online_cpus, > + if (!alloc_cpumask_var(&node_online_cpus, GFP_KERNEL)) > + return 0; NAK. This error path leaves things in an incorrect state and this approach does not make sense. Please allocate the cpumasks when we allocate the PMU. Then we can reasonably fail to probe the PMU if we don't have enough memory, and the masks will definitely be accessible in gotplug paths. The same comment applies to the whole series. Mark. > + > + ret = cpumask_and(node_online_cpus, > cpumask_of_node(cpu_to_node(cpu)), cpu_online_mask); > if (ret) > - target = cpumask_any_but(&node_online_cpus, cpu); > + target = cpumask_any_but(node_online_cpus, cpu); > else > target = cpumask_any_but(cpu_online_mask, cpu); > > if (target >= nr_cpu_ids) > - return 0; > + goto __free_cpumask; > > /* We're only reading, but this isn't the place to be involving RCU */ > mutex_lock(&ali_drw_pmu_irqs_lock); > @@ -772,6 +775,8 @@ static int ali_drw_pmu_offline_cpu(unsigned int cpu, struct hlist_node *node) > WARN_ON(irq_set_affinity_hint(irq->irq_num, cpumask_of(target))); > irq->cpu = target; > > +__free_cpumask: > + free_cpumask_var(node_online_cpus); > return 0; > } > > -- > 2.27.0 > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel