lkml.org 
[lkml]   [2013]   [Jan]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH 1/5] cpufreq: handle SW coordinated CPUs
From
I am starting to follow cpufreq patches religiously now and so have to come
back to this old thread due to some crash we got :)

Its still not pushed upstream, so better to get it resolved before 3.9.

On Thu, Dec 27, 2012 at 8:25 PM, Fabio Baltieri
<fabio.baltieri@linaro.org> wrote:

> diff --git a/drivers/cpufreq/cpufreq_governor.c b/drivers/cpufreq/cpufreq_governor.c

> static inline void dbs_timer_init(struct dbs_data *dbs_data,
> - struct cpu_dbs_common_info *cdbs, unsigned int sampling_rate)
> + struct cpu_dbs_common_info *cdbs,
> + unsigned int sampling_rate,
> + int cpu)
> {
> int delay = delay_for_sampling_rate(sampling_rate);
> + struct cpu_dbs_common_info *cdbs_local = dbs_data->get_cpu_cdbs(cpu);

I couldn't understand the real need for this, as it should really give
back the same
pointer pointed out by: cdbs and hence no need of cpu in params too..

I may be wrong here :)

>
> - INIT_DEFERRABLE_WORK(&cdbs->work, dbs_data->gov_dbs_timer);
> - schedule_delayed_work_on(cdbs->cpu, &cdbs->work, delay);
> + schedule_delayed_work_on(cpu, &cdbs_local->work, delay);
> }
>
> static inline void dbs_timer_exit(struct cpu_dbs_common_info *cdbs)
> @@ -217,6 +227,10 @@ int cpufreq_governor_dbs(struct dbs_data *dbs_data,
> if (ignore_nice)
> j_cdbs->prev_cpu_nice =
> kcpustat_cpu(j).cpustat[CPUTIME_NICE];
> +
> + mutex_init(&j_cdbs->timer_mutex);
> + INIT_DEFERRABLE_WORK(&j_cdbs->work,
> + dbs_data->gov_dbs_timer);
> }
>
> /*
> @@ -275,15 +289,33 @@ second_time:
> }
> mutex_unlock(&dbs_data->mutex);
>
> - mutex_init(&cpu_cdbs->timer_mutex);
> - dbs_timer_init(dbs_data, cpu_cdbs, *sampling_rate);
> + if (dbs_sw_coordinated_cpus(cpu_cdbs)) {
> + for_each_cpu(j, policy->cpus) {
> + struct cpu_dbs_common_info *j_cdbs;
> +
> + j_cdbs = dbs_data->get_cpu_cdbs(j);
> + dbs_timer_init(dbs_data, j_cdbs,
> + *sampling_rate, j);
> + }
> + } else {
> + dbs_timer_init(dbs_data, cpu_cdbs, *sampling_rate, cpu);
> + }

do you really need this else part? In case of uniprocessor systems also, the if
block should be enough. Isn't it?

> break;
>
> case CPUFREQ_GOV_STOP:
> if (dbs_data->governor == GOV_CONSERVATIVE)
> cs_dbs_info->enable = 0;
>
> - dbs_timer_exit(cpu_cdbs);
> + if (dbs_sw_coordinated_cpus(cpu_cdbs)) {
> + for_each_cpu(j, policy->cpus) {
> + struct cpu_dbs_common_info *j_cdbs;
> +
> + j_cdbs = dbs_data->get_cpu_cdbs(j);
> + dbs_timer_exit(j_cdbs);
> + }
> + } else {
> + dbs_timer_exit(cpu_cdbs);
> + }

ditto.

> mutex_lock(&dbs_data->mutex);
> mutex_destroy(&cpu_cdbs->timer_mutex);
> diff --git a/drivers/cpufreq/cpufreq_governor.h b/drivers/cpufreq/cpufreq_governor.h
> index f661654..5bf6fb8 100644
> --- a/drivers/cpufreq/cpufreq_governor.h
> +++ b/drivers/cpufreq/cpufreq_governor.h
> @@ -171,6 +171,7 @@ static inline int delay_for_sampling_rate(unsigned int sampling_rate)
>
> u64 get_cpu_idle_time(unsigned int cpu, u64 *wall);
> void dbs_check_cpu(struct dbs_data *dbs_data, int cpu);
> +bool dbs_sw_coordinated_cpus(struct cpu_dbs_common_info *cdbs);
> int cpufreq_governor_dbs(struct dbs_data *dbs_data,
> struct cpufreq_policy *policy, unsigned int event);
> #endif /* _CPUFREQ_GOVERNER_H */
> diff --git a/drivers/cpufreq/cpufreq_ondemand.c b/drivers/cpufreq/cpufreq_ondemand.c
> index 7731f7c..93bb56d 100644
> --- a/drivers/cpufreq/cpufreq_ondemand.c
> +++ b/drivers/cpufreq/cpufreq_ondemand.c
> @@ -243,7 +243,8 @@ static void od_dbs_timer(struct work_struct *work)
> }
> }
>
> - schedule_delayed_work_on(cpu, &dbs_info->cdbs.work, delay);
> + schedule_delayed_work_on(smp_processor_id(), &dbs_info->cdbs.work,
> + delay);
> mutex_unlock(&dbs_info->cdbs.timer_mutex);
> }
>
> --
> 1.7.12.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe cpufreq" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html


\
 
 \ /
  Last update: 2013-01-30 09:21    [W:0.118 / U:1.780 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site