On Tuesday, September 18, 2012, Tejun Heo wrote: > So, with work_on_cpu() reimplementation just posted[1], we can do the > following instead. Functionally it's about the same but less ugly. > Ugly as it may be, I think the previous open coded version is better > suited as a fix and for -stable. Thoughts?
I'd prefer this one to go into v3.7 along with the work_on_cpu() patch and then the open-coded version to go to -stable after the mainline one has got some testing coverage. Thanks, Rafael > [1] https://lkml.org/lkml/2012/9/18/430 > > drivers/cpufreq/powernow-k8.c | 63 > ++++++++++++++++++++++-------------------- > 1 file changed, 34 insertions(+), 29 deletions(-) > > diff --git a/drivers/cpufreq/powernow-k8.c b/drivers/cpufreq/powernow-k8.c > index c0e8164..1a40935 100644 > --- a/drivers/cpufreq/powernow-k8.c > +++ b/drivers/cpufreq/powernow-k8.c > @@ -35,7 +35,6 @@ > #include <linux/slab.h> > #include <linux/string.h> > #include <linux/cpumask.h> > -#include <linux/sched.h> /* for current / set_cpus_allowed() */ > #include <linux/io.h> > #include <linux/delay.h> > > @@ -1139,16 +1138,23 @@ static int transition_frequency_pstate(struct > powernow_k8_data *data, > return res; > } > > -/* Driver entry point to switch to the target frequency */ > -static int powernowk8_target(struct cpufreq_policy *pol, > - unsigned targfreq, unsigned relation) > +struct powernowk8_target_arg { > + struct cpufreq_policy *pol; > + unsigned targfreq; > + unsigned relation; > +}; > + > +static long powernowk8_target_fn(void *arg) > { > - cpumask_var_t oldmask; > + struct powernowk8_target_arg *pta = arg; > + struct cpufreq_policy *pol = pta->pol; > + unsigned targfreq = pta->targfreq; > + unsigned relation = pta->relation; > struct powernow_k8_data *data = per_cpu(powernow_data, pol->cpu); > u32 checkfid; > u32 checkvid; > unsigned int newstate; > - int ret = -EIO; > + int ret; > > if (!data) > return -EINVAL; > @@ -1156,29 +1162,16 @@ static int powernowk8_target(struct cpufreq_policy > *pol, > checkfid = data->currfid; > checkvid = data->currvid; > > - /* only run on specific CPU from here on. */ > - /* This is poor form: use a workqueue or smp_call_function_single */ > - if (!alloc_cpumask_var(&oldmask, GFP_KERNEL)) > - return -ENOMEM; > - > - cpumask_copy(oldmask, tsk_cpus_allowed(current)); > - set_cpus_allowed_ptr(current, cpumask_of(pol->cpu)); > - > - if (smp_processor_id() != pol->cpu) { > - printk(KERN_ERR PFX "limiting to cpu %u failed\n", pol->cpu); > - goto err_out; > - } > - > if (pending_bit_stuck()) { > printk(KERN_ERR PFX "failing targ, change pending bit set\n"); > - goto err_out; > + return -EIO; > } > > pr_debug("targ: cpu %d, %d kHz, min %d, max %d, relation %d\n", > pol->cpu, targfreq, pol->min, pol->max, relation); > > if (query_current_values_with_pending_wait(data)) > - goto err_out; > + return -EIO; > > if (cpu_family != CPU_HW_PSTATE) { > pr_debug("targ: curr fid 0x%x, vid 0x%x\n", > @@ -1196,7 +1189,7 @@ static int powernowk8_target(struct cpufreq_policy *pol, > > if (cpufreq_frequency_table_target(pol, data->powernow_table, > targfreq, relation, &newstate)) > - goto err_out; > + return -EIO; > > mutex_lock(&fidvid_mutex); > > @@ -1209,9 +1202,8 @@ static int powernowk8_target(struct cpufreq_policy *pol, > ret = transition_frequency_fidvid(data, newstate); > if (ret) { > printk(KERN_ERR PFX "transition frequency failed\n"); > - ret = 1; > mutex_unlock(&fidvid_mutex); > - goto err_out; > + return 1; > } > mutex_unlock(&fidvid_mutex); > > @@ -1220,12 +1212,25 @@ static int powernowk8_target(struct cpufreq_policy > *pol, > data->powernow_table[newstate].index); > else > pol->cur = find_khz_freq_from_fid(data->currfid); > - ret = 0; > > -err_out: > - set_cpus_allowed_ptr(current, oldmask); > - free_cpumask_var(oldmask); > - return ret; > + return 0; > +} > + > +/* Driver entry point to switch to the target frequency */ > +static int powernowk8_target(struct cpufreq_policy *pol, > + unsigned targfreq, unsigned relation) > +{ > + struct powernowk8_target_arg pta = { .pol = pol, .targfreq = targfreq, > + .relation = relation }; > + > + /* > + * Must run on @pol->cpu. cpufreq core is responsible for ensuring > + * that we're bound to the current CPU and pol->cpu stays online. > + */ > + if (smp_processor_id() == pol->cpu) > + return powernowk8_target_fn(&pta); > + else > + return work_on_cpu(pol->cpu, powernowk8_target_fn, &pta); > } > > /* Driver entry point to verify the policy and range of frequencies */ > > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
