Re: [PATCH 1/4] perf: Add error message for unsupported branch stack cases

[Anshuman Khandual]

Hi James,

On 3/7/22 22:49, James Clark wrote:
> EOPNOTSUPP is a possible return value when branch stacks are requested
> but they aren't enabled in the kernel or hardware. It's also returned if
> they aren't supported on the specific event type. The currently printed

specific event type ? Events carrying certain branch sampling/filter flags
which could not be supported in the kernel on a given platform ?

> error message about sampling/overflow-interrupts is not correct in this
> case.
> 
> Add a check for branch stacks before sample_period is checked because
> sample_period is also set (to the default value) when using branch
> stacks.

Makes sense.

> 
> Before this change (when branch stacks aren't supported):
> 
>   perf record -j any
>   Error:
>   cycles: PMU Hardware doesn't support sampling/overflow-interrupts. Try 'perf stat'
> 
> After this change:
> 
>   perf record -j any
>   Error:
>   cycles: PMU Hardware or event type doesn't support branch stack sampling.

Indeed better in explaining what went wrong.

> 
> Signed-off-by: James Clark <james.clark@xxxxxxx>
> ---
>  tools/perf/util/evsel.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
> index 22d3267ce294..4e10a4ec11c7 100644
> --- a/tools/perf/util/evsel.c
> +++ b/tools/perf/util/evsel.c
> @@ -2909,6 +2909,10 @@ int evsel__open_strerror(struct evsel *evsel, struct target *target,
>  	 "No such device - did you specify an out-of-range profile CPU?");
>  		break;
>  	case EOPNOTSUPP:
> +		if (evsel->core.attr.sample_type & PERF_SAMPLE_BRANCH_STACK)
> +			return scnprintf(msg, size,
> +	"%s: PMU Hardware or event type doesn't support branch stack sampling.",
> +					 evsel__name(evsel));

As this is being added right at the beginning for returned EOPNOTSUPP error,
previous fall through behaviour for all other cases will be preserved.

>  		if (evsel->core.attr.aux_output)
>  			return scnprintf(msg, size,
>  	"%s: PMU Hardware doesn't support 'aux_output' feature",

Reviewed-by: Anshuman Khandual <anshuman.khandual@xxxxxxx>