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

[Alexander Shishkin]

Ingo Molnar <mingo@xxxxxxxxxx> writes:

> 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).
>
> So the idea would be to convert such opaque error returns:
>
>         if (attr->config == PERF_COUNT_HW_BRANCH_INSTRUCTIONS &&
>             !attr->freq && hwc->sample_period == 1) {
>                 /* BTS is not supported by this architecture. */
>                 if (!x86_pmu.bts_active)
>                         return -EOPNOTSUPP;
>
> into:
>
> 			return perf_err(event, -EOPNOTSUPP, "The BTS hardware feature is not available on this CPU.");

So I poked around this a bit and came up with the patch below to give
this topic some more momentum.

Your average error will then be like this:

#define PERF_MODNAME "perf/x86"

...

               /* BTS is currently only allowed for user-mode. */
               if (!attr->exclude_kernel)
                       return perf_err(event, -EOPNOTSUPP,
                                       "BTS sampling not allowed for kernel space");

which in userspace will translate into:

---cut---
$ perf record -e branches:uk -c1 ls
kernel says: {
        "code": -95,
        "module": "perf/x86",
        "message": "BTS sampling not allowed for kernel space"
}

Error:
No hardware sampling interrupt available.
No APIC? If so then you can boot the kernel with the "lapic" boot parameter to force-enable it.
---cut---

The way I hacked it into tools/perf, it still prints out the old error
message (which, to prove the point once again, is neither here nor
there).

The patch below attaches the error to struct perf_event or copies it
directly to user's buffer in case if we don't have an event at the point
of error. Maybe a slightly easier way of doing it would be to strap it
to task_struct instead, but let's not stir the pot too much this time
around.

Also, to add an extra spin, the report is formatted as a JSON object so
that we could include both human-readable and machine-readable bits.