Re: [PATCH v4 5/7] cpufreq: Calculate number of busy CPUs

[Lukasz Majewski <l.majewski@samsung.com>]

On Thu, 27 Jun 2013 15:06:43 +0530, Viresh Kumar wrote:
> On 19 June 2013 22:42, Lukasz Majewski <l.majewski@samsung.com> wrote:
> > In the core governor code, per cpu load value is calculated. This
> > patch uses it to mark processor as a "busy" one, when load value is
> > higher than 90%.
> 
> How can we take this assumption. What about a cpu which is only 70-80%
> busy [*]? 

Do you have any idea of how to precisely set the load threshold?

As a side note:

I've thought about this patch for some time and for me it looks like we
are mixing policy (number of busy CPUs) with abilities, which shall be
provided by the driver (boost). 
Additionally, we can only roughly "estimate" [*] when boost shall run
and when it shall be turned off.

I think that, we shall leave this management [*] to the thermal
framework. This framework is designed exactly to protect from over
heating (it uses the same freq_table for passive CPU cooling) with
several trip points -> e.g. 40 deg (disable boost), 75 deg (impose max
freq as 1.0 GHz to cool down, 90 deg (shutdown immediately).
Please refer to PATH v4 7/7.

Unfortunately, since we dropped Kconfig flag for BOOST we cannot
impose "select THERMAL_FRAMEWORK", when flag for BOOST is enabled at
Kconfig.

Ideally kernel shall not even build when CONFIG_CPUFREQ_BOOST Kconfig
flag is set and thermal for target architecture is not correctly
configured (including proper trip points).




> So, we would be running at around 70% of policy->max.. Now at
> this point you want to enable boost frequency and so that will happen
> for all cores... Wouldn't that burn your chip?

As I've written above, we can only estimate here. Even with the same
manufacturer at one SoC revision BOOST will work with 95% of policy max
but the other revision will work stable only with 70%.

> 
> > New cpufreq sysfs attribute is created (busy_cpus). It is read only
> > and provides information about number of actually busy CPU.
> 
> Not required.

This attribute seems helpful for debug.

> 
> > diff --git a/drivers/cpufreq/cpufreq_governor.c
> > b/drivers/cpufreq/cpufreq_governor.c index 077cea7..3402533 100644
> > --- a/drivers/cpufreq/cpufreq_governor.c
> > +++ b/drivers/cpufreq/cpufreq_governor.c
> > @@ -148,6 +148,7 @@ void dbs_check_cpu(struct dbs_data *dbs_data,
> > int cpu) continue;
> >
> >                 load = 100 * (wall_time - idle_time) / wall_time;
> > +               cpufreq_set_busy_cpu(j, load > 90 ? 1 : 0);
> 
> do this only when boost is enabled.

Please read above side note.

> 
> >                 if (dbs_data->cdata->governor == GOV_ONDEMAND) {
> >                         int freq_avg =
> > __cpufreq_driver_getavg(policy, j);


-- 
Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group