Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1326217 > unrolled thread

[PATCH 2/11] cpufreq: governor: Use common mutex for dbs_data protection

Started by"Rafael J. Wysocki" <rjw@rjwysocki.net>
First post2016-02-04 00:40 +0100
Last post2016-02-05 04:30 +0100
Articles 11 — 4 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  [PATCH 2/11] cpufreq: governor: Use common mutex for dbs_data protection "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-02-04 00:40 +0100
    Re: [PATCH 2/11] cpufreq: governor: Use common mutex for dbs_data  protection Saravana Kannan <skannan@codeaurora.org> - 2016-02-04 02:00 +0100
    Re: [PATCH 2/11] cpufreq: governor: Use common mutex for dbs_data  protection Viresh Kumar <viresh.kumar@linaro.org> - 2016-02-04 06:20 +0100
      Re: [PATCH 2/11] cpufreq: governor: Use common mutex for dbs_data protection "Rafael J. Wysocki" <rafael@kernel.org> - 2016-02-04 17:50 +0100
        Re: [PATCH 2/11] cpufreq: governor: Use common mutex for dbs_data  protection Viresh Kumar <viresh.kumar@linaro.org> - 2016-02-05 04:10 +0100
          Re: [PATCH 2/11] cpufreq: governor: Use common mutex for dbs_data protection "Rafael J. Wysocki" <rafael@kernel.org> - 2016-02-05 04:10 +0100
            Re: [PATCH 2/11] cpufreq: governor: Use common mutex for dbs_data protection "Rafael J. Wysocki" <rafael@kernel.org> - 2016-02-05 04:20 +0100
              Re: [PATCH 2/11] cpufreq: governor: Use common mutex for dbs_data protection "Rafael J. Wysocki" <rafael@kernel.org> - 2016-02-05 04:20 +0100
                Re: [PATCH 2/11] cpufreq: governor: Use common mutex for dbs_data  protection Viresh Kumar <viresh.kumar@linaro.org> - 2016-02-05 04:30 +0100
                  Re: [PATCH 2/11] cpufreq: governor: Use common mutex for dbs_data protection "Rafael J. Wysocki" <rafael@kernel.org> - 2016-02-05 04:40 +0100
            Re: [PATCH 2/11] cpufreq: governor: Use common mutex for dbs_data  protection Viresh Kumar <viresh.kumar@linaro.org> - 2016-02-05 04:30 +0100

#1326217 — [PATCH 2/11] cpufreq: governor: Use common mutex for dbs_data protection

From"Rafael J. Wysocki" <rjw@rjwysocki.net>
Date2016-02-04 00:40 +0100
Subject[PATCH 2/11] cpufreq: governor: Use common mutex for dbs_data protection
Message-ID<qYfbJ-uu-37@gated-at.bofh.it>
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Every governor relying on the common code in cpufreq_governor.c
has to provide its own mutex in struct common_dbs_data.  However,
those mutexes are never used at the same time and doing it this
way makes it rather difficult to follow the code.  Moreover,
if two governor modules are loaded we end up with two mutexes
used for the same purpose one at a time.

Introduce a single common mutex for that instead and drop the
mutex field from struct common_dbs_data.  That at least will
ensure that the mutex is always present and initialized regardless
of what the particular governors do.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/cpufreq/cpufreq_conservative.c |    1 -
 drivers/cpufreq/cpufreq_governor.c     |    7 +++++--
 drivers/cpufreq/cpufreq_governor.h     |    6 +-----
 drivers/cpufreq/cpufreq_ondemand.c     |    5 ++---
 4 files changed, 8 insertions(+), 11 deletions(-)

Index: linux-pm/drivers/cpufreq/cpufreq_governor.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/cpufreq_governor.c
+++ linux-pm/drivers/cpufreq/cpufreq_governor.c
@@ -22,6 +22,9 @@
 
 #include "cpufreq_governor.h"
 
+DEFINE_MUTEX(dbs_data_mutex);
+EXPORT_SYMBOL_GPL(dbs_data_mutex);
+
 static struct attribute_group *get_sysfs_attr(struct dbs_data *dbs_data)
 {
 	if (have_governor_per_policy())
@@ -542,7 +545,7 @@ int cpufreq_governor_dbs(struct cpufreq_
 	int ret;
 
 	/* Lock governor to block concurrent initialization of governor */
-	mutex_lock(&cdata->mutex);
+	mutex_lock(&dbs_data_mutex);
 
 	if (have_governor_per_policy())
 		dbs_data = policy->governor_data;
@@ -575,7 +578,7 @@ int cpufreq_governor_dbs(struct cpufreq_
 	}
 
 unlock:
-	mutex_unlock(&cdata->mutex);
+	mutex_unlock(&dbs_data_mutex);
 
 	return ret;
 }
Index: linux-pm/drivers/cpufreq/cpufreq_governor.h
===================================================================
--- linux-pm.orig/drivers/cpufreq/cpufreq_governor.h
+++ linux-pm/drivers/cpufreq/cpufreq_governor.h
@@ -223,11 +223,6 @@ struct common_dbs_data {
 
 	/* Governor specific ops, see below */
 	void *gov_ops;
-
-	/*
-	 * Protects governor's data (struct dbs_data and struct common_dbs_data)
-	 */
-	struct mutex mutex;
 };
 
 /* Governor Per policy data */
@@ -272,6 +267,7 @@ static ssize_t show_sampling_rate_min_go
 	return sprintf(buf, "%u\n", dbs_data->min_sampling_rate);	\
 }
 
+extern struct mutex dbs_data_mutex;
 extern struct mutex cpufreq_governor_lock;
 void gov_set_update_util(struct cpu_common_dbs_info *shared,
 			 unsigned int delay_us);
Index: linux-pm/drivers/cpufreq/cpufreq_ondemand.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/cpufreq_ondemand.c
+++ linux-pm/drivers/cpufreq/cpufreq_ondemand.c
@@ -251,7 +251,7 @@ static void update_sampling_rate(struct
 	/*
 	 * Lock governor so that governor start/stop can't execute in parallel.
 	 */
-	mutex_lock(&od_dbs_cdata.mutex);
+	mutex_lock(&dbs_data_mutex);
 
 	cpumask_copy(&cpumask, cpu_online_mask);
 
@@ -304,7 +304,7 @@ static void update_sampling_rate(struct
 		}
 	}
 
-	mutex_unlock(&od_dbs_cdata.mutex);
+	mutex_unlock(&dbs_data_mutex);
 }
 
 static ssize_t store_sampling_rate(struct dbs_data *dbs_data, const char *buf,
@@ -550,7 +550,6 @@ static struct common_dbs_data od_dbs_cda
 	.gov_ops = &od_ops,
 	.init = od_init,
 	.exit = od_exit,
-	.mutex = __MUTEX_INITIALIZER(od_dbs_cdata.mutex),
 };
 
 static int od_cpufreq_governor_dbs(struct cpufreq_policy *policy,
Index: linux-pm/drivers/cpufreq/cpufreq_conservative.c
===================================================================
--- linux-pm.orig/drivers/cpufreq/cpufreq_conservative.c
+++ linux-pm/drivers/cpufreq/cpufreq_conservative.c
@@ -368,7 +368,6 @@ static struct common_dbs_data cs_dbs_cda
 	.gov_check_cpu = cs_check_cpu,
 	.init = cs_init,
 	.exit = cs_exit,
-	.mutex = __MUTEX_INITIALIZER(cs_dbs_cdata.mutex),
 };
 
 static int cs_cpufreq_governor_dbs(struct cpufreq_policy *policy,

[toc] | [next] | [standalone]


#1326320 — Re: [PATCH 2/11] cpufreq: governor: Use common mutex for dbs_data protection

FromSaravana Kannan <skannan@codeaurora.org>
Date2016-02-04 02:00 +0100
SubjectRe: [PATCH 2/11] cpufreq: governor: Use common mutex for dbs_data protection
Message-ID<qYgr7-1oh-5@gated-at.bofh.it>
In reply to#1326217
On 02/03/2016 03:16 PM, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> Every governor relying on the common code in cpufreq_governor.c
> has to provide its own mutex in struct common_dbs_data.  However,
> those mutexes are never used at the same time and doing it this
> way makes it rather difficult to follow the code.  Moreover,
> if two governor modules are loaded we end up with two mutexes
> used for the same purpose one at a time.
>
> Introduce a single common mutex for that instead and drop the
> mutex field from struct common_dbs_data.  That at least will
> ensure that the mutex is always present and initialized regardless
> of what the particular governors do.
>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
>   drivers/cpufreq/cpufreq_conservative.c |    1 -
>   drivers/cpufreq/cpufreq_governor.c     |    7 +++++--
>   drivers/cpufreq/cpufreq_governor.h     |    6 +-----
>   drivers/cpufreq/cpufreq_ondemand.c     |    5 ++---
>   4 files changed, 8 insertions(+), 11 deletions(-)
>
> Index: linux-pm/drivers/cpufreq/cpufreq_governor.c
> ===================================================================
> --- linux-pm.orig/drivers/cpufreq/cpufreq_governor.c
> +++ linux-pm/drivers/cpufreq/cpufreq_governor.c
> @@ -22,6 +22,9 @@
>
>   #include "cpufreq_governor.h"
>
> +DEFINE_MUTEX(dbs_data_mutex);
> +EXPORT_SYMBOL_GPL(dbs_data_mutex);
> +
>   static struct attribute_group *get_sysfs_attr(struct dbs_data *dbs_data)
>   {
>   	if (have_governor_per_policy())
> @@ -542,7 +545,7 @@ int cpufreq_governor_dbs(struct cpufreq_
>   	int ret;
>
>   	/* Lock governor to block concurrent initialization of governor */
> -	mutex_lock(&cdata->mutex);
> +	mutex_lock(&dbs_data_mutex);
>
>   	if (have_governor_per_policy())
>   		dbs_data = policy->governor_data;
> @@ -575,7 +578,7 @@ int cpufreq_governor_dbs(struct cpufreq_
>   	}
>
>   unlock:
> -	mutex_unlock(&cdata->mutex);
> +	mutex_unlock(&dbs_data_mutex);
>
>   	return ret;
>   }
> Index: linux-pm/drivers/cpufreq/cpufreq_governor.h
> ===================================================================
> --- linux-pm.orig/drivers/cpufreq/cpufreq_governor.h
> +++ linux-pm/drivers/cpufreq/cpufreq_governor.h
> @@ -223,11 +223,6 @@ struct common_dbs_data {
>
>   	/* Governor specific ops, see below */
>   	void *gov_ops;
> -
> -	/*
> -	 * Protects governor's data (struct dbs_data and struct common_dbs_data)
> -	 */
> -	struct mutex mutex;
>   };
>
>   /* Governor Per policy data */
> @@ -272,6 +267,7 @@ static ssize_t show_sampling_rate_min_go
>   	return sprintf(buf, "%u\n", dbs_data->min_sampling_rate);	\
>   }
>
> +extern struct mutex dbs_data_mutex;
>   extern struct mutex cpufreq_governor_lock;
>   void gov_set_update_util(struct cpu_common_dbs_info *shared,
>   			 unsigned int delay_us);
> Index: linux-pm/drivers/cpufreq/cpufreq_ondemand.c
> ===================================================================
> --- linux-pm.orig/drivers/cpufreq/cpufreq_ondemand.c
> +++ linux-pm/drivers/cpufreq/cpufreq_ondemand.c
> @@ -251,7 +251,7 @@ static void update_sampling_rate(struct
>   	/*
>   	 * Lock governor so that governor start/stop can't execute in parallel.
>   	 */
> -	mutex_lock(&od_dbs_cdata.mutex);
> +	mutex_lock(&dbs_data_mutex);
>
>   	cpumask_copy(&cpumask, cpu_online_mask);
>
> @@ -304,7 +304,7 @@ static void update_sampling_rate(struct
>   		}
>   	}
>
> -	mutex_unlock(&od_dbs_cdata.mutex);
> +	mutex_unlock(&dbs_data_mutex);
>   }
>
>   static ssize_t store_sampling_rate(struct dbs_data *dbs_data, const char *buf,
> @@ -550,7 +550,6 @@ static struct common_dbs_data od_dbs_cda
>   	.gov_ops = &od_ops,
>   	.init = od_init,
>   	.exit = od_exit,
> -	.mutex = __MUTEX_INITIALIZER(od_dbs_cdata.mutex),
>   };
>
>   static int od_cpufreq_governor_dbs(struct cpufreq_policy *policy,
> Index: linux-pm/drivers/cpufreq/cpufreq_conservative.c
> ===================================================================
> --- linux-pm.orig/drivers/cpufreq/cpufreq_conservative.c
> +++ linux-pm/drivers/cpufreq/cpufreq_conservative.c
> @@ -368,7 +368,6 @@ static struct common_dbs_data cs_dbs_cda
>   	.gov_check_cpu = cs_check_cpu,
>   	.init = cs_init,
>   	.exit = cs_exit,
> -	.mutex = __MUTEX_INITIALIZER(cs_dbs_cdata.mutex),
>   };
>
>   static int cs_cpufreq_governor_dbs(struct cpufreq_policy *policy,
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>


Acked-by: Saravana Kannan <skannan@codeaurora.org>


-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[toc] | [prev] | [next] | [standalone]


#1326407 — Re: [PATCH 2/11] cpufreq: governor: Use common mutex for dbs_data protection

FromViresh Kumar <viresh.kumar@linaro.org>
Date2016-02-04 06:20 +0100
SubjectRe: [PATCH 2/11] cpufreq: governor: Use common mutex for dbs_data protection
Message-ID<qYkuJ-4bg-11@gated-at.bofh.it>
In reply to#1326217
On 04-02-16, 00:16, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> Every governor relying on the common code in cpufreq_governor.c
> has to provide its own mutex in struct common_dbs_data.  However,
> those mutexes are never used at the same time

Why do you think so? I thought they can always be used in parallel.

Consider 2 or more policies, one can have ondemand as the governor,
whereas other one can have conservative.

If CPUs go online/offline or if governors are switching in parallel,
then cpufreq_governor_dbs() can very much run in parallel for ondemand
and conservative.

Or am I missing something here ?

-- 
viresh

[toc] | [prev] | [next] | [standalone]


#1326992

From"Rafael J. Wysocki" <rafael@kernel.org>
Date2016-02-04 17:50 +0100
Message-ID<qYvgu-4wj-7@gated-at.bofh.it>
In reply to#1326407
On Thu, Feb 4, 2016 at 6:09 AM, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> On 04-02-16, 00:16, Rafael J. Wysocki wrote:
>> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>>
>> Every governor relying on the common code in cpufreq_governor.c
>> has to provide its own mutex in struct common_dbs_data.  However,
>> those mutexes are never used at the same time
>
> Why do you think so? I thought they can always be used in parallel.
>
> Consider 2 or more policies, one can have ondemand as the governor,
> whereas other one can have conservative.
>
> If CPUs go online/offline or if governors are switching in parallel,
> then cpufreq_governor_dbs() can very much run in parallel for ondemand
> and conservative.
>
> Or am I missing something here ?

Well, so perhaps the changelog is inaccurate.

However, what's wrong with using a single mutex then?

Thanks,
Rafael

[toc] | [prev] | [next] | [standalone]


#1327434 — Re: [PATCH 2/11] cpufreq: governor: Use common mutex for dbs_data protection

FromViresh Kumar <viresh.kumar@linaro.org>
Date2016-02-05 04:10 +0100
SubjectRe: [PATCH 2/11] cpufreq: governor: Use common mutex for dbs_data protection
Message-ID<qYEWu-2Kf-9@gated-at.bofh.it>
In reply to#1326992
On 04-02-16, 17:46, Rafael J. Wysocki wrote:
> On Thu, Feb 4, 2016 at 6:09 AM, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> > On 04-02-16, 00:16, Rafael J. Wysocki wrote:
> >> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> >>
> >> Every governor relying on the common code in cpufreq_governor.c
> >> has to provide its own mutex in struct common_dbs_data.  However,
> >> those mutexes are never used at the same time
> >
> > Why do you think so? I thought they can always be used in parallel.
> >
> > Consider 2 or more policies, one can have ondemand as the governor,
> > whereas other one can have conservative.
> >
> > If CPUs go online/offline or if governors are switching in parallel,
> > then cpufreq_governor_dbs() can very much run in parallel for ondemand
> > and conservative.
> >
> > Or am I missing something here ?
> 
> Well, so perhaps the changelog is inaccurate.
> 
> However, what's wrong with using a single mutex then?

You are killing the possibility of running the code faster. Consider
this:
- A 16 policy system with N CPUs in every policy (IBM has something
  similar only :) )..
- 4 policies using ondemand, 4 using conservative, 4 using powersave
  and 4 with performance.
- Now if we try to change governors for all of them in parallel, only
  one will be done at a time and others have to wait for this
  BIG-kernel lock.
- Ideally the lock shouldn't have been in cdata itself, but dbs_data
  only. But there was a specific race because of which we were
  required to move it to a higher level, i.e. cdata. And so we killed
  the possibility of parallelism of multiple governors of same type
  (ofcourse only of update-sampling-rate and cpufreq_governor_dbs()..

So, it makes thing much slower..

-- 
viresh

[toc] | [prev] | [next] | [standalone]


#1327440

From"Rafael J. Wysocki" <rafael@kernel.org>
Date2016-02-05 04:10 +0100
Message-ID<qYEWu-2Kf-23@gated-at.bofh.it>
In reply to#1327434
On Fri, Feb 5, 2016 at 3:59 AM, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> On 04-02-16, 17:46, Rafael J. Wysocki wrote:
>> On Thu, Feb 4, 2016 at 6:09 AM, Viresh Kumar <viresh.kumar@linaro.org> wrote:
>> > On 04-02-16, 00:16, Rafael J. Wysocki wrote:
>> >> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>> >>
>> >> Every governor relying on the common code in cpufreq_governor.c
>> >> has to provide its own mutex in struct common_dbs_data.  However,
>> >> those mutexes are never used at the same time
>> >
>> > Why do you think so? I thought they can always be used in parallel.
>> >
>> > Consider 2 or more policies, one can have ondemand as the governor,
>> > whereas other one can have conservative.
>> >
>> > If CPUs go online/offline or if governors are switching in parallel,
>> > then cpufreq_governor_dbs() can very much run in parallel for ondemand
>> > and conservative.
>> >
>> > Or am I missing something here ?
>>
>> Well, so perhaps the changelog is inaccurate.
>>
>> However, what's wrong with using a single mutex then?
>
> You are killing the possibility of running the code faster. Consider
> this:
> - A 16 policy system with N CPUs in every policy (IBM has something
>   similar only :) )..
> - 4 policies using ondemand, 4 using conservative, 4 using powersave
>   and 4 with performance.
> - Now if we try to change governors for all of them in parallel, only
>   one will be done at a time and others have to wait for this
>   BIG-kernel lock.

And why is this a big problem, actually?  Why do we want the switching
of governors to be that efficient?

Thanks,
Rafael

[toc] | [prev] | [next] | [standalone]


#1327445

From"Rafael J. Wysocki" <rafael@kernel.org>
Date2016-02-05 04:20 +0100
Message-ID<qYF69-2VA-3@gated-at.bofh.it>
In reply to#1327440
On Fri, Feb 5, 2016 at 4:06 AM, Rafael J. Wysocki <rafael@kernel.org> wrote:
> On Fri, Feb 5, 2016 at 3:59 AM, Viresh Kumar <viresh.kumar@linaro.org> wrote:
>> On 04-02-16, 17:46, Rafael J. Wysocki wrote:
>>> On Thu, Feb 4, 2016 at 6:09 AM, Viresh Kumar <viresh.kumar@linaro.org> wrote:
>>> > On 04-02-16, 00:16, Rafael J. Wysocki wrote:
>>> >> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>>> >>
>>> >> Every governor relying on the common code in cpufreq_governor.c
>>> >> has to provide its own mutex in struct common_dbs_data.  However,
>>> >> those mutexes are never used at the same time
>>> >
>>> > Why do you think so? I thought they can always be used in parallel.
>>> >
>>> > Consider 2 or more policies, one can have ondemand as the governor,
>>> > whereas other one can have conservative.
>>> >
>>> > If CPUs go online/offline or if governors are switching in parallel,
>>> > then cpufreq_governor_dbs() can very much run in parallel for ondemand
>>> > and conservative.
>>> >
>>> > Or am I missing something here ?
>>>
>>> Well, so perhaps the changelog is inaccurate.
>>>
>>> However, what's wrong with using a single mutex then?
>>
>> You are killing the possibility of running the code faster. Consider
>> this:
>> - A 16 policy system with N CPUs in every policy (IBM has something
>>   similar only :) )..
>> - 4 policies using ondemand, 4 using conservative, 4 using powersave
>>   and 4 with performance.
>> - Now if we try to change governors for all of them in parallel, only
>>   one will be done at a time and others have to wait for this
>>   BIG-kernel lock.
>
> And why is this a big problem, actually?  Why do we want the switching
> of governors to be that efficient?

In any case, we're talking about having one lock instead of two, mind
you (out of the tree things don't count), because performance and
powersave don't even use the code in question.

I'm not really sure how much of a difference that would make on a
really big system.

Thanks,
Rafael

[toc] | [prev] | [next] | [standalone]


#1327451

From"Rafael J. Wysocki" <rafael@kernel.org>
Date2016-02-05 04:20 +0100
Message-ID<qYF69-2VA-11@gated-at.bofh.it>
In reply to#1327445
On Fri, Feb 5, 2016 at 4:15 AM, Rafael J. Wysocki <rafael@kernel.org> wrote:
> On Fri, Feb 5, 2016 at 4:06 AM, Rafael J. Wysocki <rafael@kernel.org> wrote:
>> On Fri, Feb 5, 2016 at 3:59 AM, Viresh Kumar <viresh.kumar@linaro.org> wrote:
>>> On 04-02-16, 17:46, Rafael J. Wysocki wrote:
>>>> On Thu, Feb 4, 2016 at 6:09 AM, Viresh Kumar <viresh.kumar@linaro.org> wrote:
>>>> > On 04-02-16, 00:16, Rafael J. Wysocki wrote:
>>>> >> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>>>> >>
>>>> >> Every governor relying on the common code in cpufreq_governor.c
>>>> >> has to provide its own mutex in struct common_dbs_data.  However,
>>>> >> those mutexes are never used at the same time
>>>> >
>>>> > Why do you think so? I thought they can always be used in parallel.
>>>> >
>>>> > Consider 2 or more policies, one can have ondemand as the governor,
>>>> > whereas other one can have conservative.
>>>> >
>>>> > If CPUs go online/offline or if governors are switching in parallel,
>>>> > then cpufreq_governor_dbs() can very much run in parallel for ondemand
>>>> > and conservative.
>>>> >
>>>> > Or am I missing something here ?
>>>>
>>>> Well, so perhaps the changelog is inaccurate.
>>>>
>>>> However, what's wrong with using a single mutex then?
>>>
>>> You are killing the possibility of running the code faster. Consider
>>> this:
>>> - A 16 policy system with N CPUs in every policy (IBM has something
>>>   similar only :) )..
>>> - 4 policies using ondemand, 4 using conservative, 4 using powersave
>>>   and 4 with performance.
>>> - Now if we try to change governors for all of them in parallel, only
>>>   one will be done at a time and others have to wait for this
>>>   BIG-kernel lock.
>>
>> And why is this a big problem, actually?  Why do we want the switching
>> of governors to be that efficient?
>
> In any case, we're talking about having one lock instead of two, mind
> you (out of the tree things don't count), because performance and
> powersave don't even use the code in question.
>
> I'm not really sure how much of a difference that would make on a
> really big system.

And don't we switch governors under policy->rwsem anyway?

Thanks,
Rafael

[toc] | [prev] | [next] | [standalone]


#1327457 — Re: [PATCH 2/11] cpufreq: governor: Use common mutex for dbs_data protection

FromViresh Kumar <viresh.kumar@linaro.org>
Date2016-02-05 04:30 +0100
SubjectRe: [PATCH 2/11] cpufreq: governor: Use common mutex for dbs_data protection
Message-ID<qYFfQ-31p-3@gated-at.bofh.it>
In reply to#1327451
On 05-02-16, 04:17, Rafael J. Wysocki wrote:
> And don't we switch governors under policy->rwsem anyway?

So ? That is blocking only a single policy only, but with the new
change, we will block all policies from doing that concurrently.

-- 
viresh

[toc] | [prev] | [next] | [standalone]


#1327462

From"Rafael J. Wysocki" <rafael@kernel.org>
Date2016-02-05 04:40 +0100
Message-ID<qYFpv-36k-11@gated-at.bofh.it>
In reply to#1327457
On Fri, Feb 5, 2016 at 4:24 AM, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> On 05-02-16, 04:17, Rafael J. Wysocki wrote:
>> And don't we switch governors under policy->rwsem anyway?
>
> So ? That is blocking only a single policy only, but with the new
> change, we will block all policies from doing that concurrently.

No, it won't.  Again: one lock instead of two.  How much of a
difference this makes performance-wise?

And the price is the stupid dance we need to do to even get to those
locks!  Come on.

Thanks,
Rafael

[toc] | [prev] | [next] | [standalone]


#1327459 — Re: [PATCH 2/11] cpufreq: governor: Use common mutex for dbs_data protection

FromViresh Kumar <viresh.kumar@linaro.org>
Date2016-02-05 04:30 +0100
SubjectRe: [PATCH 2/11] cpufreq: governor: Use common mutex for dbs_data protection
Message-ID<qYFfQ-31p-7@gated-at.bofh.it>
In reply to#1327440
On 05-02-16, 04:06, Rafael J. Wysocki wrote:
> And why is this a big problem, actually?  Why do we want the switching
> of governors to be that efficient?

I am not saying its a big problem, just that its kind of a big lock,
which could have been finer.

-- 
viresh

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web