[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAP-5=fX9+jbZo4qd4JDY=tHNJM_hD+pn2B_0ytf0_6A78XeMaQ@mail.gmail.com>
Date: Wed, 18 Dec 2024 17:29:01 -0800
From: Ian Rogers <irogers@...gle.com>
To: Namhyung Kim <namhyung@...nel.org>
Cc: Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...hat.com>,
Arnaldo Carvalho de Melo <acme@...nel.org>, Mark Rutland <mark.rutland@....com>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>, Jiri Olsa <jolsa@...nel.org>,
Adrian Hunter <adrian.hunter@...el.com>, Kan Liang <kan.liang@...ux.intel.com>,
Sun Haiyong <sunhaiyong@...ngson.cn>, Yanteng Si <siyanteng@...ngson.cn>,
linux-perf-users@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 3/5] perf synthetic-events: Ensure features are aligned
On Wed, Dec 18, 2024 at 5:08 PM Namhyung Kim <namhyung@...nel.org> wrote:
>
> On Mon, Dec 16, 2024 at 03:24:57PM -0800, Ian Rogers wrote:
> > Features like hostname have arbitrary size and break the assumed
> > 8-byte alignment of perf events. Pad all feature events until 8-byte
> > alignment is restored.
>
> But it seems write_hostname() and others use do_write_string() which
> handles the padding already.
Well it does something :-) (I agree my description isn't 100%)
https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/util/header.c?h=perf-tools-next#n188
```
static int do_write_string(struct feat_fd *ff, const char *str)
{
u32 len, olen;
int ret;
olen = strlen(str) + 1;
len = PERF_ALIGN(olen, NAME_ALIGN);
/* write len, incl. \0 */
ret = do_write(ff, &len, sizeof(len));
if (ret < 0)
return ret;
return write_padded(ff, str, olen, len);
}
```
but NAME_ALIGN is 64:
https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/util/header.h?h=perf-tools-next#n187
whereas the data file is expecting things aligned to sizeof(u64) which
is 8-bytes. We're potentially writing out 7 u64s worth of 0s for no
alignment gain. It should be safe, given I don't see other use of this
alignment value, to switch the string alignment to being sizeof(u64)
but following this change we could also just get rid of the padding
knowing it will be fixed in the caller.
Thanks,
Ian
Powered by blists - more mailing lists