Re: [PATCH 1/9] perf header: Tolerate inconsistent HEADER_GROUP_DESC

Adrian Hunter <[email protected]>
Newsgroups org.kernel.vger.linux-doc,org.infradead.lists.linux-arm-kernel,org.kernel.vger.linux-perf-users
Organization Intel Finland Oy, Registered Address: c/o Alberga Business Park, 6 krs, Bertel Jungin Aukio 5, 02600 Espoo, Business Identity Code: 0357606 - 4, Domiciled in Helsinki
Message-ID <[email protected]>
On 03/08/2026 12:06, Amir Ayupov wrote:
> process_group_desc() rejects the entire perf.data file ("invalid group
> desc" -> "incompatible file format") whenever the group description is
> inconsistent with the event list. Group information is optional metadata
> and is not needed to decode samples, so a single bad group descriptor
> should not make an otherwise valid file unreadable.
> 
> This is observable with AUX area recordings (Arm CoreSight ETM, Intel PT)
> that use aux-action pause/resume: the aux-action regrouping inflates the
> AUX group leader's nr_members, producing a group descriptor that the

Regrouping by auxtrace_parse_aux_action() is done before group
description is written, so it should match.  What exactly is
wrong with it?

Can you give example perf commands?

> strict reader rejects, even though the file is otherwise fine (older perf
> and other tooling read it by rebuilding groups from event records).
> 
> Warn and fall back to a consistent ungrouped event list instead of
> failing the read.

Seems like just papering over the issue.  Why can't the group
description be written correctly in the first place?

> 
> Signed-off-by: Amir Ayupov <[email protected]>
> ---
>  tools/perf/util/header.c | 42 +++++++++++++++++++++++++++++++---------
>  1 file changed, 33 insertions(+), 9 deletions(-)
> 
> diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
> index e90e541f546b4..ddc59624d639b 100644
> --- a/tools/perf/util/header.c
> +++ b/tools/perf/util/header.c
> @@ -3373,6 +3373,19 @@ static int process_group_desc(struct feat_fd *ff, void *data __maybe_unused)
>  	i = nr = 0;
>  	evlist__for_each_entry(session->evlist, evsel) {
>  		if (i < nr_groups && evsel->core.idx == (int) desc[i].leader_idx) {
> +			if (!desc[i].nr_members)
> +				goto out_inconsistent;
> +
> +			if (nr > 0) {
> +				/*
> +				 * A new leader was found before the previous
> +				 * group's members were all consumed, so the
> +				 * group description is inconsistent with the
> +				 * event list.
> +				 */
> +				goto out_inconsistent;
> +			}
> +
>  			evsel__set_leader(evsel, evsel);
>  			/* {anon_group} is a dummy name */
>  			if (strcmp(desc[i].name, "{anon_group}")) {
> @@ -3381,11 +3394,6 @@ static int process_group_desc(struct feat_fd *ff, void *data __maybe_unused)
>  			}
>  			evsel->core.nr_members = desc[i].nr_members;
>  
> -			if (i >= nr_groups || nr > 0) {
> -				pr_debug("invalid group desc\n");
> -				goto out_free;
> -			}
> -
>  			leader = evsel;
>  			nr = evsel->core.nr_members - 1;
>  			i++;
> @@ -3397,10 +3405,8 @@ static int process_group_desc(struct feat_fd *ff, void *data __maybe_unused)
>  		}
>  	}
>  
> -	if (i != nr_groups || nr != 0) {
> -		pr_debug("invalid group desc\n");
> -		goto out_free;
> -	}
> +	if (i != nr_groups || nr != 0)
> +		goto out_inconsistent;
>  
>  	ret = 0;
>  out_free:
> @@ -3409,6 +3415,24 @@ static int process_group_desc(struct feat_fd *ff, void *data __maybe_unused)
>  	free(desc);
>  
>  	return ret;
> +
> +out_inconsistent:
> +	/*
> +	 * Group information is optional metadata and is not required to decode
> +	 * samples. Rather than rejecting the whole file, warn and fall back to
> +	 * a consistent ungrouped event list. This can happen with otherwise
> +	 * valid perf.data files, e.g. AUX area (Intel PT, Arm CoreSight ETM)
> +	 * recordings using aux-action pause/resume.
> +	 */
> +	pr_warning("Inconsistent HEADER_GROUP_DESC, ignoring group information\n");
> +	env->nr_groups = 0;
> +	evlist__for_each_entry(session->evlist, evsel) {
> +		evsel__set_leader(evsel, evsel);
> +		evsel->core.nr_members = 1;
> +		zfree(&evsel->group_name);
> +	}
> +	ret = 0;
> +	goto out_free;
>  }
>  
>  static int process_auxtrace(struct feat_fd *ff, void *data __maybe_unused)
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.