linaro-kernel August 2013

linaro-kernel@lists.linaro.org

110 participants
185 discussions

Re: [RFC][PATCH] cpufreq: Do not hold driver module references for additional policy CPUs

by Viresh Kumar

On 1 August 2013 13:41, Srivatsa S. Bhat <srivatsa.bhat(a)linux.vnet.ibm.com> wrote: > On 08/01/2013 05:38 AM, Rafael J. Wysocki wrote: >> From: Rafael J. Wysocki <rafael.j.wysocki(a)intel.com> >> >> The cpufreq core is a little inconsistent in the way it uses the >> driver module refcount. >> >> Namely, if __cpufreq_add_dev() is called for a CPU without siblings >> or generally a CPU for which a new policy object has to be created, >> it grabs a reference to the driver module to start with, but drops >> that reference before returning. As a result, the driver module >> refcount is then equal to 0 after __cpufreq_add_dev() has returned. >> >> On the other hand, if the given CPU is a sibling of some other >> CPU already having a policy, cpufreq_add_policy_cpu() is called >> to link the new CPU to the existing policy. In that case, >> cpufreq_cpu_get() is called to obtain that policy and grabs a >> reference to the driver module, but that reference is not >> released and the module refcount will be different from 0 after >> __cpufreq_add_dev() returns (unless there is an error). That >> prevents the driver module from being unloaded until >> __cpufreq_remove_dev() is called for all the CPUs that >> cpufreq_add_policy_cpu() was called for previously. >> >> To remove that inconsistency make cpufreq_add_policy_cpu() execute >> cpufreq_cpu_put() for the given policy before returning, which >> decrements the driver module refcount so that it will be 0 after >> __cpufreq_add_dev() returns, > > Removing the inconsistency is a good thing, but I think we should > make it consistent the other way around - make a CPU-online increment > the driver module refcount and decrement it only on CPU-offline. I took time to review to this mail as I was looking at the problem yesterday. I am sorry to say, but I have completely different views as compared to You and Rafael both :) First of all, Rafael's patch is incomplete as it hasn't fixed the issue completely. When we have multiple CPUs per policy and cpufreq_add_dev() is called for the first one, it call cpufreq_get_cpu() for all cpus of this policy(), so count is == x (no. of CPUs in this policy) + 1 (This is the initial value of .owner). And so we still have module count getting incremented for other cpus :) Now few lines about My point of view to this whole thing. I believe we should get rid of .owner field from struct cpufreq_driver completely. It doesn't make sense to me in doing all this management at all. Surprised? Shocked? Laughing at me? :) Well I may be wrong but this is what I think: - It looks stupid to me that I can't do this from userspace in one go: $ insmod cpufreq_driver.ko $ rmmod cpufreq_driver.ko What the hell changed in between that isn't visible to user? It looked completely stupid in that way.. Something like this sure makes sense: $ insmod ondemand-governor.ko $ change governor to ondemand for few CPUs $ rmmod ondemand-governor.ko as we have deliberately add few users of governor. And so without second step, rmmod should really work smoothly. And it does. Now, why shouldn't there be a problem with this approach? I will write that inline to the problems you just described. > The reason is, one should not be able to unload the back-end cpufreq > driver module when some CPUs are still being managed. Nasty things > will result if we allow that. For example, if we unload the module, > and then try to do a CPU offline, then the cpufreq hotplug notifier > won't even be called (because cpufreq_unregister_driver also > unregisters the hotplug notifier). And that might be troublesome. So what? Its simply equivalent to we have booted our system, haven't inserted cpufreq module and taken out a cpu. > Even worse, if we unload a cpufreq driver module and load a new > one and *then* try to offline the CPU, then the cpufreq_driver->exit() > function that we call during CPU offline will end up calling the > corresponding function of an entirely different driver! So the > ->init() and ->exit() calls won't match. That's not true. When we unload the module, it must call cpufreq_unregister_driver() which should call cpufreq_remove_cpu() for all cpus and so exit() is already called for last module. If we get something new now, it should simply work. What do you think gentlemen? -- viresh

11 years, 4 months

Re: BUG_ON(irqs_disabled()) in cpufreq.c

by Viresh Kumar

More lists cc'd On Thu, Aug 1, 2013 at 6:08 PM, Michael Giardino <giardino(a)ece.gatech.edu> wrote: > Hi, > > I hope this is the right spot to ask this. > > Is there any way to change the cpu frequency using the cpufreq driver > from within an hrtimer callback function? I encounter a kernel panic > at cpufreq.c line 255 (260 in 3.9.5) > > void __cpufreq_notify_transition(struct cpufreq_policy *policy, > 253 struct cpufreq_freqs *freqs, unsigned int state) > 254 { > 255 BUG_ON(irqs_disabled()); > > > I'm assuming this is due to the cpufreq_notify_transition needs > interrupts to notify? Is there a way around this? It appears that > acpi-cpufreq.c is calling the notifier both before and after the > transition (CPUFREQ_PRECHANGE and CPUFREQ_POSTCHANGE). > > Are there any frequency change functions that can operate without interrupts? > > The background is this: > > For the past few days I have been trying to get to the bottom of a > problem involving a nonSMP governor I have written. The goal of this > governor is to change the frequency on a predefined schedule (in the > ms scale). The basic breakdown is this: > > 1. The external scheduler computes schedules and then passes them to > the governor (with much code appropriated from the userspace governor) > via an exported function. > 2. The governor sets the frequency, then sets a timer to call the next > frequency change when it goes off > > In order to do this, I was using hrtimers. These timer's callback > functions run with interrupts disabled. > > Michael Giardino > <giardino(a)ece.gatech.edu> > <michael.giardino(a)gmail.com> > -- > To unsubscribe from this list: send the line "unsubscribe cpufreq" in > the body of a message to majordomo(a)vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html

11 years, 4 months

[PATCH 0/3] nohz/full: final Kconfig bits for ARM support

by Kevin Hilman

As of v3.10, the core ARM support is in mainline for NO_HZ_FULL. The only remaining part is the removal of the hard-coded Kconfig requirement on 64-bit platforms, which I believe can now be removed after the nsec granularity cputime was made to work on non 64_BIT (c.f. commit 8c23b80e, cputime_nsecs: use math64.h for nsec resolution conversion helpers.) This series makes the final Kconfig changes to bring NO_HZ_FULL support to ARM. Series applies to v3.10. I will queue up the arch/arm patch for Russell separately once the generic changes are accepted. Kevin arch/arm/Kconfig | 1 + init/Kconfig | 2 +- kernel/time/Kconfig | 2 -- 3 files changed, 2 insertions(+), 3 deletions(-) -- 1.8.3

11 years, 4 months

[PATCH] ASoC: samsung: Make secondary I2S DAI device a child of primary

by Mark Brown

From: Mark Brown <broonie(a)linaro.org> More for neatness than for any great utility. Really we shouldn't be creating the child device at all, refactoring will follow. Signed-off-by: Mark Brown <broonie(a)linaro.org> --- sound/soc/samsung/i2s.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/sound/soc/samsung/i2s.c b/sound/soc/samsung/i2s.c index a3e4b49..47e08dd 100644 --- a/sound/soc/samsung/i2s.c +++ b/sound/soc/samsung/i2s.c @@ -1019,6 +1019,8 @@ static struct i2s_dai *i2s_alloc_dai(struct platform_device *pdev, bool sec) if (IS_ERR(i2s->pdev)) return NULL; + i2s->pdev->dev.parent = &pdev->dev; + platform_set_drvdata(i2s->pdev, i2s); ret = platform_device_add(i2s->pdev); if (ret < 0) -- 1.8.3.2

11 years, 4 months

[PATCH] cpufreq: Re-arrange header files

by Viresh Kumar

This exercise has been done earlier but hasn't been done for few files. Lets do it now. It targets to fix following problems: - Include headers in ascending order, so that we don't add same multiple times by mistake. - <asm/> must be included after <linux/>, so that they override whatever they need. Signed-off-by: Viresh Kumar <viresh.kumar(a)linaro.org> --- drivers/cpufreq/cpufreq.c | 23 +++++++++++------------ drivers/cpufreq/cpufreq_governor.c | 2 +- drivers/cpufreq/cpufreq_ondemand.c | 2 +- drivers/cpufreq/cpufreq_performance.c | 4 ++-- drivers/cpufreq/cpufreq_powersave.c | 4 ++-- drivers/cpufreq/cpufreq_stats.c | 12 ++++++------ include/linux/cpufreq.h | 10 +++++----- 7 files changed, 28 insertions(+), 29 deletions(-) diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index 170d344..d5f2a09 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -17,25 +17,24 @@ #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt -#include <asm/cputime.h> +#include <linux/cpu.h> +#include <linux/cpufreq.h> +#include <linux/completion.h> +#include <linux/delay.h> +#include <linux/device.h> +#include <linux/init.h> +#include <linux/interrupt.h> #include <linux/kernel.h> #include <linux/kernel_stat.h> #include <linux/module.h> -#include <linux/init.h> +#include <linux/mutex.h> #include <linux/notifier.h> -#include <linux/cpufreq.h> -#include <linux/delay.h> -#include <linux/interrupt.h> -#include <linux/spinlock.h> -#include <linux/tick.h> -#include <linux/device.h> #include <linux/slab.h> -#include <linux/cpu.h> -#include <linux/completion.h> -#include <linux/mutex.h> +#include <linux/spinlock.h> #include <linux/syscore_ops.h> - +#include <linux/tick.h> #include <trace/events/power.h> +#include <asm/cputime.h> /** * The "cpufreq driver" - the arch- or hardware-dependent low diff --git a/drivers/cpufreq/cpufreq_governor.c b/drivers/cpufreq/cpufreq_governor.c index 7409dbd..5876788 100644 --- a/drivers/cpufreq/cpufreq_governor.c +++ b/drivers/cpufreq/cpufreq_governor.c @@ -16,7 +16,6 @@ #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt -#include <asm/cputime.h> #include <linux/cpufreq.h> #include <linux/cpumask.h> #include <linux/export.h> @@ -25,6 +24,7 @@ #include <linux/slab.h> #include <linux/types.h> #include <linux/workqueue.h> +#include <asm/cputime.h> #include "cpufreq_governor.h" diff --git a/drivers/cpufreq/cpufreq_ondemand.c b/drivers/cpufreq/cpufreq_ondemand.c index a3c5574..d3344b5 100644 --- a/drivers/cpufreq/cpufreq_ondemand.c +++ b/drivers/cpufreq/cpufreq_ondemand.c @@ -12,6 +12,7 @@ #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt +#include <linux/cpu.h> #include <linux/cpufreq.h> #include <linux/init.h> #include <linux/kernel.h> @@ -24,7 +25,6 @@ #include <linux/sysfs.h> #include <linux/tick.h> #include <linux/types.h> -#include <linux/cpu.h> #include "cpufreq_governor.h" diff --git a/drivers/cpufreq/cpufreq_performance.c b/drivers/cpufreq/cpufreq_performance.c index 9fef7d6..bdfda75 100644 --- a/drivers/cpufreq/cpufreq_performance.c +++ b/drivers/cpufreq/cpufreq_performance.c @@ -12,10 +12,10 @@ #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt -#include <linux/kernel.h> -#include <linux/module.h> #include <linux/cpufreq.h> #include <linux/init.h> +#include <linux/kernel.h> +#include <linux/module.h> static int cpufreq_governor_performance(struct cpufreq_policy *policy, unsigned int event) diff --git a/drivers/cpufreq/cpufreq_powersave.c b/drivers/cpufreq/cpufreq_powersave.c index 32109a1..47235e2 100644 --- a/drivers/cpufreq/cpufreq_powersave.c +++ b/drivers/cpufreq/cpufreq_powersave.c @@ -12,10 +12,10 @@ #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt -#include <linux/kernel.h> -#include <linux/module.h> #include <linux/cpufreq.h> #include <linux/init.h> +#include <linux/kernel.h> +#include <linux/module.h> static int cpufreq_governor_powersave(struct cpufreq_policy *policy, unsigned int event) diff --git a/drivers/cpufreq/cpufreq_stats.c b/drivers/cpufreq/cpufreq_stats.c index cb38413..7610e4b 100644 --- a/drivers/cpufreq/cpufreq_stats.c +++ b/drivers/cpufreq/cpufreq_stats.c @@ -9,17 +9,17 @@ * published by the Free Software Foundation. */ -#include <linux/kernel.h> -#include <linux/slab.h> #include <linux/cpu.h> -#include <linux/sysfs.h> #include <linux/cpufreq.h> -#include <linux/module.h> #include <linux/jiffies.h> -#include <linux/percpu.h> +#include <linux/kernel.h> #include <linux/kobject.h> -#include <linux/spinlock.h> +#include <linux/module.h> #include <linux/notifier.h> +#include <linux/percpu.h> +#include <linux/slab.h> +#include <linux/spinlock.h> +#include <linux/sysfs.h> #include <asm/cputime.h> static spinlock_t cpufreq_stats_lock; diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h index e1fd215..247d83a 100644 --- a/include/linux/cpufreq.h +++ b/include/linux/cpufreq.h @@ -11,15 +11,15 @@ #ifndef _LINUX_CPUFREQ_H #define _LINUX_CPUFREQ_H -#include <asm/cputime.h> +#include <linux/cpumask.h> +#include <linux/completion.h> +#include <linux/kobject.h> #include <linux/mutex.h> #include <linux/notifier.h> -#include <linux/threads.h> -#include <linux/kobject.h> #include <linux/sysfs.h> -#include <linux/completion.h> +#include <linux/threads.h> #include <linux/workqueue.h> -#include <linux/cpumask.h> +#include <asm/cputime.h> #include <asm/div64.h> #define CPUFREQ_NAME_LEN 16 -- 1.7.12.rc2.18.g61b472e

11 years, 4 months

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

linaro-kernel August 2013