Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20230825152552.112913-1-namhyung@kernel.org> <20230825152552.112913-3-namhyung@kernel.org>
 <150a4c24-6082-cf74-035d-c6cf4dacc46d@intel.com>
In-Reply-To: <150a4c24-6082-cf74-035d-c6cf4dacc46d@intel.com>
From:   Namhyung Kim <namhyung@kernel.org>
Date:   Tue, 29 Aug 2023 19:15:58 -0700
Message-ID: <CAM9d7cg5vaD6zUThLWSpL-j3nXJgs3PK=_ensRw8P0Xye-VcfA@mail.gmail.com>
Subject: Re: [PATCH v2 3/4] perf tools: Convert to perf_record_header_attr_id()
To:     Adrian Hunter <adrian.hunter@intel.com>
Cc:     Arnaldo Carvalho de Melo <acme@kernel.org>,
        Jiri Olsa <jolsa@kernel.org>, Ian Rogers <irogers@google.com>,
        Peter Zijlstra <peterz@infradead.org>,
        Ingo Molnar <mingo@kernel.org>,
        LKML <linux-kernel@vger.kernel.org>,
        linux-perf-users@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 8BIT
Precedence: bulk

Hi Adrian,

On Mon, Aug 28, 2023 at 12:59 AM Adrian Hunter <adrian.hunter@intel.com> wrote:
>
> On 25/08/23 18:25, Namhyung Kim wrote:
> > Instead of accessing the attr.id directly, use the
> > perf_record_header_attr_id() helper to handle old versions.
> >
> > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> > ---
> >  tools/perf/util/header.c           | 2 +-
> >  tools/perf/util/session.c          | 4 ++--
> >  tools/perf/util/synthetic-events.c | 2 +-
> >  3 files changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
> > index f89321cbfdee..63967c34157d 100644
> > --- a/tools/perf/util/header.c
> > +++ b/tools/perf/util/header.c
> > @@ -4408,7 +4408,7 @@ int perf_event__process_attr(struct perf_tool *tool __maybe_unused,
> >       if (perf_evsel__alloc_id(&evsel->core, 1, n_ids))
> >               return -ENOMEM;
> >
> > -     ids = (void *)&event->attr.attr + event->attr.attr.size;
> > +     ids = perf_record_header_attr_id(event);
> >       for (i = 0; i < n_ids; i++) {
> >               perf_evlist__id_add(&evlist->core, &evsel->core, 0, i, ids[i]);
> >       }
> > diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
> > index 00d18c74c090..1e9aa8ed15b6 100644
> > --- a/tools/perf/util/session.c
> > +++ b/tools/perf/util/session.c
> > @@ -833,8 +833,8 @@ static void perf_event__hdr_attr_swap(union perf_event *event,
> >       perf_event__attr_swap(&event->attr.attr);
> >
> >       size = event->header.size;
> > -     size -= (void *)&event->attr.id - (void *)event;
> > -     mem_bswap_64(event->attr.id, size);
> > +     size -= perf_record_header_attr_id(event) - (void *)event;
> > +     mem_bswap_64(perf_record_header_attr_id(event), size);
>
> Should this and below also be a fix for stable?

Then it'd need the perf_record_header_attr_id() change as well.
Also I'm not sure if it's really needed for stable as it's a pipe output
(saved to a file though) so very unlikely to have a different endian.

Thanks,
Namhyung

>
> >  }
> >
> >  static void perf_event__event_update_swap(union perf_event *event,
> > diff --git a/tools/perf/util/synthetic-events.c b/tools/perf/util/synthetic-events.c
> > index 45714a2785fd..a0579c7d7b9e 100644
> > --- a/tools/perf/util/synthetic-events.c
> > +++ b/tools/perf/util/synthetic-events.c
> > @@ -2145,7 +2145,7 @@ int perf_event__synthesize_attr(struct perf_tool *tool, struct perf_event_attr *
> >               return -ENOMEM;
> >
> >       ev->attr.attr = *attr;
> > -     memcpy(ev->attr.id, id, ids * sizeof(u64));
> > +     memcpy(perf_record_header_attr_id(ev), id, ids * sizeof(u64));
> >
> >       ev->attr.header.type = PERF_RECORD_HEADER_ATTR;
> >       ev->attr.header.size = (u16)size;
>