linux-kernel - Re: [PATCH V2 9/9] cpufreq: scmi: Use .register

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20210812042204.lxugyjxa2cyz45ib@vireshk-i7>
Date:   Thu, 12 Aug 2021 09:52:04 +0530
From:   Viresh Kumar <viresh.kumar@...aro.org>
To:     Lukasz Luba <lukasz.luba@....com>
Cc:     Rafael Wysocki <rjw@...ysocki.net>,
        Sudeep Holla <sudeep.holla@....com>,
        Cristian Marussi <cristian.marussi@....com>,
        linux-pm@...r.kernel.org,
        Vincent Guittot <vincent.guittot@...aro.org>,
        Quentin Perret <qperret@...gle.com>,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH V2 9/9] cpufreq: scmi: Use .register_em() callback

On 11-08-21, 17:32, Lukasz Luba wrote:
> 
> 
> On 8/11/21 12:58 PM, Viresh Kumar wrote:
> > Set the newly added .register_em() callback to register with the EM
> > after the cpufreq policy is properly initialized.
> > 
> > Signed-off-by: Viresh Kumar <viresh.kumar@...aro.org>
> > ---
> >   drivers/cpufreq/scmi-cpufreq.c | 55 ++++++++++++++++++++--------------
> >   1 file changed, 32 insertions(+), 23 deletions(-)
> 
> > +static void scmi_cpufreq_register_em(struct cpufreq_policy *policy)
> > +{
> > +	struct em_data_callback em_cb = EM_DATA_CB(scmi_get_cpu_power);
> > +	bool power_scale_mw = perf_ops->power_scale_mw_get(ph);
> > +	struct scmi_data *priv = policy->driver_data;
> > +
> > +	em_dev_register_perf_domain(get_cpu_device(policy->cpu), priv->nr_opp,
> > +				    &em_cb, priv->opp_shared_cpus,
> > +				    power_scale_mw);
> 
> I would free the priv->opp_shared_cpus mask here, since we don't
> need it anymore and memory can be reclaimed.

Yes, we don't need it anymore, but this isn't a good place to undo
what init() has done. Moreover, it is possible that register_em() may
not get called at all, if some error has occurred after init() has
successfully returned. It is always better to use exit() for such
things. It won't hurt a lot to keep this around anyway.

> Don't forget this
> setup would be called N CPUs times, on this per-CPU policy platform.

Yes, but EM will just ignore this call. Though I have made a change
here now to check for non-zero nr_opp to avoid the unnecessary call.

> If freed here, then also there wouldn't be a need to free it in
> scmi_cpufreq_exit() so you can remove it from there.

-- 
viresh