Re: [PATCH 3/5] intel_pstate: remove intel_pstate.get()

[Rafael J. Wysocki]

On Sat, Jun 17, 2017 at 3:21 AM, Rafael J. Wysocki <rjw@xxxxxxxxxxxxx> 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