Re: [PATCH] libperf: Fix read_size calculations

[Arnaldo Carvalho de Melo <acme@kernel.org>]

Em Wed, Feb 23, 2022 at 03:13:49PM +0200, Tzvetomir Stoyanov (VMware) escreveu:
> Reading the counters from perf descriptor fails because of size
> mismatch when PERF_FORMAT_GROUP is set. The group count must include the
> parent and the count of siblings.

Yeah, it should match the calculation done in the kernel, in
__perf_event_read_size(), that has...

static void __perf_event_read_size(struct perf_event *event, int nr_siblings)
{
        int entry = sizeof(u64); /* value */
        int size = 0;
        int nr = 1;

        if (event->attr.read_format & PERF_FORMAT_TOTAL_TIME_ENABLED)
                size += sizeof(u64);

        if (event->attr.read_format & PERF_FORMAT_TOTAL_TIME_RUNNING)
                size += sizeof(u64);

        if (event->attr.read_format & PERF_FORMAT_ID)
                entry += sizeof(u64);

        if (event->attr.read_format & PERF_FORMAT_GROUP) {
                nr += nr_siblings;
                size += sizeof(u64);
        }

        size += entry * nr;
        event->read_size = size;
}

So this bug has been with us since forever (well, 2017), so we need:

Fixes: 5c30af92f2b1e9d8 ("libperf: Adopt perf_evsel__read() function from tools/perf")

To fix it in libperf and probably:

Fixes: de63403bfd14ae8d ("perf tools: Add perf_evsel__read_size function")

To get this to stable kernels versions predating the move of this
function to libperf.

Jiri?

- Arnaldo
 
> Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
> ---
>  tools/lib/perf/evsel.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/lib/perf/evsel.c b/tools/lib/perf/evsel.c
> index 210ea7c06ce8..4597a53c0152 100644
> --- a/tools/lib/perf/evsel.c
> +++ b/tools/lib/perf/evsel.c
> @@ -299,7 +299,7 @@ int perf_evsel__read_size(struct perf_evsel *evsel)
>  		entry += sizeof(u64);
>  
>  	if (read_format & PERF_FORMAT_GROUP) {
> -		nr = evsel->nr_members;
> +		nr += evsel->nr_members;
>  		size += sizeof(u64);
>  	}
>  
> -- 
> 2.34.1

-- 

- Arnaldo