linux-kernel - Re: [PATCH V2 1/4] cpufreq: stats: Defer stats update to cpufreq_stats_record

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Fri, 25 Sep 2020 16:56:51 +0530
From:   Viresh Kumar <viresh.kumar@...aro.org>
To:     "Rafael J. Wysocki" <rafael@...nel.org>
Cc:     Rafael Wysocki <rjw@...ysocki.net>,
        Linux PM <linux-pm@...r.kernel.org>,
        Vincent Guittot <vincent.guittot@...aro.org>,
        Lukasz Luba <lukasz.luba@....com>,
        Cristian Marussi <cristian.marussi@....com>,
        Sudeep Holla <sudeep.holla@....com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH V2 1/4] cpufreq: stats: Defer stats update to cpufreq_stats_record_transition()

On Fri, 25 Sep 2020 at 16:39, Rafael J. Wysocki <rafael@...nel.org> wrote:
>
> On Fri, Sep 25, 2020 at 12:58 PM Viresh Kumar <viresh.kumar@...aro.org> wrote:
> >
> > On 25-09-20, 12:04, Rafael J. Wysocki wrote:
> > > I'm actually wondering if reset_time is necessary at all.
> > >
> > > If cpufreq_stats_record_transition() is the only updater of the stats,
> > > which will be the case after applying this series IIUC, it may as well
> > > simply set the new starting point and discard all of the data
> > > collected so far if reset_pending is set.
> > >
> > > IOW, the time when the reset has been requested isn't particularly
> > > relevant IMV (and it is not exact anyway), because the user is
> > > basically asking for discarding "history" and that may very well be
> > > interpreted to include the current sample.
> >
> > There are times when this would be visible to userspace and won't look nice.
> >
> > Like, set governor to performance, reset the stats and after 10 seconds, read
> > the stats again, everything will be 0.
>
> Unless I'm missing something, the real reset happens when
> cpufreq_stats_record_transition() runs next time, so the old stats
> will still be visible at that point, won't they?

For userspace the stats shouldn't be visible after reset is requested
by it and so with
this series, we check for reset-pending in all the show_*() helpers and print
stats since the time reset was requested.

> > Because cpufreq_stats_record_transition()
> > doesn't get called at all here, we would never clear them until the time
> > governor is changed and so we need to keep a track of reset-time.
>
> Or trigger a forced update.

That would add races while updating the actual stats. And so I found the current
way to be more reliable.