Messages in this thread | | | From | Danny Lin <> | Subject | Re: [PATCH] cpufreq: Record stats when fast switching is enabled | Date | Thu, 2 Jul 2020 18:00:09 -0700 |
| |
On Thu, Jan 31, 2019 at 2:14 AM, Rafael J. Wysocki wrote: > On Thu, Jan 31, 2019 at 11:07 AM Viresh Kumar <viresh.kumar@linaro.org> wrote: > > > > On 31-01-19, 11:03, Rafael J. Wysocki wrote: > > > On Thu, Jan 31, 2019 at 9:30 AM Viresh Kumar <viresh.kumar@linaro.org> wrote: > > > > > > > > The only problem that I can think of (or recall) is that this routine > > > > also gets called when time_in_state sysfs file is read and that can > > > > end up taking lock which the scheduler's hotpath will wait for. > > > > > > What about the extra locking overhead in the scheduler context? > > > > What about using READ_ONCE/WRITE_ONCE here ? Not sure if we really > > need locking in this particular case. > > If that works, then fine, but ISTR some synchronization issues related to that.
Maybe using READ/WRITE_ONCE for time_in_state is problematic, but is there any reason why atomics wouldn't work for this? As far as I can tell, atomics are necessary to protect time_in_state due to its multi-step add operation, and READ/WRITE_ONCE can be used for last_time because all operations on it are single-op sets/gets.
I've been using the setup described above on a downstream arm64 4.14 kernel for nearly a year with no issues. I haven't noticed any significant anomalies in the stats so far. The system in question has 8 CPUs split into 3 cpufreq policies and fast switch is used with the schedutil governor, so it should be exercising the stats update path enough.
Sorry for bumping an old thread.
| |