Hello Viresh, On Wed, Jan 30, 2013 at 10:41:08PM +0530, Viresh Kumar wrote: > On 30 January 2013 22:16, Fabio Baltieri <fabio.baltieri@xxxxxxxxxx> 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@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html -- Fabio Baltieri -- To unsubscribe from this list: send the line "unsubscribe cpufreq" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
