On 14/06/2014 06:45 μμ, Doug Smythies wrote: > I am sorry to be late chiming in on this one. > > On 2014.06.10 09:27 Stratos Karafotis wrote: >> On 10/06/2014 07:05 μμ, Dirk Brandewie wrote: >> On 06/09/2014 02:00 PM, Stratos Karafotis wrote: >>> Store busy_scaled value to avoid to duplicate call of >>> intel_pstate_get_scaled_busy on every sampling interval. >>> >>> >>> The second call *only* happens if the tracepoint is being used otherwise >>> the whole function call to trace_pstate_sample() is a noop. > >> Yes, I'm sorry, I forgot to add this in my changelog. I have written this >> in cover letter. >> I made this change mostly to support patch 3/7. > >>> This makes the code less readable IMHO the reader is left wondering >>> how cpu->sample.busy_scaled was set in intel_pstate_adjust_busy_pstate() >>> > >> I agree that the the original code is more readable. If we don't care >> about the small overhead when tracing is on and forget patch 3/7, >> of course the original code is by far better. > > Actually, when reading the code, I found it odd to call the function > twice. > > However by far the much more important issue here, in my opinion, > is that if one is using the tracepoint stuff, then the second call > to intel_pstate_get_scaled_busy can give a different result than > the first call. Why? Because "cpu->pstate.current_pstate" may have > changed between the two calls. > > In the end the user (me in this case) of the tracepoint stuff can > end up pulling (what's left of) their hair out and going around in > circles attempting to figure out why doing the so simple math by > hand doesn't seem to agree with the tracepoint data.
:) > As a side note: I am now pulling the tracepoint data into a > spreadsheet and calculating what "scaled" should be myself. > I think you are right. Tracepoint data might be inconsistent. I will re-submit this patch in v2 series, updating the changelog. Thanks for pointing this out! Stratos -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
