From mboxrd@z Thu Jan 1 00:00:00 1970 From: Fabio Baltieri Subject: Re: [PATCH v7 2/4] cpufreq: ondemand: call dbs_check_cpu only when necessary Date: Thu, 31 Jan 2013 09:39:09 +0100 Message-ID: <20130131083825.GA30667@balto.lan> References: <1359550803-18577-1-git-send-email-fabio.baltieri@linaro.org> <1359550803-18577-3-git-send-email-fabio.baltieri@linaro.org> <20130130164614.GC30559@balto.lan> Mime-Version: 1.0 Return-path: Content-Disposition: inline In-Reply-To: Sender: linux-pm-owner@vger.kernel.org List-ID: Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Viresh Kumar Cc: "Rafael J. Wysocki" , cpufreq@vger.kernel.org, linux-pm@vger.kernel.org, Linus Walleij , swarren@wwwdotorg.org, linaro-dev@lists.linaro.org, Nicolas Pitre , mathieu.poirier@linaro.org, Joseph Lo , linux-kernel@vger.kernel.org Hello Viresh, On Wed, Jan 30, 2013 at 10:41:08PM +0530, Viresh Kumar wrote: > On 30 January 2013 22:16, Fabio Baltieri wrote: > > Isn't that how it works now? The current cpu ktime is not checked > > against its own, but against the "leader" cpu (dbs_info_local->cdbs.cpu), > > that's why it's initialized only for the first. > > > > Maybe I should have used dbs_info_leader/dbs_info instead of > > dbs_info_local/dbs_info. > > This routine is called as wq handler. Which will recover dbs_info from work > using container_of. Which would give dbs_info_local for the cpu j. > > Then we will execute below code. > > + /* use leader CPU's dbs_info */ > + dbs_info = &per_cpu(od_cpu_dbs_info, dbs_info_local->cdbs.cpu); > > dbs_info_local->cdbs.cpu was uninitialized for all cpus except policy->cpu. Ok, now I see the problem: cdbs->cpu is initialized only on the leader cpu and this is working by coincidence on my system, while cdbs->time_stamp is initialized only on the leader cpu, and that should be correct even when cpu hotplugging as that's reinitialized every time. That's a fix so I'll send a patch just to set ->cpu into the for_each_cpu cycle. Thanks, Fabio > And so, might be initialized with 0 as its a global variable... But if you > offline cpu 0 and online it back, then policy->cpu would be 1 and this logic, > which worked by mistake will fail. > > + mutex_lock(&dbs_info->cdbs.timer_mutex); > + > + time_now = ktime_get(); > + delta_us = ktime_us_delta(time_now, dbs_info->cdbs.time_stamp); > > and so as this. > > Correct? > -- > To unsubscribe from this list: send the line "unsubscribe cpufreq" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- Fabio Baltieri