Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Tue, 9 Jun 2020 11:16:47 -0300
From:   Arnaldo Carvalho de Melo <acme@redhat.com>
To:     Ian Rogers <irogers@google.com>
Cc:     Stephane Eranian <eranian@google.com>,
        LKML <linux-kernel@vger.kernel.org>,
        Jiri Olsa <jolsa@redhat.com>,
        Peter Zijlstra <peterz@infradead.org>, mingo@elte.hu
Subject: Re: [PATCH] perf headers: fix processing of pmu_mappings
Message-ID: <20200609141647.GA10172@redhat.com>
References: <20200608161805.65841-1-eranian@google.com>
 <CAP-5=fXmrbBqDj6udGJCLHF5ePERr1S5qTKGZZAUBC1EmA-8LQ@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CAP-5=fXmrbBqDj6udGJCLHF5ePERr1S5qTKGZZAUBC1EmA-8LQ@mail.gmail.com>
User-Agent: Mutt/1.5.20 (2009-12-10)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Em Mon, Jun 08, 2020 at 09:52:43AM -0700, Ian Rogers escreveu:
> On Mon, Jun 8, 2020 at 9:18 AM Stephane Eranian <eranian@google.com> wrote:
> > This patch fixes a bug in process_pmu_mappings() where the code
> > would not produce an env->pmu_mappings string that was easily parsable.
> > The function parses the PMU_MAPPING header information into a string
> > consisting of value:name pairs where value is the PMU type identifier
> > and name is the PMU name, e.g., 10:ibs_fetch. As it was, the code
> > was producing a truncated string with only the first pair showing
> > even though the rest was there but after the \0.
> > This patch fixes the problem byt adding a proper white space between
> > pairs and moving the \0 termination to the end. With this patch applied,
> > all pairs appear and are easily parsed.

> > Before:
> > 14:amd_iommu_1

> > After:
> > 14:amd_iommu_1 7:uprobe 5:breakpoint 10:amd_l3 19:amd_iommu_6 8:power 4:cpu 17:amd_iommu_4 15:amd_iommu_2 1:software 6:kprobe 13:amd_iommu_0 9:amd_df 20:amd_iommu_7 18:amd_iommu_5 2:tracepoint 21:msr 12:ibs_op 16:amd_iommu_3 11:ibs_fetch

Please check print_pmu_mappings() in tools/perf/util/header.c

Before your patch:

  [root@five ~]# perf report --header-only | grep "pmu mappings"
  # pmu mappings: amd_df = 8, software = 1, ibs_op = 11, ibs_fetch = 10, uprobe = 7, cpu = 4, amd_iommu_0 = 12, breakpoint = 5, amd_l3 = 9, tracepoint = 2, kprobe = 6, msr = 13
  [root@five ~]#

After your patch:

  [root@five ~]# perf report --header-only | grep "pmu mappings"
  # pmu mappings: amd_df 1:software 11:ibs_op 10:ibs_fetch 7:uprobe 4:cpu 12:amd_iommu_0 5:breakpoint 9:amd_l3 2:tracepoint 6:kprobe 13:msr = 8# pmu mappings: unable to read
  [root@five ~]# 

I think having it space separated, as you propose, is best, but haven't
checked if there are other cases that process ff->ph->env.pmu_mappings
and expect it to be \0 separated, like print_pmu_mappings().

Regards,

- Arnaldo