Re: [GIT PULL 0/8] perf/pt -> Intel PT/BTS

Ingo Molnar
2015-07-02T19:01:13Z

* Adrian Hunter <adrian.hunter@intel.com> wrote:

> > So I really think we need an extended error reporting feature on the perf
> > kernel side, so that a 'natural' error (plus a string) is reported back to
> > tooling, instead of the current -EINVAL.
> >
> > No need to do it for everything, doing it for BTS and related functionality
> > would be a good first step to start this.
> >
> > If you are interested you could try this, or I can try to write something
> > (after the merge window).
>
> Don't have time sorry.

So who on the Intel side has time to finish Intel/PT support properly?

As a first step I think we need to disable the Intel/PT kernel side - it was
merged with the express promise that proper tooling would come along promptly,
but if that's not happening due to lack of resources, then the kernel side is
obviously in limbo as well.

> > Does this make sense to you?
>
> Normally the warts of syscalls are hidden by libraries. I am not sure a library
> couldn't do just as well with the advantage that it would work for old kernels
> too. A library could also have information about multiple architectures, not
> just the one that is actually running e.g. your BTS example won't work on ARM.

The method I suggested works _both_ with old and new kernels, with the new kernel
simply giving better error feedback.

And these are not warts really, it's simply a threshold issue: above a certain
complexity of features it makes sense to introduce a qualitatively better error
reporting interface. When I tested Intel PT support I happened to go past that
threshold and it became obvious that we need it.

> A library could provide a similar API to the one you described. Either it just
> does the syscall and returns, or if extended error information is requested, it
> probes the API with various options, checks perf_event_paranoid, and generally
> uses whatever information it can to best figure out what went wrong.

So I think eventually a proper libperf will crystalize out of perf's syscall
wrappers (I suggested it for a long time), but this has very little impact on the
end user who doesn't care whether it's done in perf, or in some intermediate
library.

So it can be done in the current perf code just as much - when libperf gets
introduced it will be factored out into tools/lib/perf/ without much problem and
then other tools can use it too.

> > I.e. if the BTS driver was not created, we'd still have
> > /sys/bus/event_source/devices/intel_bts/error (and no other file), which gives
> > tooling a good way to discover why a particular PMU is not available. This
> > adds a tiny bit more RAM overhead, but it's for the better I think, because
> > tooling could be a lot more certain about what the capabilities of the kernel
> > are.
>
> That presumes the user understands what is or is not available on their
> architecture, because on a non-x86 architecture they still get nothing
> x86-specific.
>
> It also runs a bit against the way config works e.g. even if the PMU driver is
> config'ed out it still has to provide a sysfs interface.

I partially agree - but it's really a problem that we currently only get two
states:

- the PMU driver is there in sysfs
- the PMU driver is not there in sysfs

while we really want to a lot more about why it's not there - and it's the kernel
that knows this best, not tooling.

So maybe instead of having a separate directory, we could have a sysfs file that
listed all the PMU drivers that the kernel knows about, with a status line that
tells us whether they are:

- not configured
- configured but runtime disabled due to reason X or Y or Z
- configured and enabled

OTOH having the directories with a single file in them is a close substitute, and
better meets the sysfs 'one file, one value' principle.

> So, isn't the patch I proposed sufficient for now?

Well, no, because it really does the check in the wrong place and introduces
possible friction between what tooling thinks is the supported environment and
what the kernel thinks - and thus postpones the problem.

I could live with this current solution as the initial version if a subsequent
effort fixes it up properly by adding much better error reporting infrastructure,
but the 'no time' comment makes me doubt the whole Intel/PT feature set.

Thanks,

Ingo