On Mon, Dec 10, 2018 at 11:51 PM Peter Zijlstra [off-list ref] wrote:

On Mon, Dec 10, 2018 at 10:36:40PM +0100, Rafael J. Wysocki wrote:

quoted

On Mon, Dec 10, 2018 at 1:21 PM Peter Zijlstra [off-list ref] wrote:

quoted

quoted

One question on this; why is this tracked unconditionally?

Because I didn't quite see how to make that conditional in a sensible way.

Something like:

        if (static_branch_unlikely(__tracepoint_idle_above) ||
            static_branch_unlikely(__tracepoint_idle_below)) {

                // do stuff that calls trace_idle_above() /
                // trace_idle_below().

        }

quoted

These things are counters and counting with the help of tracepoints
isn't particularly convenient (and one needs debugfs to be there to
use tracepoints and they require root access etc).

Root only should not be a problem for a developer; and aren't these
numbers only really interesting if you're prodding at the idle governor?

What about regression testing?

quoted

quoted

Would not a tracepoint be better?; then there is no overhead in the
normal case where nobody gives a crap about these here numbers.

There is an existing tracepoint that in principle could be used to
produce this information, but it is such a major PITA in practice that
nobody does that.  Guess why. :-)

Sounds like you need to ship a convenient script or something :-)

That would be a very ugly script.  Also I'd need to expose
drv->states[i].disabled somehow (that's not accessible from user space
now).

quoted

Also, the "usage" and "time" counters are there in sysfs, so why not these two?

And is the overhead really that horrible?

Dunno; it could be cold cachelines, at which point it can be fairly
expensive. Also, being stuck with API is fairly horrible if you want to
'fix' it.

All of the cache lines involved should've been touched earlier in this
code path by the governor.  At least menu and the new one both touch
them.

The API part I'm not too worried about.  I know it is useful and two
other people have told that to me already. :-)