Re: [PATCH] cpufreq: userspace: Simplify governor

2013-06-18 Thread Rafael J. Wysocki 
On Tuesday, June 18, 2013 10:20:33 AM Viresh Kumar wrote:
> On 5 June 2013 18:38, Viresh Kumar  wrote:
> > To be honest with the amount of experience I have now, my log was
> > poor :(
> >
> > I have used following log in the attached patch:
> >
> > Subject: [PATCH] cpufreq: userspace: Simplify governor
> >
> > Userspace governor has got more code than what it needs for its functioning.
> > Lets simplify it. Portions of code removed are:
> > - Extra header files which aren't required anymore (rearranged them as well)
> > - cpu_{max|min|cur|set}_freq, as they are always same as 
> > policy->{max|min|cur}
> > - userspace_cpufreq_notifier_block as we don't need to set cpu_cur_freq 
> > anymore
> > - cpus_using_userspace_governor as it was for notifier code
> >
> > Signed-off-by: Viresh Kumar 
> 
> Hi Rafael,
> 
> Are you happy with the log & description of this patch now?

Yes, I am.  I'm hoping to get to the pending patches later today.

Thanks,
Rafael

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] cpufreq: userspace: Simplify governor

2013-06-18 Thread Rafael J. Wysocki 
On Tuesday, June 18, 2013 10:20:33 AM Viresh Kumar wrote:
 On 5 June 2013 18:38, Viresh Kumar viresh.ku...@linaro.org wrote:
  To be honest with the amount of experience I have now, my log was
  poor :(
 
  I have used following log in the attached patch:
 
  Subject: [PATCH] cpufreq: userspace: Simplify governor
 
  Userspace governor has got more code than what it needs for its functioning.
  Lets simplify it. Portions of code removed are:
  - Extra header files which aren't required anymore (rearranged them as well)
  - cpu_{max|min|cur|set}_freq, as they are always same as 
  policy-{max|min|cur}
  - userspace_cpufreq_notifier_block as we don't need to set cpu_cur_freq 
  anymore
  - cpus_using_userspace_governor as it was for notifier code
 
  Signed-off-by: Viresh Kumar viresh.ku...@linaro.org
 
 Hi Rafael,
 
 Are you happy with the log  description of this patch now?

Yes, I am.  I'm hoping to get to the pending patches later today.

Thanks,
Rafael

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] cpufreq: userspace: Simplify governor

2013-06-17 Thread Viresh Kumar 
On 5 June 2013 18:38, Viresh Kumar  wrote:
> To be honest with the amount of experience I have now, my log was
> poor :(
>
> I have used following log in the attached patch:
>
> Subject: [PATCH] cpufreq: userspace: Simplify governor
>
> Userspace governor has got more code than what it needs for its functioning.
> Lets simplify it. Portions of code removed are:
> - Extra header files which aren't required anymore (rearranged them as well)
> - cpu_{max|min|cur|set}_freq, as they are always same as policy->{max|min|cur}
> - userspace_cpufreq_notifier_block as we don't need to set cpu_cur_freq 
> anymore
> - cpus_using_userspace_governor as it was for notifier code
>
> Signed-off-by: Viresh Kumar 

Hi Rafael,

Are you happy with the log & description of this patch now?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] cpufreq: userspace: Simplify governor

2013-06-17 Thread Viresh Kumar 
On 5 June 2013 18:38, Viresh Kumar viresh.ku...@linaro.org wrote:
 To be honest with the amount of experience I have now, my log was
 poor :(

 I have used following log in the attached patch:

 Subject: [PATCH] cpufreq: userspace: Simplify governor

 Userspace governor has got more code than what it needs for its functioning.
 Lets simplify it. Portions of code removed are:
 - Extra header files which aren't required anymore (rearranged them as well)
 - cpu_{max|min|cur|set}_freq, as they are always same as policy-{max|min|cur}
 - userspace_cpufreq_notifier_block as we don't need to set cpu_cur_freq 
 anymore
 - cpus_using_userspace_governor as it was for notifier code

 Signed-off-by: Viresh Kumar viresh.ku...@linaro.org

Hi Rafael,

Are you happy with the log  description of this patch now?
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] cpufreq: userspace: Simplify governor

2013-06-08 Thread Xiaoguang Chen 
2013/6/5 Viresh Kumar :
> On 5 June 2013 18:07, Rafael J. Wysocki  wrote:
>> On Wednesday, June 05, 2013 02:34:33 PM Viresh Kumar wrote:
>>> Userspace governor has got more code than what it needs for its functioning.
>>> Lets simplify it.
>>
>> OK
>>
>> Why exactly is the code you're removing unnecessary?
>>
>>> Over that it will fix issues in cpufreq_governor_userspace(), which isn't 
>>> doing
>>> right things in START/STOP.
>>
>> What exactly is the problem?
>
> I sent a reply now to the problem reported by Xiaoguang as I don't feel
> now there is a problem :(
>
Yes, I think the problem will disappear since the related code is
deleted. the original code path will not be executed.

>>> It is working per-cpu currently whereas it just
>>> required to manage policy->cpu.
>>>
>>> Reported-by: Xiaoguang Chen 
>>> Signed-off-by: Viresh Kumar 
>>> ---
>>> @Rafael:
>>>
>>> I don't know why this code was initially added. Please let me know if I am 
>>> doing
>>> something stupid.
>>>
>>> Also, please apply it as a fix for 3.10 as it is broken recently in 3.9.
>>
>> I'd love to, but I need answers to the above questions before I do that.
>
> To be honest with the amount of experience I have now, my log was
> poor :(
>
> I have used following log in the attached patch:
>
> Subject: [PATCH] cpufreq: userspace: Simplify governor
>
> Userspace governor has got more code than what it needs for its functioning.
> Lets simplify it. Portions of code removed are:
> - Extra header files which aren't required anymore (rearranged them as well)
> - cpu_{max|min|cur|set}_freq, as they are always same as policy->{max|min|cur}
> - userspace_cpufreq_notifier_block as we don't need to set cpu_cur_freq 
> anymore
> - cpus_using_userspace_governor as it was for notifier code
>
> Signed-off-by: Viresh Kumar 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] cpufreq: userspace: Simplify governor

2013-06-08 Thread Xiaoguang Chen 
2013/6/5 Viresh Kumar viresh.ku...@linaro.org:
 On 5 June 2013 18:07, Rafael J. Wysocki r...@sisk.pl wrote:
 On Wednesday, June 05, 2013 02:34:33 PM Viresh Kumar wrote:
 Userspace governor has got more code than what it needs for its functioning.
 Lets simplify it.

 OK

 Why exactly is the code you're removing unnecessary?

 Over that it will fix issues in cpufreq_governor_userspace(), which isn't 
 doing
 right things in START/STOP.

 What exactly is the problem?

 I sent a reply now to the problem reported by Xiaoguang as I don't feel
 now there is a problem :(

Yes, I think the problem will disappear since the related code is
deleted. the original code path will not be executed.

 It is working per-cpu currently whereas it just
 required to manage policy-cpu.

 Reported-by: Xiaoguang Chen chenxg.marv...@gmail.com
 Signed-off-by: Viresh Kumar viresh.ku...@linaro.org
 ---
 @Rafael:

 I don't know why this code was initially added. Please let me know if I am 
 doing
 something stupid.

 Also, please apply it as a fix for 3.10 as it is broken recently in 3.9.

 I'd love to, but I need answers to the above questions before I do that.

 To be honest with the amount of experience I have now, my log was
 poor :(

 I have used following log in the attached patch:

 Subject: [PATCH] cpufreq: userspace: Simplify governor

 Userspace governor has got more code than what it needs for its functioning.
 Lets simplify it. Portions of code removed are:
 - Extra header files which aren't required anymore (rearranged them as well)
 - cpu_{max|min|cur|set}_freq, as they are always same as policy-{max|min|cur}
 - userspace_cpufreq_notifier_block as we don't need to set cpu_cur_freq 
 anymore
 - cpus_using_userspace_governor as it was for notifier code

 Signed-off-by: Viresh Kumar viresh.ku...@linaro.org
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] cpufreq: userspace: Simplify governor

2013-06-05 Thread Viresh Kumar 
On 5 June 2013 18:07, Rafael J. Wysocki  wrote:
> On Wednesday, June 05, 2013 02:34:33 PM Viresh Kumar wrote:
>> Userspace governor has got more code than what it needs for its functioning.
>> Lets simplify it.
>
> OK
>
> Why exactly is the code you're removing unnecessary?
>
>> Over that it will fix issues in cpufreq_governor_userspace(), which isn't 
>> doing
>> right things in START/STOP.
>
> What exactly is the problem?

I sent a reply now to the problem reported by Xiaoguang as I don't feel
now there is a problem :(

>> It is working per-cpu currently whereas it just
>> required to manage policy->cpu.
>>
>> Reported-by: Xiaoguang Chen 
>> Signed-off-by: Viresh Kumar 
>> ---
>> @Rafael:
>>
>> I don't know why this code was initially added. Please let me know if I am 
>> doing
>> something stupid.
>>
>> Also, please apply it as a fix for 3.10 as it is broken recently in 3.9.
>
> I'd love to, but I need answers to the above questions before I do that.

To be honest with the amount of experience I have now, my log was
poor :(

I have used following log in the attached patch:

Subject: [PATCH] cpufreq: userspace: Simplify governor

Userspace governor has got more code than what it needs for its functioning.
Lets simplify it. Portions of code removed are:
- Extra header files which aren't required anymore (rearranged them as well)
- cpu_{max|min|cur|set}_freq, as they are always same as policy->{max|min|cur}
- userspace_cpufreq_notifier_block as we don't need to set cpu_cur_freq anymore
- cpus_using_userspace_governor as it was for notifier code

Signed-off-by: Viresh Kumar 


0001-cpufreq-userspace-Simplify-governor.patch
Description: Binary data

Re: [PATCH] cpufreq: userspace: Simplify governor

2013-06-05 Thread Rafael J. Wysocki 
On Wednesday, June 05, 2013 02:34:33 PM Viresh Kumar wrote:
> Userspace governor has got more code than what it needs for its functioning.
> Lets simplify it.

OK

Why exactly is the code you're removing unnecessary?

> Over that it will fix issues in cpufreq_governor_userspace(), which isn't 
> doing
> right things in START/STOP.

What exactly is the problem?

> It is working per-cpu currently whereas it just
> required to manage policy->cpu.
> 
> Reported-by: Xiaoguang Chen 
> Signed-off-by: Viresh Kumar 
> ---
> @Rafael:
> 
> I don't know why this code was initially added. Please let me know if I am 
> doing
> something stupid.
> 
> Also, please apply it as a fix for 3.10 as it is broken recently in 3.9.

I'd love to, but I need answers to the above questions before I do that.

Thanks,
Rafael


>  drivers/cpufreq/cpufreq_userspace.c | 108 
> 
>  1 file changed, 12 insertions(+), 96 deletions(-)
> 
> diff --git a/drivers/cpufreq/cpufreq_userspace.c 
> b/drivers/cpufreq/cpufreq_userspace.c
> index bbeb9c0..5dc77b7 100644
> --- a/drivers/cpufreq/cpufreq_userspace.c
> +++ b/drivers/cpufreq/cpufreq_userspace.c
> @@ -13,55 +13,13 @@
>  
>  #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>  
> -#include 
> -#include 
> -#include 
> -#include 
> -#include 
> -#include 
>  #include 
> -#include 
> -#include 
> -#include 
> -#include 
> +#include 
> +#include 
>  #include 
>  
> -/**
> - * A few values needed by the userspace governor
> - */
> -static DEFINE_PER_CPU(unsigned int, cpu_max_freq);
> -static DEFINE_PER_CPU(unsigned int, cpu_min_freq);
> -static DEFINE_PER_CPU(unsigned int, cpu_cur_freq); /* current CPU freq */
> -static DEFINE_PER_CPU(unsigned int, cpu_set_freq); /* CPU freq desired by
> - userspace */
>  static DEFINE_PER_CPU(unsigned int, cpu_is_managed);
> -
>  static DEFINE_MUTEX(userspace_mutex);
> -static int cpus_using_userspace_governor;
> -
> -/* keep track of frequency transitions */
> -static int
> -userspace_cpufreq_notifier(struct notifier_block *nb, unsigned long val,
> - void *data)
> -{
> - struct cpufreq_freqs *freq = data;
> -
> - if (!per_cpu(cpu_is_managed, freq->cpu))
> - return 0;
> -
> - if (val == CPUFREQ_POSTCHANGE) {
> - pr_debug("saving cpu_cur_freq of cpu %u to be %u kHz\n",
> - freq->cpu, freq->new);
> - per_cpu(cpu_cur_freq, freq->cpu) = freq->new;
> - }
> -
> - return 0;
> -}
> -
> -static struct notifier_block userspace_cpufreq_notifier_block = {
> - .notifier_call  = userspace_cpufreq_notifier
> -};
> -
>  
>  /**
>   * cpufreq_set - set the CPU frequency
> @@ -80,13 +38,6 @@ static int cpufreq_set(struct cpufreq_policy *policy, 
> unsigned int freq)
>   if (!per_cpu(cpu_is_managed, policy->cpu))
>   goto err;
>  
> - per_cpu(cpu_set_freq, policy->cpu) = freq;
> -
> - if (freq < per_cpu(cpu_min_freq, policy->cpu))
> - freq = per_cpu(cpu_min_freq, policy->cpu);
> - if (freq > per_cpu(cpu_max_freq, policy->cpu))
> - freq = per_cpu(cpu_max_freq, policy->cpu);
> -
>   /*
>* We're safe from concurrent calls to ->target() here
>* as we hold the userspace_mutex lock. If we were calling
> @@ -107,7 +58,7 @@ static int cpufreq_set(struct cpufreq_policy *policy, 
> unsigned int freq)
>  
>  static ssize_t show_speed(struct cpufreq_policy *policy, char *buf)
>  {
> - return sprintf(buf, "%u\n", per_cpu(cpu_cur_freq, policy->cpu));
> + return sprintf(buf, "%u\n", policy->cur);
>  }
>  
>  static int cpufreq_governor_userspace(struct cpufreq_policy *policy,
> @@ -119,66 +70,31 @@ static int cpufreq_governor_userspace(struct 
> cpufreq_policy *policy,
>   switch (event) {
>   case CPUFREQ_GOV_START:
>   BUG_ON(!policy->cur);
> - mutex_lock(_mutex);
> -
> - if (cpus_using_userspace_governor == 0) {
> - cpufreq_register_notifier(
> - _cpufreq_notifier_block,
> - CPUFREQ_TRANSITION_NOTIFIER);
> - }
> - cpus_using_userspace_governor++;
> + pr_debug("started managing cpu %u\n", cpu);
>  
> + mutex_lock(_mutex);
>   per_cpu(cpu_is_managed, cpu) = 1;
> - per_cpu(cpu_min_freq, cpu) = policy->min;
> - per_cpu(cpu_max_freq, cpu) = policy->max;
> - per_cpu(cpu_cur_freq, cpu) = policy->cur;
> - per_cpu(cpu_set_freq, cpu) = policy->cur;
> - pr_debug("managing cpu %u started "
> - "(%u - %u kHz, currently %u kHz)\n",
> - cpu,
> - per_cpu(cpu_min_freq, cpu),
> - per_cpu(cpu_max_freq, cpu),
> - per_cpu(cpu_cur_freq, cpu));
> -
>   mutex_unlock(_mutex);

Re: [PATCH] cpufreq: userspace: Simplify governor

2013-06-05 Thread Rafael J. Wysocki 
On Wednesday, June 05, 2013 02:34:33 PM Viresh Kumar wrote:
 Userspace governor has got more code than what it needs for its functioning.
 Lets simplify it.

OK

Why exactly is the code you're removing unnecessary?

 Over that it will fix issues in cpufreq_governor_userspace(), which isn't 
 doing
 right things in START/STOP.

What exactly is the problem?

 It is working per-cpu currently whereas it just
 required to manage policy-cpu.
 
 Reported-by: Xiaoguang Chen chenxg.marv...@gmail.com
 Signed-off-by: Viresh Kumar viresh.ku...@linaro.org
 ---
 @Rafael:
 
 I don't know why this code was initially added. Please let me know if I am 
 doing
 something stupid.
 
 Also, please apply it as a fix for 3.10 as it is broken recently in 3.9.

I'd love to, but I need answers to the above questions before I do that.

Thanks,
Rafael


  drivers/cpufreq/cpufreq_userspace.c | 108 
 
  1 file changed, 12 insertions(+), 96 deletions(-)
 
 diff --git a/drivers/cpufreq/cpufreq_userspace.c 
 b/drivers/cpufreq/cpufreq_userspace.c
 index bbeb9c0..5dc77b7 100644
 --- a/drivers/cpufreq/cpufreq_userspace.c
 +++ b/drivers/cpufreq/cpufreq_userspace.c
 @@ -13,55 +13,13 @@
  
  #define pr_fmt(fmt) KBUILD_MODNAME :  fmt
  
 -#include linux/kernel.h
 -#include linux/module.h
 -#include linux/smp.h
 -#include linux/init.h
 -#include linux/spinlock.h
 -#include linux/interrupt.h
  #include linux/cpufreq.h
 -#include linux/cpu.h
 -#include linux/types.h
 -#include linux/fs.h
 -#include linux/sysfs.h
 +#include linux/init.h
 +#include linux/module.h
  #include linux/mutex.h
  
 -/**
 - * A few values needed by the userspace governor
 - */
 -static DEFINE_PER_CPU(unsigned int, cpu_max_freq);
 -static DEFINE_PER_CPU(unsigned int, cpu_min_freq);
 -static DEFINE_PER_CPU(unsigned int, cpu_cur_freq); /* current CPU freq */
 -static DEFINE_PER_CPU(unsigned int, cpu_set_freq); /* CPU freq desired by
 - userspace */
  static DEFINE_PER_CPU(unsigned int, cpu_is_managed);
 -
  static DEFINE_MUTEX(userspace_mutex);
 -static int cpus_using_userspace_governor;
 -
 -/* keep track of frequency transitions */
 -static int
 -userspace_cpufreq_notifier(struct notifier_block *nb, unsigned long val,
 - void *data)
 -{
 - struct cpufreq_freqs *freq = data;
 -
 - if (!per_cpu(cpu_is_managed, freq-cpu))
 - return 0;
 -
 - if (val == CPUFREQ_POSTCHANGE) {
 - pr_debug(saving cpu_cur_freq of cpu %u to be %u kHz\n,
 - freq-cpu, freq-new);
 - per_cpu(cpu_cur_freq, freq-cpu) = freq-new;
 - }
 -
 - return 0;
 -}
 -
 -static struct notifier_block userspace_cpufreq_notifier_block = {
 - .notifier_call  = userspace_cpufreq_notifier
 -};
 -
  
  /**
   * cpufreq_set - set the CPU frequency
 @@ -80,13 +38,6 @@ static int cpufreq_set(struct cpufreq_policy *policy, 
 unsigned int freq)
   if (!per_cpu(cpu_is_managed, policy-cpu))
   goto err;
  
 - per_cpu(cpu_set_freq, policy-cpu) = freq;
 -
 - if (freq  per_cpu(cpu_min_freq, policy-cpu))
 - freq = per_cpu(cpu_min_freq, policy-cpu);
 - if (freq  per_cpu(cpu_max_freq, policy-cpu))
 - freq = per_cpu(cpu_max_freq, policy-cpu);
 -
   /*
* We're safe from concurrent calls to -target() here
* as we hold the userspace_mutex lock. If we were calling
 @@ -107,7 +58,7 @@ static int cpufreq_set(struct cpufreq_policy *policy, 
 unsigned int freq)
  
  static ssize_t show_speed(struct cpufreq_policy *policy, char *buf)
  {
 - return sprintf(buf, %u\n, per_cpu(cpu_cur_freq, policy-cpu));
 + return sprintf(buf, %u\n, policy-cur);
  }
  
  static int cpufreq_governor_userspace(struct cpufreq_policy *policy,
 @@ -119,66 +70,31 @@ static int cpufreq_governor_userspace(struct 
 cpufreq_policy *policy,
   switch (event) {
   case CPUFREQ_GOV_START:
   BUG_ON(!policy-cur);
 - mutex_lock(userspace_mutex);
 -
 - if (cpus_using_userspace_governor == 0) {
 - cpufreq_register_notifier(
 - userspace_cpufreq_notifier_block,
 - CPUFREQ_TRANSITION_NOTIFIER);
 - }
 - cpus_using_userspace_governor++;
 + pr_debug(started managing cpu %u\n, cpu);
  
 + mutex_lock(userspace_mutex);
   per_cpu(cpu_is_managed, cpu) = 1;
 - per_cpu(cpu_min_freq, cpu) = policy-min;
 - per_cpu(cpu_max_freq, cpu) = policy-max;
 - per_cpu(cpu_cur_freq, cpu) = policy-cur;
 - per_cpu(cpu_set_freq, cpu) = policy-cur;
 - pr_debug(managing cpu %u started 
 - (%u - %u kHz, currently %u kHz)\n,
 - cpu,
 - per_cpu(cpu_min_freq, cpu),
 - per_cpu(cpu_max_freq, cpu),
 -

Re: [PATCH] cpufreq: userspace: Simplify governor

2013-06-05 Thread Viresh Kumar 
On 5 June 2013 18:07, Rafael J. Wysocki r...@sisk.pl wrote:
 On Wednesday, June 05, 2013 02:34:33 PM Viresh Kumar wrote:
 Userspace governor has got more code than what it needs for its functioning.
 Lets simplify it.

 OK

 Why exactly is the code you're removing unnecessary?

 Over that it will fix issues in cpufreq_governor_userspace(), which isn't 
 doing
 right things in START/STOP.

 What exactly is the problem?

I sent a reply now to the problem reported by Xiaoguang as I don't feel
now there is a problem :(

 It is working per-cpu currently whereas it just
 required to manage policy-cpu.

 Reported-by: Xiaoguang Chen chenxg.marv...@gmail.com
 Signed-off-by: Viresh Kumar viresh.ku...@linaro.org
 ---
 @Rafael:

 I don't know why this code was initially added. Please let me know if I am 
 doing
 something stupid.

 Also, please apply it as a fix for 3.10 as it is broken recently in 3.9.

 I'd love to, but I need answers to the above questions before I do that.

To be honest with the amount of experience I have now, my log was
poor :(

I have used following log in the attached patch:

Subject: [PATCH] cpufreq: userspace: Simplify governor

Userspace governor has got more code than what it needs for its functioning.
Lets simplify it. Portions of code removed are:
- Extra header files which aren't required anymore (rearranged them as well)
- cpu_{max|min|cur|set}_freq, as they are always same as policy-{max|min|cur}
- userspace_cpufreq_notifier_block as we don't need to set cpu_cur_freq anymore
- cpus_using_userspace_governor as it was for notifier code

Signed-off-by: Viresh Kumar viresh.ku...@linaro.org


0001-cpufreq-userspace-Simplify-governor.patch
Description: Binary data