From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753936AbdDKLGN (ORCPT ); Tue, 11 Apr 2017 07:06:13 -0400 Received: from mail-pf0-f180.google.com ([209.85.192.180]:33094 "EHLO mail-pf0-f180.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752765AbdDKLGK (ORCPT ); Tue, 11 Apr 2017 07:06:10 -0400 Date: Tue, 11 Apr 2017 16:36:05 +0530 From: Viresh Kumar To: "Rafael J. Wysocki" Cc: Ingo Molnar , Peter Zijlstra , linaro-kernel@lists.linaro.org, linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org, Vincent Guittot , smuckle.linux@gmail.com, juri.lelli@arm.com, Morten.Rasmussen@arm.com, patrick.bellasi@arm.com, eas-dev@lists.linaro.org Subject: Re: [RFC 7/9] cpufreq: governor: support scheduler cpufreq callbacks on remote CPUs Message-ID: <20170411110605.GD13627@vireshk-i7> References: <49216ebaad6b26a1d5916350d07654181662b15b.1489058244.git.viresh.kumar@linaro.org> <3331999.EqKcTVSiQr@aspire.rjw.lan> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <3331999.EqKcTVSiQr@aspire.rjw.lan> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 > > > > 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 > > [ vk: minor updates in commit log ] > > Signed-off-by: Viresh Kumar > > --- > > 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