Messages in this thread | | | Date | Tue, 11 Apr 2017 16:36:05 +0530 | From | Viresh Kumar <> | Subject | Re: [RFC 7/9] cpufreq: governor: support scheduler cpufreq callbacks on remote CPUs |
| |
On 30-03-17, 00:14, Rafael J. Wysocki wrote: > On Thursday, March 09, 2017 05:15:17 PM Viresh Kumar wrote: > > From: Steve Muckle <smuckle.linux@gmail.com> > > > > In preparation for the scheduler cpufreq callback happening on remote > > CPUs, add support for this in the legacy (ondemand and conservative) > > governors. The legacy governors make assumptions about the callback > > occurring on the CPU being updated. > > > > Signed-off-by: Steve Muckle <smuckle.linux@gmail.com> > > [ vk: minor updates in commit log ] > > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> > > --- > > drivers/cpufreq/cpufreq_governor.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/cpufreq/cpufreq_governor.c b/drivers/cpufreq/cpufreq_governor.c > > index 47e24b5384b3..c9e786e7ee1f 100644 > > --- a/drivers/cpufreq/cpufreq_governor.c > > +++ b/drivers/cpufreq/cpufreq_governor.c > > @@ -315,7 +315,7 @@ static void dbs_update_util_handler(struct update_util_data *data, u64 time, > > > > policy_dbs->last_sample_time = time; > > policy_dbs->work_in_progress = true; > > - irq_work_queue(&policy_dbs->irq_work); > > + irq_work_queue_on(&policy_dbs->irq_work, data->cpu); > > I'm totally unconvinced that this is sufficient. > > This function carries out lockless computations with the assumption that it > will always run on the CPU being updated. > > For instance, how is it prevented from being run on two CPUs in parallel in > the single-CPU policy case if cross-CPU updates are allowed to happen?
I am convinced that it is insufficient and yes I too missed the obvious race here as well for single cpu per policy. Sorry about that.
> Second, is accessing rq_clock(rq) of a remote runqueue a good idea entirely?
I am not sure about how costly that can be.
-- viresh
| |