Messages in this thread | | | Date | Sun, 7 Feb 2016 14:40:40 +0530 | From | Viresh Kumar <> | Subject | Re: [PATCH 3/3 v3] cpufreq: governor: Replace timers with utilization update callbacks |
| |
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
| |