| Message ID | 98330b2deb910453a356404b8cf774c94326bc42.1355636864.git.viresh.kumar@linaro.org (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
On Sunday, December 16, 2012 11:20:08 AM Viresh Kumar wrote: > cpufreq core doesn't manage offline cpus and if driver->init() has returned > mask including offline cpus, it may result in unwanted behavior by cpufreq core > or governors. > > We need to get only online cpus in this mask. There are two places to fix this > mask, cpufreq core and cpufreq driver. It makes sense to do this at common place > and hence is done in core. Well, this series makes sense to me, but I'd like to hear what the other people think. Thanks, Rafael > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> > --- > drivers/cpufreq/cpufreq.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c > index 1f93dbd..de99517 100644 > --- a/drivers/cpufreq/cpufreq.c > +++ b/drivers/cpufreq/cpufreq.c > @@ -970,6 +970,13 @@ static int cpufreq_add_dev(struct device *dev, struct subsys_interface *sif) > pr_debug("initialization failed\n"); > goto err_unlock_policy; > } > + > + /* > + * affected cpus must always be the one, which are online. We aren't > + * managing offline cpus here. > + */ > + cpumask_and(policy->cpus, policy->cpus, cpu_online_mask); > + > policy->user_policy.min = policy->min; > policy->user_policy.max = policy->max; > >
On 16 December 2012 18:34, Rafael J. Wysocki <rjw@sisk.pl> wrote: > On Sunday, December 16, 2012 11:20:08 AM Viresh Kumar wrote: >> cpufreq core doesn't manage offline cpus and if driver->init() has returned >> mask including offline cpus, it may result in unwanted behavior by cpufreq core >> or governors. >> >> We need to get only online cpus in this mask. There are two places to fix this >> mask, cpufreq core and cpufreq driver. It makes sense to do this at common place >> and hence is done in core. > > Well, this series makes sense to me, but I'd like to hear what the other people > think. That sounds great :) Some more information for others about how i reached to these issues is present here: https://lkml.org/lkml/2012/12/10/44 -- 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
On 16 December 2012 19:07, Viresh Kumar <viresh.kumar@linaro.org> wrote: > On 16 December 2012 18:34, Rafael J. Wysocki <rjw@sisk.pl> wrote: >> Well, this series makes sense to me, but I'd like to hear what the other people >> think. > > That sounds great :) > > Some more information for others about how i reached to these issues > is present here: > > https://lkml.org/lkml/2012/12/10/44 Hmm.. I don't know, if we are going to get any feedback from others :( BTW, i consider them as fixes and so would make sense to get them in next rc. What do you think? -- viresh -- 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
On Wednesday, January 02, 2013 11:59:57 AM Viresh Kumar wrote: > On 16 December 2012 19:07, Viresh Kumar <viresh.kumar@linaro.org> wrote: > > On 16 December 2012 18:34, Rafael J. Wysocki <rjw@sisk.pl> wrote: > > >> Well, this series makes sense to me, but I'd like to hear what the other people > >> think. > > > > That sounds great :) > > > > Some more information for others about how i reached to these issues > > is present here: > > > > https://lkml.org/lkml/2012/12/10/44 > > Hmm.. I don't know, if we are going to get any feedback from others :( Surely somebody cares except for us two? > BTW, i consider them as fixes and so would make sense to get them in next rc. > What do you think? Yes, if somebody tells me "yes, this fixes a problem for me". Otherwise, I don't quite see the reason. Thanks, Rafael
On 3 January 2013 06:43, Rafael J. Wysocki <rjw@sisk.pl> wrote: >> BTW, i consider them as fixes and so would make sense to get them in next rc. >> What do you think? > > Yes, if somebody tells me "yes, this fixes a problem for me". Otherwise, > I don't quite see the reason. I don't know how much people test HOTPLUG, but there are clear bugs related to hotplug of cpus on a multiple cpu system :) -- 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
On Thursday, January 03, 2013 09:02:22 AM Viresh Kumar wrote: > On 3 January 2013 06:43, Rafael J. Wysocki <rjw@sisk.pl> wrote: > >> BTW, i consider them as fixes and so would make sense to get them in next rc. > >> What do you think? > > > > Yes, if somebody tells me "yes, this fixes a problem for me". Otherwise, > > I don't quite see the reason. > > I don't know how much people test HOTPLUG, but there are clear bugs related > to hotplug of cpus on a multiple cpu system :) True, but have those bugs been introduced recently (ie. in v3.8-rc1 or later)? Rafael
On 3 January 2013 17:32, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> True, but have those bugs been introduced recently (ie. in v3.8-rc1 or later)?
Don't know... I feel they were always there, its just that nobody
tested it that way :)
--
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
On Friday, January 04, 2013 10:44:36 AM Viresh Kumar wrote: > On 3 January 2013 17:32, Rafael J. Wysocki <rjw@sisk.pl> wrote: > > True, but have those bugs been introduced recently (ie. in v3.8-rc1 or later)? > > Don't know... I feel they were always there, its just that nobody > tested it that way :) That exactly is my point. :-) If they have always been there, I don't see much reason for hurrying with the fixes, so I'll queue them up for v3.9. Thanks, Rafael
On Sunday, December 16, 2012 11:20:08 AM Viresh Kumar wrote: > cpufreq core doesn't manage offline cpus and if driver->init() has returned > mask including offline cpus, it may result in unwanted behavior by cpufreq core > or governors. > > We need to get only online cpus in this mask. There are two places to fix this > mask, cpufreq core and cpufreq driver. It makes sense to do this at common place > and hence is done in core. Applied to the linux-next branch of the linux-pm.git tree as v3.9 material. Thanks, Rafael > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> > --- > drivers/cpufreq/cpufreq.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c > index 1f93dbd..de99517 100644 > --- a/drivers/cpufreq/cpufreq.c > +++ b/drivers/cpufreq/cpufreq.c > @@ -970,6 +970,13 @@ static int cpufreq_add_dev(struct device *dev, struct subsys_interface *sif) > pr_debug("initialization failed\n"); > goto err_unlock_policy; > } > + > + /* > + * affected cpus must always be the one, which are online. We aren't > + * managing offline cpus here. > + */ > + cpumask_and(policy->cpus, policy->cpus, cpu_online_mask); > + > policy->user_policy.min = policy->min; > policy->user_policy.max = policy->max; > >
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index 1f93dbd..de99517 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -970,6 +970,13 @@ static int cpufreq_add_dev(struct device *dev, struct subsys_interface *sif) pr_debug("initialization failed\n"); goto err_unlock_policy; } + + /* + * affected cpus must always be the one, which are online. We aren't + * managing offline cpus here. + */ + cpumask_and(policy->cpus, policy->cpus, cpu_online_mask); + policy->user_policy.min = policy->min; policy->user_policy.max = policy->max;
cpufreq core doesn't manage offline cpus and if driver->init() has returned mask including offline cpus, it may result in unwanted behavior by cpufreq core or governors. We need to get only online cpus in this mask. There are two places to fix this mask, cpufreq core and cpufreq driver. It makes sense to do this at common place and hence is done in core. Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> --- drivers/cpufreq/cpufreq.c | 7 +++++++ 1 file changed, 7 insertions(+)