On 21-07-17, 15:03, Peter Zijlstra wrote: > On Thu, Jul 13, 2017 at 12:14:37PM +0530, Viresh Kumar wrote:
> > @@ -42,6 +42,7 @@ void cpufreq_add_update_util_hook(int cpu, struct > > update_util_data *data, > > return; > > > > data->func = func; > > + data->cpu = cpu; > > rcu_assign_pointer(per_cpu(cpufreq_update_util_data, cpu), data); > > } > > EXPORT_SYMBOL_GPL(cpufreq_add_update_util_hook); > > redundant. Actually we will still need it. We pass hook->cpu to sugov_get_util() in the 2nd patch of this series and there is no work around possible around that. > > diff --git a/kernel/sched/cpufreq_schedutil.c > > b/kernel/sched/cpufreq_schedutil.c > > index 29a397067ffa..ed9c589e5386 100644 > > --- a/kernel/sched/cpufreq_schedutil.c > > +++ b/kernel/sched/cpufreq_schedutil.c > > @@ -218,6 +218,10 @@ static void sugov_update_single(struct > > update_util_data *hook, u64 time, > > unsigned int next_f; > > bool busy; > > > > + /* Remote callbacks aren't allowed for policies which aren't shared */ > > + if (smp_processor_id() != hook->cpu) > > + return; > > + > > sugov_set_iowait_boost(sg_cpu, time, flags); > > sg_cpu->last_update = time; > > > > @@ -290,6 +294,10 @@ static void sugov_update_shared(struct > > update_util_data *hook, u64 time, > > unsigned long util, max; > > unsigned int next_f; > > > > + /* Don't allow remote callbacks */ > > + if (smp_processor_id() != hook->cpu) > > + return; > > + > > sugov_get_util(&util, &max); > > > > raw_spin_lock(&sg_policy->update_lock); > > > Given the whole rq->lock thing, I suspect we could actually not do these > two. You meant sugov_get_util() and raw_spin_lock()? Why? The locking is required here in the shared-policy case to make sure only one CPU is updating the frequency for the entire policy. And we can't really avoid that even with the rq->lock guarantees from the scheduler for the target CPU. > That would then continue to process the iowait and other accounting > stuff, but stall the moment we call into the actual driver, which will > then drop the request on the floor as per the first few hunks. I am not sure I understood your comment completely though. > This seems ok. Except of course you'll have conflicts with Juri's patch > set, but that should be trivial to sort out. Yeah, I wouldn't mind rebasing if his series gets in first. -- viresh
