linux-kernel - Re: [PATCH 3/5] intel_pstate: remove intel

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAJZ5v0itMGFq3PZSoy23ybSXfhP_-v4WKWXCT6A0dOssED-gwA@mail.gmail.com>
Date:   Sat, 17 Jun 2017 03:40:09 +0200
From:   "Rafael J. Wysocki" <rafael@...nel.org>
To:     "Rafael J. Wysocki" <rjw@...ysocki.net>
Cc:     Len Brown <lenb@...nel.org>, X86 ML <x86@...nel.org>,
        Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>,
        "H. Peter Anvin" <hpa@...ux.intel.com>,
        Peter Zijlstra <peterz@...radead.org>,
        "Rafael J. Wysocki" <rafael@...nel.org>,
        Linux PM list <linux-pm@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Len Brown <len.brown@...el.com>
Subject: Re: [PATCH 3/5] intel_pstate: remove intel_pstate.get()

On Sat, Jun 17, 2017 at 3:21 AM, Rafael J. Wysocki <rjw@...ysocki.net> wrote:
> On Friday, June 16, 2017 09:10:39 PM Len Brown wrote:
>> >> >> -             get_avg_frequency(cpu),
>> >> >> +             aperfmperf_khz_on_cpu(cpu->cpu),
>> >>
>> >> Note that I deleted the line above in an updated version of this patch
>> >> that I'm ready to send out.
>> >>
>> >> There were a couple of problems with it.
>> >> The first is that it was  ugly that tracing (which occurs only in the
>> >> SW governor case)
>> >> could shorten the measurement interval as seen by the sysfs interface.
>> >>
>> >> The second is that this trace point can be called with irqs off,
>> >> and smp_call_function_single() will WARN when called with irqs off.
>> >>
>> >> Srinivas Acked that I simply remove this field from the tracepoint --
>> >> as it is redundant to calculate it in the kernel when we are already
>> >> exporting the raw values of aperf and mperf.
>> >
>> > This changes the tracepoint format and I know about a couple of user space
>> > scripts that consume these tracepoints though.
>> > What would be wrong with leaving it as is?
>>
>> I'm fine keeping get_avg_frequency() for the purpose of just this tracepoint
>> compatibility, if you think it is useful to do so.
>>
>> I removed it because its only function was the (removed) intel_pstate.get()
>> and this tracepoint.  And this tracepoint already includes aperf and mperf,
>> which can be used to calculate frequency in user-space, if desired.
>> Srinivas Acked' that updating his user-space script would be fine --
>> dunno if that is sufficient.
>
> Well, we can try to make this change and see if there are any complaints
> about it.
>
> But the Srinivas' script is in the tree, so it would be good to update it too
> along with the tracepoint.

On a second thought, in order to compute the frequency, user space
needs to know the scaling and the max_pstate_physical value too, which
may not be straightforward to obtain (on some Atoms, for example).

So why don't we leave the tracepoint as is for now?

Thanks,
Rafael