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 mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id D92A1C433F5 for ; Sat, 13 Nov 2021 18:55:39 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id BC52C611EE for ; Sat, 13 Nov 2021 18:55:39 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S236006AbhKMS6a (ORCPT ); Sat, 13 Nov 2021 13:58:30 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:49358 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230363AbhKMS6a (ORCPT ); Sat, 13 Nov 2021 13:58:30 -0500 Received: from mail-oi1-x233.google.com (mail-oi1-x233.google.com [IPv6:2607:f8b0:4864:20::233]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id BD480C061767 for ; Sat, 13 Nov 2021 10:55:37 -0800 (PST) Received: by mail-oi1-x233.google.com with SMTP id m6so25139141oim.2 for ; Sat, 13 Nov 2021 10:55:37 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=aEaoQk4S2VEKJUWN86163Buqvqi3IEqLz/+vozzoOP4=; b=TW/Ad79WaYF4EY3jQBBipTw3TfDqUwRUflKXpjIunx6UOui1ILIWRVQBUhVDCCm/et +tJBp2SGpjrFmHJVw//MrMWKsGahuwAeSyNkqlV24EZt9YSwUi0Qpnn4+W3DIZERjNPb ubrqU5qqTw4VP2j86wlzb188PRvKcudMgRncMNJXWtcfvQXm6Bk68/EA8Hyq5sH98mBi NcAcCxW8G/JCVcJtC4kRIbgqkra0IbefUiCqN4eIovjHQfJUE2y6Dnr8AjOyQwkbp5Ys Pwp/LitZ939vqRYOBD70ru9jttzvsR8sWt49Qcjr7tueohKYxD7rSnWQ84hSXUUjFBT1 POyQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=aEaoQk4S2VEKJUWN86163Buqvqi3IEqLz/+vozzoOP4=; b=QhU4phigqCOppWj33BBfZJgjnoum+AvNRy5yLxFHvLkcRrDkYq1vOzIfSpR2v5ofuO Gc8CcjCdhB7t3QZCi565nRH3Qsii/T5GthTH1A9c36TRwZWLPRPSqk4zWC+z7IpFP/vq JYHxoxVtLcY5XjF92THBVVQtS/cGxYD8/l/qMMOiHw2Or1YxvFhsq1a2L9LoLXgc56cS KIEAfeCse+XtBl5u567OppMAG7qTPtST/VoUX2iGD008ejnSXbRvkA5wx5cyC7cIzPXA GK6Nyp1Dby/Qcl1TlF5Sq2Es9ObQ1Vud/tYYVDISwSJGMpMp4b/bhLewg9L7NSJXO3KF kc0Q== X-Gm-Message-State: AOAM532a9DNXq9+HA6g1DibQ0Xf9s+UQozxOAYB9EuHxJaAeDpNSO7t9 CJ6dZtkDsmQ6dXdralF/jHIFVA== X-Google-Smtp-Source: ABdhPJxPEAu48dyuGhj2JUv8X/eJPmCFi7hIHbunZ2UyNUnWMWOO3GwDSrehXn9dqGOnXR99HpouBQ== X-Received: by 2002:aca:b386:: with SMTP id c128mr35777005oif.161.1636829737116; Sat, 13 Nov 2021 10:55:37 -0800 (PST) Received: from builder.lan (104-57-184-186.lightspeed.austtx.sbcglobal.net. [104.57.184.186]) by smtp.gmail.com with ESMTPSA id s2sm589548otr.69.2021.11.13.10.55.36 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 13 Nov 2021 10:55:36 -0800 (PST) Date: Sat, 13 Nov 2021 12:55:32 -0600 From: Bjorn Andersson To: Dmitry Baryshkov Cc: Vladimir Zapolskiy , Viresh Kumar , "Rafael J. Wysocki" , Thara Gopinath , linux-arm-msm@vger.kernel.org, linux-pm@vger.kernel.org, Ard Biesheuvel , Andy Gross , Steev Klimaszewski Subject: Re: [PATCH 1/3][RESEND] cpufreq: qcom-cpufreq-hw: Avoid stack buffer for IRQ name Message-ID: References: <20211111154808.2024808-1-vladimir.zapolskiy@linaro.org> <20211111154808.2024808-2-vladimir.zapolskiy@linaro.org> <3711b990-a00a-9fce-0e08-657070d0a990@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <3711b990-a00a-9fce-0e08-657070d0a990@linaro.org> Precedence: bulk List-ID: X-Mailing-List: linux-arm-msm@vger.kernel.org On Fri 12 Nov 14:16 CST 2021, Dmitry Baryshkov wrote: > On 11/11/2021 18:48, Vladimir Zapolskiy wrote: > > From: Ard Biesheuvel > > > > Registering an IRQ requires the string buffer containing the name to > > remain allocated, as the name is not copied into another buffer. > > > > So let's add a irq_name field to the data struct instead, which is > > guaranteed to have the appropriate lifetime. > > > > Cc: Thara Gopinath > > Cc: Bjorn Andersson > > Cc: Andy Gross > > Cc: linux-arm-msm@vger.kernel.org > > Signed-off-by: Ard Biesheuvel > > Reviewed-by: Thara Gopinath > > Tested-by: Steev Klimaszewski > > Signed-off-by: Vladimir Zapolskiy > > --- > > vzapolskiy: rebased, added all collected tags and resend the change from Ard > > > > drivers/cpufreq/qcom-cpufreq-hw.c | 8 ++++---- > > 1 file changed, 4 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c b/drivers/cpufreq/qcom-cpufreq-hw.c > > index a2be0df7e174..3b5835336658 100644 > > --- a/drivers/cpufreq/qcom-cpufreq-hw.c > > +++ b/drivers/cpufreq/qcom-cpufreq-hw.c > > @@ -46,6 +46,7 @@ struct qcom_cpufreq_data { > > */ > > struct mutex throttle_lock; > > int throttle_irq; > > + char irq_name[15]; > > bool cancel_throttle; > > struct delayed_work throttle_work; > > struct cpufreq_policy *policy; > > @@ -375,7 +376,6 @@ static int qcom_cpufreq_hw_lmh_init(struct cpufreq_policy *policy, int index) > > { > > struct qcom_cpufreq_data *data = policy->driver_data; > > struct platform_device *pdev = cpufreq_get_driver_data(); > > - char irq_name[15]; > > int ret; > > /* > > @@ -392,11 +392,11 @@ static int qcom_cpufreq_hw_lmh_init(struct cpufreq_policy *policy, int index) > > mutex_init(&data->throttle_lock); > > INIT_DEFERRABLE_WORK(&data->throttle_work, qcom_lmh_dcvs_poll); > > - snprintf(irq_name, sizeof(irq_name), "dcvsh-irq-%u", policy->cpu); > > + snprintf(data->irq_name, sizeof(data->irq_name), "dcvsh-irq-%u", policy->cpu); > > It might be easier to use devm_kasprintf() here. > Yes, in itself that would be nice. But this function might be called > 1 times per device, in which case we would end up with N unused copies of the string on the heap, until the device is removed. I'm working on some patches where these things are allocated at probe time, with those in place it makes more sense to just devm_kasprintf() this and "forget" about the pointer. Regards, Bjorn > > ret = request_threaded_irq(data->throttle_irq, NULL, qcom_lmh_dcvs_handle_irq, > > - IRQF_ONESHOT, irq_name, data); > > + IRQF_ONESHOT, data->irq_name, data); > > if (ret) { > > - dev_err(&pdev->dev, "Error registering %s: %d\n", irq_name, ret); > > + dev_err(&pdev->dev, "Error registering %s: %d\n", data->irq_name, ret); > > return 0; > > } > > > > > -- > With best wishes > Dmitry