Re: [PATCH] cpufreq: Simplify __cpufreq_remove_dev()
On 12 January 2013 03:41, Rafael J. Wysocki wrote: > Can any one of you two create a series of patches for me to take for v3.9 > with all of the correct Tested-by and Reviewed-by etc. tags, because I've > already lost track of your multiple versions and threads? :) I will. -- 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: Simplify __cpufreq_remove_dev()
On Thursday, January 10, 2013 04:10:55 PM Shawn Guo wrote: > On 10 January 2013 16:05, Viresh Kumar wrote: > > Another thing, can i have a tested-by from you for both my patches ? remove > > and > > add dev? > > > For both: > > Tested-by: Shawn Guo OK Can any one of you two create a series of patches for me to take for v3.9 with all of the correct Tested-by and Reviewed-by etc. tags, because I've already lost track of your multiple versions and threads? Rafael -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center. -- 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: Simplify __cpufreq_remove_dev()
On Thursday, January 10, 2013 04:10:55 PM Shawn Guo wrote: On 10 January 2013 16:05, Viresh Kumar viresh.ku...@linaro.org wrote: Another thing, can i have a tested-by from you for both my patches ? remove and add dev? For both: Tested-by: Shawn Guo shawn@linaro.org OK Can any one of you two create a series of patches for me to take for v3.9 with all of the correct Tested-by and Reviewed-by etc. tags, because I've already lost track of your multiple versions and threads? Rafael -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center. -- 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: Simplify __cpufreq_remove_dev()
On 12 January 2013 03:41, Rafael J. Wysocki r...@sisk.pl wrote: Can any one of you two create a series of patches for me to take for v3.9 with all of the correct Tested-by and Reviewed-by etc. tags, because I've already lost track of your multiple versions and threads? :) I will. -- 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: Simplify __cpufreq_remove_dev()
On 9 January 2013 16:50, Viresh Kumar wrote:
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> +static void update_policy_cpu(struct cpufreq_policy *policy, unsigned int
> cpu)
> +{
> + cpufreq_frequency_table_update_policy_cpu(old_cpu, cpu);
> + cpufreq_stats_update_policy_cpu(old_cpu, cpu);
This looked a bit ugly to me, as there may be other users too who want
to get this notification and so have below patch for this: pushed in my repo.
Tested on TC2. I have tested my patches well now and the activity is over from
my end now, unless somebody finds something objectionable :)
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index e3f7c7b..8a0b65e0 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -990,9 +990,9 @@ module_out:
static void update_policy_cpu(struct cpufreq_policy *policy, unsigned int cpu)
{
- unsigned int old_cpu = policy->cpu;
int j;
+ policy->last_cpu = policy->cpu;
policy->cpu = cpu;
for_each_cpu(j, policy->cpus) {
@@ -1001,8 +1001,9 @@ static void update_policy_cpu(struct
cpufreq_policy *policy, unsigned int cpu)
per_cpu(cpufreq_policy_cpu, j) = cpu;
}
- cpufreq_frequency_table_update_policy_cpu(old_cpu, cpu);
- cpufreq_stats_update_policy_cpu(old_cpu, cpu);
+ cpufreq_frequency_table_update_policy_cpu(policy);
+ blocking_notifier_call_chain(_policy_notifier_list,
+ CPUFREQ_UPDATE_POLICY_CPU, policy);
}
/**
diff --git a/drivers/cpufreq/cpufreq_stats.c b/drivers/cpufreq/cpufreq_stats.c
index b0df77a..2665f0a 100644
--- a/drivers/cpufreq/cpufreq_stats.c
+++ b/drivers/cpufreq/cpufreq_stats.c
@@ -267,17 +267,17 @@ error_get_fail:
return ret;
}
-void cpufreq_stats_update_policy_cpu(unsigned int old_cpu,
- unsigned int new_cpu)
+static void cpufreq_stats_update_policy_cpu(struct cpufreq_policy *policy)
{
- struct cpufreq_stats *stat = per_cpu(cpufreq_stats_table, old_cpu);
-
- pr_debug("Updating stats_table for new_cpu %u from old_cpu %u\n",
- new_cpu, old_cpu);
- per_cpu(cpufreq_stats_table, new_cpu) = per_cpu(cpufreq_stats_table,
- old_cpu);
- per_cpu(cpufreq_stats_table, old_cpu) = NULL;
- stat->cpu = new_cpu;
+ struct cpufreq_stats *stat = per_cpu(cpufreq_stats_table,
+ policy->last_cpu);
+
+ pr_debug("Updating stats_table for new_cpu %u from last_cpu %u\n",
+ policy->cpu, policy->last_cpu);
+ per_cpu(cpufreq_stats_table, policy->cpu) = per_cpu(cpufreq_stats_table,
+ policy->last_cpu);
+ per_cpu(cpufreq_stats_table, policy->last_cpu) = NULL;
+ stat->cpu = policy->cpu;
}
static int cpufreq_stat_notifier_policy(struct notifier_block *nb,
@@ -287,6 +287,12 @@ static int cpufreq_stat_notifier_policy(struct
notifier_block *nb,
struct cpufreq_policy *policy = data;
struct cpufreq_frequency_table *table;
unsigned int cpu = policy->cpu;
+
+ if (val == CPUFREQ_UPDATE_POLICY_CPU) {
+ cpufreq_stats_update_policy_cpu(policy);
+ return 0;
+ }
+
if (val != CPUFREQ_NOTIFY)
return 0;
table = cpufreq_frequency_get_table(cpu);
diff --git a/drivers/cpufreq/freq_table.c b/drivers/cpufreq/freq_table.c
index 2c3b07b..281e3b4 100644
--- a/drivers/cpufreq/freq_table.c
+++ b/drivers/cpufreq/freq_table.c
@@ -225,14 +225,13 @@ void cpufreq_frequency_table_put_attr(unsigned int cpu)
}
EXPORT_SYMBOL_GPL(cpufreq_frequency_table_put_attr);
-void cpufreq_frequency_table_update_policy_cpu(unsigned int old_cpu,
- unsigned int new_cpu)
+void cpufreq_frequency_table_update_policy_cpu(struct cpufreq_policy *policy)
{
- pr_debug("Updating show_table for new_cpu %u from old_cpu %u\n",
- new_cpu, old_cpu);
- per_cpu(cpufreq_show_table, new_cpu) = per_cpu(cpufreq_show_table,
- old_cpu);
- per_cpu(cpufreq_show_table, old_cpu) = NULL;
+ pr_debug("Updating show_table for new_cpu %u from last_cpu %u\n",
+ policy->cpu, policy->last_cpu);
+ per_cpu(cpufreq_show_table, policy->cpu) = per_cpu(cpufreq_show_table,
+ policy->last_cpu);
+ per_cpu(cpufreq_show_table, policy->last_cpu) = NULL;
}
struct cpufreq_frequency_table *cpufreq_frequency_get_table(unsigned int cpu)
diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
index af50dbd..7a3192a 100644
--- a/include/linux/cpufreq.h
+++ b/include/linux/cpufreq.h
@@ -90,7 +90,9 @@ struct cpufreq_policy {
cpumask_var_t related_cpus; /* CPUs with any coordination */
unsigned intshared_type; /* ANY or ALL affected CPUs
should set cpufreq */
- unsigned intcpu;
Re: [PATCH] cpufreq: Simplify __cpufreq_remove_dev()
On 10 January 2013 16:05, Viresh Kumar wrote: > Another thing, can i have a tested-by from you for both my patches ? remove > and > add dev? > For both: Tested-by: Shawn Guo -- 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: Simplify __cpufreq_remove_dev()
On 10 January 2013 13:24, Shawn Guo wrote: > On Wed, Jan 09, 2013 at 04:50:44PM +0530, Viresh Kumar wrote: >> @Shawn: I believe your driver don't require that ugly code anymore (Though i >> know there is a situation for that to happen, if we have two cpus, you remove >> second one and then add it back. With this cpufreq_add_dev() would call >> init() >> first and then try to match if there are any managed_policies present. But >> the >> issue you pointed out about unregistering the driver would be solved by this >> patch.) > > Yes, just played it and it works for me. However, I would have to keep > that little ugly code in my patch to save the dependency on your patch. > Will send a follow-up to clean that up once your patch hits mainline. Another thing, can i have a tested-by from you for both my patches ? remove and add dev? That would help it go quicker. -- 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: Simplify __cpufreq_remove_dev()
On 10 January 2013 13:24, Shawn Guo wrote: > Yes, just played it and it works for me. However, I would have to keep > that little ugly code in my patch to save the dependency on your patch. > Will send a follow-up to clean that up once your patch hits mainline. Good. Hopefully, patches from both of us would hit 3.9 and Rafael can apply yours after mine, with this check removed :) -- viresh -- 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: Simplify __cpufreq_remove_dev()
On 10 January 2013 13:24, Shawn Guo shawn@linaro.org wrote: Yes, just played it and it works for me. However, I would have to keep that little ugly code in my patch to save the dependency on your patch. Will send a follow-up to clean that up once your patch hits mainline. Good. Hopefully, patches from both of us would hit 3.9 and Rafael can apply yours after mine, with this check removed :) -- viresh -- 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: Simplify __cpufreq_remove_dev()
On 10 January 2013 13:24, Shawn Guo shawn@linaro.org wrote: On Wed, Jan 09, 2013 at 04:50:44PM +0530, Viresh Kumar wrote: @Shawn: I believe your driver don't require that ugly code anymore (Though i know there is a situation for that to happen, if we have two cpus, you remove second one and then add it back. With this cpufreq_add_dev() would call init() first and then try to match if there are any managed_policies present. But the issue you pointed out about unregistering the driver would be solved by this patch.) Yes, just played it and it works for me. However, I would have to keep that little ugly code in my patch to save the dependency on your patch. Will send a follow-up to clean that up once your patch hits mainline. Another thing, can i have a tested-by from you for both my patches ? remove and add dev? That would help it go quicker. -- 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: Simplify __cpufreq_remove_dev()
On 10 January 2013 16:05, Viresh Kumar viresh.ku...@linaro.org wrote: Another thing, can i have a tested-by from you for both my patches ? remove and add dev? For both: Tested-by: Shawn Guo shawn@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: Simplify __cpufreq_remove_dev()
On 9 January 2013 16:50, Viresh Kumar viresh.ku...@linaro.org wrote:
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
+static void update_policy_cpu(struct cpufreq_policy *policy, unsigned int
cpu)
+{
+ cpufreq_frequency_table_update_policy_cpu(old_cpu, cpu);
+ cpufreq_stats_update_policy_cpu(old_cpu, cpu);
This looked a bit ugly to me, as there may be other users too who want
to get this notification and so have below patch for this: pushed in my repo.
Tested on TC2. I have tested my patches well now and the activity is over from
my end now, unless somebody finds something objectionable :)
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index e3f7c7b..8a0b65e0 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -990,9 +990,9 @@ module_out:
static void update_policy_cpu(struct cpufreq_policy *policy, unsigned int cpu)
{
- unsigned int old_cpu = policy-cpu;
int j;
+ policy-last_cpu = policy-cpu;
policy-cpu = cpu;
for_each_cpu(j, policy-cpus) {
@@ -1001,8 +1001,9 @@ static void update_policy_cpu(struct
cpufreq_policy *policy, unsigned int cpu)
per_cpu(cpufreq_policy_cpu, j) = cpu;
}
- cpufreq_frequency_table_update_policy_cpu(old_cpu, cpu);
- cpufreq_stats_update_policy_cpu(old_cpu, cpu);
+ cpufreq_frequency_table_update_policy_cpu(policy);
+ blocking_notifier_call_chain(cpufreq_policy_notifier_list,
+ CPUFREQ_UPDATE_POLICY_CPU, policy);
}
/**
diff --git a/drivers/cpufreq/cpufreq_stats.c b/drivers/cpufreq/cpufreq_stats.c
index b0df77a..2665f0a 100644
--- a/drivers/cpufreq/cpufreq_stats.c
+++ b/drivers/cpufreq/cpufreq_stats.c
@@ -267,17 +267,17 @@ error_get_fail:
return ret;
}
-void cpufreq_stats_update_policy_cpu(unsigned int old_cpu,
- unsigned int new_cpu)
+static void cpufreq_stats_update_policy_cpu(struct cpufreq_policy *policy)
{
- struct cpufreq_stats *stat = per_cpu(cpufreq_stats_table, old_cpu);
-
- pr_debug(Updating stats_table for new_cpu %u from old_cpu %u\n,
- new_cpu, old_cpu);
- per_cpu(cpufreq_stats_table, new_cpu) = per_cpu(cpufreq_stats_table,
- old_cpu);
- per_cpu(cpufreq_stats_table, old_cpu) = NULL;
- stat-cpu = new_cpu;
+ struct cpufreq_stats *stat = per_cpu(cpufreq_stats_table,
+ policy-last_cpu);
+
+ pr_debug(Updating stats_table for new_cpu %u from last_cpu %u\n,
+ policy-cpu, policy-last_cpu);
+ per_cpu(cpufreq_stats_table, policy-cpu) = per_cpu(cpufreq_stats_table,
+ policy-last_cpu);
+ per_cpu(cpufreq_stats_table, policy-last_cpu) = NULL;
+ stat-cpu = policy-cpu;
}
static int cpufreq_stat_notifier_policy(struct notifier_block *nb,
@@ -287,6 +287,12 @@ static int cpufreq_stat_notifier_policy(struct
notifier_block *nb,
struct cpufreq_policy *policy = data;
struct cpufreq_frequency_table *table;
unsigned int cpu = policy-cpu;
+
+ if (val == CPUFREQ_UPDATE_POLICY_CPU) {
+ cpufreq_stats_update_policy_cpu(policy);
+ return 0;
+ }
+
if (val != CPUFREQ_NOTIFY)
return 0;
table = cpufreq_frequency_get_table(cpu);
diff --git a/drivers/cpufreq/freq_table.c b/drivers/cpufreq/freq_table.c
index 2c3b07b..281e3b4 100644
--- a/drivers/cpufreq/freq_table.c
+++ b/drivers/cpufreq/freq_table.c
@@ -225,14 +225,13 @@ void cpufreq_frequency_table_put_attr(unsigned int cpu)
}
EXPORT_SYMBOL_GPL(cpufreq_frequency_table_put_attr);
-void cpufreq_frequency_table_update_policy_cpu(unsigned int old_cpu,
- unsigned int new_cpu)
+void cpufreq_frequency_table_update_policy_cpu(struct cpufreq_policy *policy)
{
- pr_debug(Updating show_table for new_cpu %u from old_cpu %u\n,
- new_cpu, old_cpu);
- per_cpu(cpufreq_show_table, new_cpu) = per_cpu(cpufreq_show_table,
- old_cpu);
- per_cpu(cpufreq_show_table, old_cpu) = NULL;
+ pr_debug(Updating show_table for new_cpu %u from last_cpu %u\n,
+ policy-cpu, policy-last_cpu);
+ per_cpu(cpufreq_show_table, policy-cpu) = per_cpu(cpufreq_show_table,
+ policy-last_cpu);
+ per_cpu(cpufreq_show_table, policy-last_cpu) = NULL;
}
struct cpufreq_frequency_table *cpufreq_frequency_get_table(unsigned int cpu)
diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
index af50dbd..7a3192a 100644
--- a/include/linux/cpufreq.h
+++ b/include/linux/cpufreq.h
@@ -90,7 +90,9 @@ struct cpufreq_policy {
cpumask_var_t related_cpus; /* CPUs with any coordination */
unsigned intshared_type; /* ANY or ALL affected CPUs
should set cpufreq */
- unsigned intcpu;/*
Re: [PATCH] cpufreq: Simplify __cpufreq_remove_dev()
On Wed, Jan 09, 2013 at 04:50:44PM +0530, Viresh Kumar wrote: > @Shawn: I believe your driver don't require that ugly code anymore (Though i > know there is a situation for that to happen, if we have two cpus, you remove > second one and then add it back. With this cpufreq_add_dev() would call init() > first and then try to match if there are any managed_policies present. But the > issue you pointed out about unregistering the driver would be solved by this > patch.) Yes, just played it and it works for me. However, I would have to keep that little ugly code in my patch to save the dependency on your patch. Will send a follow-up to clean that up once your patch hits mainline. Shawn -- 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: Simplify __cpufreq_remove_dev()
On 9 January 2013 21:09, Viresh Kumar wrote: > I have tried that too, it is also pushed at: > > https://lkml.org/lkml/2012/12/16/5 Bad link :( http://git.linaro.org/gitweb?p=arm/big.LITTLE/mp.git;a=shortlog;h=refs/heads/cpufreq-fixes-v2 -- 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: Simplify __cpufreq_remove_dev()
On 9 January 2013 21:09, Viresh Kumar wrote: > On 9 January 2013 16:50, Viresh Kumar wrote: >> [Probably need to simplify cpufreq_add_dev() too, but that can be done as >> next >> step.] > > I have tried that too, it is also pushed at: > > https://lkml.org/lkml/2012/12/16/5 > > [Untested for now, will be doing it tomorrow] > > From: Viresh Kumar > Date: Wed, 9 Jan 2013 21:02:50 +0530 > Subject: [PATCH] cpufreq: Simplify cpufreq_add_dev() > > Currently cpufreq_add_dev() firsts allocated policy, calls ->init() and then > checks if this cpu should be already managed or not. And if it already > managed, > free its policy. > > We can save all this if we somehow know if this cpu is managed or not in > advance. policy->related_cpus contains list of all valid sibling cpus of > policy->cpu. We can check this to know if current cpu is already managed. > > Signed-off-by: Viresh Kumar Tested-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: Simplify __cpufreq_remove_dev()
On 9 January 2013 16:50, Viresh Kumar wrote:
> [Probably need to simplify cpufreq_add_dev() too, but that can be done as next
> step.]
I have tried that too, it is also pushed at:
https://lkml.org/lkml/2012/12/16/5
[Untested for now, will be doing it tomorrow]
From: Viresh Kumar
Date: Wed, 9 Jan 2013 21:02:50 +0530
Subject: [PATCH] cpufreq: Simplify cpufreq_add_dev()
Currently cpufreq_add_dev() firsts allocated policy, calls ->init() and then
checks if this cpu should be already managed or not. And if it already managed,
free its policy.
We can save all this if we somehow know if this cpu is managed or not in
advance. policy->related_cpus contains list of all valid sibling cpus of
policy->cpu. We can check this to know if current cpu is already managed.
Signed-off-by: Viresh Kumar
---
drivers/cpufreq/cpufreq.c | 150 --
1 file changed, 52 insertions(+), 98 deletions(-)
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index b8709899..e3f7c7b 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -701,92 +701,6 @@ static struct kobj_type ktype_cpufreq = {
.release= cpufreq_sysfs_release,
};
-/*
- * Returns:
- * Negative: Failure
- * 0:Success
- * Positive: When we have a managed CPU and the sysfs got symlinked
- */
-static int cpufreq_add_dev_policy(unsigned int cpu,
- struct cpufreq_policy *policy,
- struct device *dev)
-{
- int ret = 0;
-#ifdef CONFIG_SMP
- unsigned long flags;
- unsigned int j;
-#ifdef CONFIG_HOTPLUG_CPU
- struct cpufreq_governor *gov;
-
- gov = __find_governor(per_cpu(cpufreq_cpu_governor, cpu));
- if (gov) {
- policy->governor = gov;
- pr_debug("Restoring governor %s for cpu %d\n",
- policy->governor->name, cpu);
- }
-#endif
-
- for_each_cpu(j, policy->cpus) {
- struct cpufreq_policy *managed_policy;
-
- if (cpu == j)
- continue;
-
- /* Check for existing affected CPUs.
-* They may not be aware of it due to CPU Hotplug.
-* cpufreq_cpu_put is called when the device is removed
-* in __cpufreq_remove_dev()
-*/
- managed_policy = cpufreq_cpu_get(j);
- if (unlikely(managed_policy)) {
-
- /* Set proper policy_cpu */
- unlock_policy_rwsem_write(cpu);
- per_cpu(cpufreq_policy_cpu, cpu) = managed_policy->cpu;
-
- if (lock_policy_rwsem_write(cpu) < 0) {
- /* Should not go through policy unlock path */
- if (cpufreq_driver->exit)
- cpufreq_driver->exit(policy);
- cpufreq_cpu_put(managed_policy);
- return -EBUSY;
- }
-
- __cpufreq_governor(managed_policy, CPUFREQ_GOV_STOP);
-
- spin_lock_irqsave(_driver_lock, flags);
- cpumask_copy(managed_policy->cpus, policy->cpus);
- per_cpu(cpufreq_cpu_data, cpu) = managed_policy;
- spin_unlock_irqrestore(_driver_lock, flags);
-
- __cpufreq_governor(managed_policy, CPUFREQ_GOV_START);
- __cpufreq_governor(managed_policy, CPUFREQ_GOV_LIMITS);
-
- pr_debug("CPU already managed, adding link\n");
- ret = sysfs_create_link(>kobj,
- _policy->kobj,
- "cpufreq");
- if (ret)
- cpufreq_cpu_put(managed_policy);
- /*
-* Success. We only needed to be added to the mask.
-* Call driver->exit() because only the cpu parent of
-* the kobj needed to call init().
-*/
- if (cpufreq_driver->exit)
- cpufreq_driver->exit(policy);
-
- if (!ret)
- return 1;
- else
- return ret;
- }
- }
-#endif
- return ret;
-}
-
-
/* symlink affected CPUs */
static int cpufreq_add_dev_symlink(unsigned int cpu,
struct cpufreq_policy *policy)
@@ -891,6 +805,41 @@ err_out_kobj_put:
return ret;
}
+static int cpufreq_add_policy_cpu(unsigned int cpu, unsigned int sibling,
+ struct device *dev)
+{
+ struct cpufreq_policy *policy;
+ int ret = 0;
+ unsigned long flags;
+
+ policy =
[PATCH] cpufreq: Simplify __cpufreq_remove_dev()
__cpufreq_remove_dev() is called on multiple occasions: cpufreq_driver
unregister and cpu removals.
Current implementation of this routine is overly complex without much need. If
the cpu to be removed is the policy->cpu, we remove the policy first and add all
other cpus again from policy->cpus and then finally call __cpufreq_remove_dev()
again to remove the cpu to be deleted. Haa..
There exist a simple solution to removal of a cpu:
- Simply use the old policy structure
- update its fields like: policy->cpu, etc.
- notify any users of cpufreq, which depend on changing policy->cpu
Hence this patch, which tries to implement the above theory. It is tested well
by myself on ARM big.LITTLE TC2 SoC, which has 5 cores (2 A15 and 3 A7). Both
A15's share same struct policy and all A7's share same policy structure.
Signed-off-by: Viresh Kumar
Tested-by: Viresh Kumar
---
Hi Guys,
I am just an beginner in cpufreq stuff, please ignore any foolish mistakes. :)
Yesterday, i reviewed a cpufreq driver that had some ugly code in init()
routine:
http://www.spinics.net/lists/arm-kernel/msg215348.html
It wasn't ugly due to the author of the patch, but the way
__cpufreq_remove_dev() is implemented. And then i thought to simplify it.
[Probably need to simplify cpufreq_add_dev() too, but that can be done as next
step.]
I have rebased this patch over some other cpufreq core fixes i had posted
earlier:
https://lkml.org/lkml/2012/12/16/5
ARM mail servers are broken and hence this patch can't be applied as is. :(
I have pushed this and the dependency patch here:
http://git.linaro.org/gitweb?p=arm/big.LITTLE/mp.git;a=shortlog;h=refs/heads/cpufreq-fixes-v2
@Shawn: I believe your driver don't require that ugly code anymore (Though i
know there is a situation for that to happen, if we have two cpus, you remove
second one and then add it back. With this cpufreq_add_dev() would call init()
first and then try to match if there are any managed_policies present. But the
issue you pointed out about unregistering the driver would be solved by this
patch.)
drivers/cpufreq/cpufreq.c | 158 +---
drivers/cpufreq/cpufreq_stats.c | 21 +-
drivers/cpufreq/freq_table.c| 10 +++
include/linux/cpufreq.h | 5 ++
4 files changed, 101 insertions(+), 93 deletions(-)
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 271d3be..8df41ad 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -1036,6 +1036,22 @@ module_out:
return ret;
}
+static void update_policy_cpu(struct cpufreq_policy *policy, unsigned int cpu)
+{
+ unsigned int old_cpu = policy->cpu;
+ int j;
+
+ policy->cpu = cpu;
+
+ for_each_cpu(j, policy->cpus) {
+ if (!cpu_online(j))
+ continue;
+ per_cpu(cpufreq_policy_cpu, j) = cpu;
+ }
+
+ cpufreq_frequency_table_update_policy_cpu(old_cpu, cpu);
+ cpufreq_stats_update_policy_cpu(old_cpu, cpu);
+}
/**
* __cpufreq_remove_dev - remove a CPU device
@@ -1046,132 +1062,92 @@ module_out:
*/
static int __cpufreq_remove_dev(struct device *dev, struct subsys_interface
*sif)
{
- unsigned int cpu = dev->id;
+ unsigned int cpu = dev->id, ret, cpus;
unsigned long flags;
struct cpufreq_policy *data;
struct kobject *kobj;
struct completion *cmp;
-#ifdef CONFIG_SMP
struct device *cpu_dev;
- unsigned int j;
-#endif
- pr_debug("unregistering CPU %u\n", cpu);
+ pr_debug("%s: unregistering CPU %u\n", __func__, cpu);
spin_lock_irqsave(_driver_lock, flags);
data = per_cpu(cpufreq_cpu_data, cpu);
if (!data) {
+ pr_debug("%s: No cpu_data found\n", __func__);
spin_unlock_irqrestore(_driver_lock, flags);
unlock_policy_rwsem_write(cpu);
return -EINVAL;
}
- per_cpu(cpufreq_cpu_data, cpu) = NULL;
-#ifdef CONFIG_SMP
- /* if this isn't the CPU which is the parent of the kobj, we
-* only need to unlink, put and exit
-*/
- if (unlikely(cpu != data->cpu)) {
- pr_debug("removing link\n");
+ if (cpufreq_driver->target)
__cpufreq_governor(data, CPUFREQ_GOV_STOP);
- cpumask_clear_cpu(cpu, data->cpus);
- spin_unlock_irqrestore(_driver_lock, flags);
-
- __cpufreq_governor(data, CPUFREQ_GOV_START);
- __cpufreq_governor(data, CPUFREQ_GOV_LIMITS);
-
- kobj = >kobj;
- cpufreq_cpu_put(data);
- unlock_policy_rwsem_write(cpu);
- sysfs_remove_link(kobj, "cpufreq");
- return 0;
- }
-#endif
-
-#ifdef CONFIG_SMP
#ifdef CONFIG_HOTPLUG_CPU
strncpy(per_cpu(cpufreq_cpu_governor, cpu), data->governor->name,
CPUFREQ_NAME_LEN);
#endif
- /* if we have other CPUs
[PATCH] cpufreq: Simplify __cpufreq_remove_dev()
__cpufreq_remove_dev() is called on multiple occasions: cpufreq_driver
unregister and cpu removals.
Current implementation of this routine is overly complex without much need. If
the cpu to be removed is the policy-cpu, we remove the policy first and add all
other cpus again from policy-cpus and then finally call __cpufreq_remove_dev()
again to remove the cpu to be deleted. Haa..
There exist a simple solution to removal of a cpu:
- Simply use the old policy structure
- update its fields like: policy-cpu, etc.
- notify any users of cpufreq, which depend on changing policy-cpu
Hence this patch, which tries to implement the above theory. It is tested well
by myself on ARM big.LITTLE TC2 SoC, which has 5 cores (2 A15 and 3 A7). Both
A15's share same struct policy and all A7's share same policy structure.
Signed-off-by: Viresh Kumar viresh.ku...@linaro.org
Tested-by: Viresh Kumar viresh.ku...@linaro.org
---
Hi Guys,
I am just an beginner in cpufreq stuff, please ignore any foolish mistakes. :)
Yesterday, i reviewed a cpufreq driver that had some ugly code in init()
routine:
http://www.spinics.net/lists/arm-kernel/msg215348.html
It wasn't ugly due to the author of the patch, but the way
__cpufreq_remove_dev() is implemented. And then i thought to simplify it.
[Probably need to simplify cpufreq_add_dev() too, but that can be done as next
step.]
I have rebased this patch over some other cpufreq core fixes i had posted
earlier:
https://lkml.org/lkml/2012/12/16/5
ARM mail servers are broken and hence this patch can't be applied as is. :(
I have pushed this and the dependency patch here:
http://git.linaro.org/gitweb?p=arm/big.LITTLE/mp.git;a=shortlog;h=refs/heads/cpufreq-fixes-v2
@Shawn: I believe your driver don't require that ugly code anymore (Though i
know there is a situation for that to happen, if we have two cpus, you remove
second one and then add it back. With this cpufreq_add_dev() would call init()
first and then try to match if there are any managed_policies present. But the
issue you pointed out about unregistering the driver would be solved by this
patch.)
drivers/cpufreq/cpufreq.c | 158 +---
drivers/cpufreq/cpufreq_stats.c | 21 +-
drivers/cpufreq/freq_table.c| 10 +++
include/linux/cpufreq.h | 5 ++
4 files changed, 101 insertions(+), 93 deletions(-)
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 271d3be..8df41ad 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -1036,6 +1036,22 @@ module_out:
return ret;
}
+static void update_policy_cpu(struct cpufreq_policy *policy, unsigned int cpu)
+{
+ unsigned int old_cpu = policy-cpu;
+ int j;
+
+ policy-cpu = cpu;
+
+ for_each_cpu(j, policy-cpus) {
+ if (!cpu_online(j))
+ continue;
+ per_cpu(cpufreq_policy_cpu, j) = cpu;
+ }
+
+ cpufreq_frequency_table_update_policy_cpu(old_cpu, cpu);
+ cpufreq_stats_update_policy_cpu(old_cpu, cpu);
+}
/**
* __cpufreq_remove_dev - remove a CPU device
@@ -1046,132 +1062,92 @@ module_out:
*/
static int __cpufreq_remove_dev(struct device *dev, struct subsys_interface
*sif)
{
- unsigned int cpu = dev-id;
+ unsigned int cpu = dev-id, ret, cpus;
unsigned long flags;
struct cpufreq_policy *data;
struct kobject *kobj;
struct completion *cmp;
-#ifdef CONFIG_SMP
struct device *cpu_dev;
- unsigned int j;
-#endif
- pr_debug(unregistering CPU %u\n, cpu);
+ pr_debug(%s: unregistering CPU %u\n, __func__, cpu);
spin_lock_irqsave(cpufreq_driver_lock, flags);
data = per_cpu(cpufreq_cpu_data, cpu);
if (!data) {
+ pr_debug(%s: No cpu_data found\n, __func__);
spin_unlock_irqrestore(cpufreq_driver_lock, flags);
unlock_policy_rwsem_write(cpu);
return -EINVAL;
}
- per_cpu(cpufreq_cpu_data, cpu) = NULL;
-#ifdef CONFIG_SMP
- /* if this isn't the CPU which is the parent of the kobj, we
-* only need to unlink, put and exit
-*/
- if (unlikely(cpu != data-cpu)) {
- pr_debug(removing link\n);
+ if (cpufreq_driver-target)
__cpufreq_governor(data, CPUFREQ_GOV_STOP);
- cpumask_clear_cpu(cpu, data-cpus);
- spin_unlock_irqrestore(cpufreq_driver_lock, flags);
-
- __cpufreq_governor(data, CPUFREQ_GOV_START);
- __cpufreq_governor(data, CPUFREQ_GOV_LIMITS);
-
- kobj = dev-kobj;
- cpufreq_cpu_put(data);
- unlock_policy_rwsem_write(cpu);
- sysfs_remove_link(kobj, cpufreq);
- return 0;
- }
-#endif
-
-#ifdef CONFIG_SMP
#ifdef CONFIG_HOTPLUG_CPU
strncpy(per_cpu(cpufreq_cpu_governor, cpu), data-governor-name,
CPUFREQ_NAME_LEN);
Re: [PATCH] cpufreq: Simplify __cpufreq_remove_dev()
On 9 January 2013 16:50, Viresh Kumar viresh.ku...@linaro.org wrote:
[Probably need to simplify cpufreq_add_dev() too, but that can be done as next
step.]
I have tried that too, it is also pushed at:
https://lkml.org/lkml/2012/12/16/5
[Untested for now, will be doing it tomorrow]
From: Viresh Kumar viresh.ku...@linaro.org
Date: Wed, 9 Jan 2013 21:02:50 +0530
Subject: [PATCH] cpufreq: Simplify cpufreq_add_dev()
Currently cpufreq_add_dev() firsts allocated policy, calls -init() and then
checks if this cpu should be already managed or not. And if it already managed,
free its policy.
We can save all this if we somehow know if this cpu is managed or not in
advance. policy-related_cpus contains list of all valid sibling cpus of
policy-cpu. We can check this to know if current cpu is already managed.
Signed-off-by: Viresh Kumar viresh.ku...@linaro.org
---
drivers/cpufreq/cpufreq.c | 150 --
1 file changed, 52 insertions(+), 98 deletions(-)
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index b8709899..e3f7c7b 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -701,92 +701,6 @@ static struct kobj_type ktype_cpufreq = {
.release= cpufreq_sysfs_release,
};
-/*
- * Returns:
- * Negative: Failure
- * 0:Success
- * Positive: When we have a managed CPU and the sysfs got symlinked
- */
-static int cpufreq_add_dev_policy(unsigned int cpu,
- struct cpufreq_policy *policy,
- struct device *dev)
-{
- int ret = 0;
-#ifdef CONFIG_SMP
- unsigned long flags;
- unsigned int j;
-#ifdef CONFIG_HOTPLUG_CPU
- struct cpufreq_governor *gov;
-
- gov = __find_governor(per_cpu(cpufreq_cpu_governor, cpu));
- if (gov) {
- policy-governor = gov;
- pr_debug(Restoring governor %s for cpu %d\n,
- policy-governor-name, cpu);
- }
-#endif
-
- for_each_cpu(j, policy-cpus) {
- struct cpufreq_policy *managed_policy;
-
- if (cpu == j)
- continue;
-
- /* Check for existing affected CPUs.
-* They may not be aware of it due to CPU Hotplug.
-* cpufreq_cpu_put is called when the device is removed
-* in __cpufreq_remove_dev()
-*/
- managed_policy = cpufreq_cpu_get(j);
- if (unlikely(managed_policy)) {
-
- /* Set proper policy_cpu */
- unlock_policy_rwsem_write(cpu);
- per_cpu(cpufreq_policy_cpu, cpu) = managed_policy-cpu;
-
- if (lock_policy_rwsem_write(cpu) 0) {
- /* Should not go through policy unlock path */
- if (cpufreq_driver-exit)
- cpufreq_driver-exit(policy);
- cpufreq_cpu_put(managed_policy);
- return -EBUSY;
- }
-
- __cpufreq_governor(managed_policy, CPUFREQ_GOV_STOP);
-
- spin_lock_irqsave(cpufreq_driver_lock, flags);
- cpumask_copy(managed_policy-cpus, policy-cpus);
- per_cpu(cpufreq_cpu_data, cpu) = managed_policy;
- spin_unlock_irqrestore(cpufreq_driver_lock, flags);
-
- __cpufreq_governor(managed_policy, CPUFREQ_GOV_START);
- __cpufreq_governor(managed_policy, CPUFREQ_GOV_LIMITS);
-
- pr_debug(CPU already managed, adding link\n);
- ret = sysfs_create_link(dev-kobj,
- managed_policy-kobj,
- cpufreq);
- if (ret)
- cpufreq_cpu_put(managed_policy);
- /*
-* Success. We only needed to be added to the mask.
-* Call driver-exit() because only the cpu parent of
-* the kobj needed to call init().
-*/
- if (cpufreq_driver-exit)
- cpufreq_driver-exit(policy);
-
- if (!ret)
- return 1;
- else
- return ret;
- }
- }
-#endif
- return ret;
-}
-
-
/* symlink affected CPUs */
static int cpufreq_add_dev_symlink(unsigned int cpu,
struct cpufreq_policy *policy)
@@ -891,6 +805,41 @@ err_out_kobj_put:
return ret;
}
+static int cpufreq_add_policy_cpu(unsigned int cpu, unsigned int sibling,
+ struct device *dev)
+{
+ struct cpufreq_policy *policy;
+
Re: [PATCH] cpufreq: Simplify __cpufreq_remove_dev()
On 9 January 2013 21:09, Viresh Kumar viresh.ku...@linaro.org wrote: On 9 January 2013 16:50, Viresh Kumar viresh.ku...@linaro.org wrote: [Probably need to simplify cpufreq_add_dev() too, but that can be done as next step.] I have tried that too, it is also pushed at: https://lkml.org/lkml/2012/12/16/5 [Untested for now, will be doing it tomorrow] From: Viresh Kumar viresh.ku...@linaro.org Date: Wed, 9 Jan 2013 21:02:50 +0530 Subject: [PATCH] cpufreq: Simplify cpufreq_add_dev() Currently cpufreq_add_dev() firsts allocated policy, calls -init() and then checks if this cpu should be already managed or not. And if it already managed, free its policy. We can save all this if we somehow know if this cpu is managed or not in advance. policy-related_cpus contains list of all valid sibling cpus of policy-cpu. We can check this to know if current cpu is already managed. Signed-off-by: Viresh Kumar viresh.ku...@linaro.org Tested-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: Simplify __cpufreq_remove_dev()
On 9 January 2013 21:09, Viresh Kumar viresh.ku...@linaro.org wrote: I have tried that too, it is also pushed at: https://lkml.org/lkml/2012/12/16/5 Bad link :( http://git.linaro.org/gitweb?p=arm/big.LITTLE/mp.git;a=shortlog;h=refs/heads/cpufreq-fixes-v2 -- 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: Simplify __cpufreq_remove_dev()
On Wed, Jan 09, 2013 at 04:50:44PM +0530, Viresh Kumar wrote: @Shawn: I believe your driver don't require that ugly code anymore (Though i know there is a situation for that to happen, if we have two cpus, you remove second one and then add it back. With this cpufreq_add_dev() would call init() first and then try to match if there are any managed_policies present. But the issue you pointed out about unregistering the driver would be solved by this patch.) Yes, just played it and it works for me. However, I would have to keep that little ugly code in my patch to save the dependency on your patch. Will send a follow-up to clean that up once your patch hits mainline. Shawn -- 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/
