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.ku...@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.ku...@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.
