* Thomas Renninger (trenn@xxxxxxx) wrote:
> On Friday 24 July 2009 15:25:02 Thomas Renninger wrote:
>
> This patch is wrong, only one hunk should be added:
>
> > Signed-off-by: Thomas Renninger <trenn@xxxxxxx>
> > ---
> > drivers/cpufreq/cpufreq.c | 9 +++------
> > 1 files changed, 3 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> > index 5cc77fb..d467631 100644
> > --- a/drivers/cpufreq/cpufreq.c
> > +++ b/drivers/cpufreq/cpufreq.c
> > @@ -781,6 +781,8 @@ int cpufreq_add_dev_policy(unsigned int cpu,
> struct cpufreq_policy *policy,
> >
> > /* 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)) {
> > @@ -793,7 +795,6 @@ int cpufreq_add_dev_policy(unsigned int cpu,
> struct cpufreq_policy *policy,
> > /* Should not go through policy unlock path */
> > if (cpufreq_driver->exit)
> > cpufreq_driver->exit(policy);
> > - cpufreq_cpu_put(managed_policy);
> > return -EBUSY;
> -EBUSY is returned, thus the device will not be added and also not
> removed later -> cpufreq_cpu_put(managed_policy); will not be called
>
> -> Do not submit this hunk.
>
> > }
> >
> > @@ -806,8 +807,6 @@ int cpufreq_add_dev_policy(unsigned int cpu,
> struct cpufreq_policy *policy,
> > ret = sysfs_create_link(&sys_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
> Hmm, only the "!" must get reverted which was my initial fix, but I
> wanted to do better :(.
> So this hunk is also wrong it must be:
> - if (!ret)
> + if (ret)
Ah, yes, this logic inversion is indeed a problem. Can you resubmit with
just this fix ?
Thanks,
Mathieu
>
> > @@ -843,10 +842,8 @@ int cpufreq_add_dev_symlink(unsigned int cpu,
> struct cpufreq_policy *policy)
> > cpu_sys_dev = get_cpu_sysdev(j);
> > ret = sysfs_create_link(&cpu_sys_dev->kobj, &policy->kobj,
> > "cpufreq");
> > - if (ret) {
> > - cpufreq_cpu_put(managed_policy);
> > + if (ret)
> > return ret;
> > - }
> > }
> > return ret;
> > }
> This hunk should also not get applied, an error is returned,
> cpufreq_cpu_put should be needed.
>
> Sorry for the inconvenience...,
>
> Thomas
--
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
--
To unsubscribe from this list: send the line "unsubscribe cpufreq" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
