[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <861a6077-a7da-4899-b971-a003d6e2290e@kylinos.cn>
Date: Thu, 4 Sep 2025 15:56:08 +0800
From: Zihuan Zhang <zhangzihuan@...inos.cn>
To: Krzysztof Kozlowski <krzk@...nel.org>,
"Rafael J. Wysocki" <rafael@...nel.org>
Cc: Viresh Kumar <viresh.kumar@...aro.org>,
Catalin Marinas <catalin.marinas@....com>, Will Deacon <will@...nel.org>,
Borislav Petkov <bp@...en8.de>, Dave Hansen <dave.hansen@...ux.intel.com>,
Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>,
Michael Ellerman <mpe@...erman.id.au>, Alim Akhtar
<alim.akhtar@...sung.com>, Thierry Reding <thierry.reding@...il.com>,
MyungJoo Ham <myungjoo.ham@...sung.com>,
Kyungmin Park <kyungmin.park@...sung.com>,
Chanwoo Choi <cw00.choi@...sung.com>,
Jani Nikula <jani.nikula@...ux.intel.com>,
Rodrigo Vivi <rodrigo.vivi@...el.com>, Tvrtko Ursulin
<tursulin@...ulin.net>, David Airlie <airlied@...il.com>,
Simona Vetter <simona@...ll.ch>, Daniel Lezcano <daniel.lezcano@...nel.org>,
Sascha Hauer <s.hauer@...gutronix.de>, Shawn Guo <shawnguo@...nel.org>,
Eduardo Valentin <edubezval@...il.com>, Keerthy <j-keerthy@...com>,
Ben Horgan <ben.horgan@....com>, zhenglifeng <zhenglifeng1@...wei.com>,
Zhang Rui <rui.zhang@...el.com>, Len Brown <lenb@...nel.org>,
Lukasz Luba <lukasz.luba@....com>,
Pengutronix Kernel Team <kernel@...gutronix.de>,
Beata Michalska <beata.michalska@....com>, Fabio Estevam
<festevam@...il.com>, Pavel Machek <pavel@...nel.org>,
Sumit Gupta <sumitg@...dia.com>,
Prasanna Kumar T S M <ptsm@...ux.microsoft.com>,
Sudeep Holla <sudeep.holla@....com>, Yicong Yang <yangyicong@...ilicon.com>,
linux-pm@...r.kernel.org, linux-acpi@...r.kernel.org,
linuxppc-dev@...ts.ozlabs.org, linux-arm-kernel@...ts.infradead.org,
intel-gfx@...ts.freedesktop.org, dri-devel@...ts.freedesktop.org,
imx@...ts.linux.dev, linux-omap@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 10/10] PM: EM: Use scope-based cleanup helper
在 2025/9/3 21:43, Krzysztof Kozlowski 写道:
> On 03/09/2025 15:41, Rafael J. Wysocki wrote:
>>>> em_cpufreq_update_efficiencies(struct device *dev, struct em_perf_state *table)
>>>> {
>>>> struct em_perf_domain *pd = dev->em_pd;
>>>> - struct cpufreq_policy *policy;
>>>> + struct cpufreq_policy *policy __free(put_cpufreq_policy) = NULL;
>>> This is not really correct coding style. Please read how to use
>>> cleanup.h expressed in that header. You should have here proper
>>> constructor or this should be moved. Or this should not be __free()...
>> I gather that this is what you mean (quoted verbatim from cleanup.h)
>>
>> * Given that the "__free(...) = NULL" pattern for variables defined at
>> * the top of the function poses this potential interdependency problem
>> * the recommendation is to always define and assign variables in one
>> * statement and not group variable definitions at the top of the
>> * function when __free() is used.
>>
>> and thanks for pointing this out!
>
> ... and the only exception would be if there is no single constructor,
> but multiple (in if() block). That's not the case here, I think.
>
> Best regards,
> Krzysztof
Sorry, I didn’t fully understand this earlier. In v3 I split the
definition and assignment mainly because the CPU value was obtained
later, so I thought I couldn’t initialize it in one go at the top of
the function. Honestly, it was also for “prettier” style.
After looking at the code Rafael just committed, I realized I can
simply define and assign the variable later in one line, without
needing to separate them. I’ll fix this in the next version.
Thanks for pointing it out!
Powered by blists - more mailing lists