Re: [PATCH 3/3 v3] cpufreq: governor: Replace timers with utilization update callbacks

[Viresh Kumar <viresh.kumar@linaro.org>]

On 06-02-16, 00:10, Rafael J. Wysocki wrote:
> On Friday, February 05, 2016 08:17:56 PM Viresh Kumar wrote:
> > Okay, how about this then.
> > 
> > We do some computations here and based on them, conditionally want to
> > update sample_delay_ns. Because there is no penalty now, in terms of
> > removing/adding timers/wq, etc, why shouldn't we simply update the
> > sample_delay_ns everytime without any checks? That would mean that the
> > change of sampling rate is effective immediately, what can be better than that?
> 
> Yes, we can do that.
> 
> There is a small concern about updating in parallel with dbs_work_handler()
> in which case we may overwrite the (hopefully already correct) sample_delay_ns
> value that it has just written, but then it will be corrected next time we
> take a sample, so it shouldn't be a big deal.
> 
> OK, I'll update the patch to do that.

Great.

> > Also, we should do the same from update-sampling-rate of conservative
> > governor as well.
> 
> Let's just not change the whole world in one patch, OK?

Yeah, I wasn't asking to update in the same patch, but just that we
should do that as well.

> > I did bit of that this morning, and there weren't any serious issues as
> > as far as I could see :)
> 
> The case I'm mostly concerned about is when update_sampling_rate() looks
> at a CPU with a policy completely unrelated to the dbs_data it was called
> for.  In that case the "shared" object may just go away from under it at
> any time while it is looking at that object in theory.

Right, a way (ofcourse we should try find something better) is to move
that update to a separate work item, just as I did it in my patch..

But, I am quite sure we can get that fixed.

-- 
viresh