Re: [RFC 04/10] drm/i915: Expose a PMU interface for perf queries

["Rogozhkin, Dmitry V" <dmitry.v.rogozhkin@intel.com>]

On Wed, 2017-08-23 at 20:22 +0200, Peter Zijlstra wrote:
> On Wed, Aug 23, 2017 at 05:51:38PM +0000, Rogozhkin, Dmitry V wrote:
> 
> > Anyhow, returning to the metrics i915 exposes. Some metrics are just
> > exposure of some counters supported already inside i915 PMU which do not
> > require any special sampling: at any given moment you can request the
> > counter value (these are interrupts counts, i915 power consumption).
> 
> > Other metrics are similar to the ever-existing which I just described,
> > but they require activation for i915 to start to count them - this is
> > done on the event initialization (these are engine busy stats).
> 
> Right, so depending on how expensive this activation is and if it can be
> done without scheduling, there are two options:
> 
>  1) activate/deactivate from pmu::start()/pmu::stop()
>  2) activate/deactivate from pmu::event_init()/event->destroy() and
>     disregard all counting between pmu::stop() and pmu::start().
> 
> > Finally, there is a third group which require sampling counting: they
> > are needed to be initialized and i915 pmu starts an internal timer to
> > count these values (these are some engines characteristics referenced
> > in the code as QUEUED, SEMA, WAIT).
> 
> So uncore PMUs can't really do sampling. That is, perf defines sampling
> as interrupting the relevant task and then providing things like the
> %RIP value at interrupt time. Since uncore activity cannot be associated
> with any one task, no sampling allowed.

I read this as we need to add:
static int i915_pmu_event_init(struct perf_event *event)
{
...
	/* Sampling not supported yet */
	if (hwc->sample_period)
		return -EINVAL;
...
}
And deny sampling period for our events, right? And what should happen
is that the following command will start to fail: "perf stat -e ev -a -I
100"? Essentially you are saying that uncore PMUs should work only in
the "get value when asked" mode, for perf-stat it will add results in
the end of the run. However, there is no strict perf subsystem denying
of the uncore event sampling, but this is delegated to the each PMU
implementation. Is that correct? I wonder, how this is supposed to work
in case PMU is capable to produce both global and per-task metrics?


> 
> Now, I'm thinking that what i915 does is slightly different, it doesn't
> provide registers to read out the counter state, but instead
> periodically writes state snapshots into some memory buffer, right?
> 
> That's a bit tricky, maybe the best fit would be what PPC HV 24x7 does.
> They create an event-group, that is a set of counters that are
> co-scheduled, matching the set of counters they get from the HV
> interface (or a subset) and then sys_read() will use a TXN_READ to
> group-read the entire thing at once. In your case it could consume the
> last state snapshot instead of request one (or wait for the next,
> whatever works best).
> 
> Would that work?

Hm. Not sure I understand this with my userspace brain:). Why group
read? Just to eliminate overhead of doing same/similar things for all
the metrics? Anyway, what completely slept away from me is how this
relates to the sampling if it is not allowed for uncore PMUs? So, there
will or will not be a sampling?

Essentially, i915 PMU for such "sampling" metrics is implemented in the
following way:
1. If such a metric is requested i915 PMU starts to do some internal
sampling which is not exposed to perf subsystem. I think there is some
timer staff currently associated with this.
2. On each received i915_pmu_event_read() we just immediately return
those values we have gathered so far internally
3. Since we were not aware about no uncore sampling policy, we have some
code for sampling exposed to perf subsystem, but this is non-related to
#1. Essentially it is used to just call i915_pmu_event_read(). Here is a
code:

static DEFINE_PER_CPU(struct pt_regs, i915_pmu_pt_regs);

static enum hrtimer_restart hrtimer_sample(struct hrtimer *hrtimer)
{
	struct pt_regs *regs = this_cpu_ptr(&i915_pmu_pt_regs);
	struct perf_sample_data data;
	struct perf_event *event;
	u64 period;

	event = container_of(hrtimer, struct perf_event, hw.hrtimer);
	if (event->state != PERF_EVENT_STATE_ACTIVE)
		return HRTIMER_NORESTART;

	event->pmu->read(event);

	perf_sample_data_init(&data, 0, event->hw.last_period);
	perf_event_overflow(event, &data, regs);

	period = max_t(u64, 10000, event->hw.sample_period);
	hrtimer_forward_now(hrtimer, ns_to_ktime(period));
	return HRTIMER_RESTART;
}


By the way, I think Tvrtko had a question on why he needs this
"DEFINE_PER_CPU(struct pt_regs, i915_pmu_pt_regs)" staff which looked
somewhat strange? Here is a quote: "Also, once I exported the events for
enumeration and tried accessing them with perf, I had to create fake
pt_regs to pass to perf_event_overflow."

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx