Re: [PATCH] perf script python: Fix buffer size to report iregs in perf script
From: Paul A. Clarke <hidden>
Date: 2021-06-28 15:11:03
Also in:
linux-perf-users, lkml
On Mon, Jun 28, 2021 at 11:53:41AM +0530, Kajol Jain wrote:
quoted hunk ↗ jump to hunk
Commit 48a1f565261d ("perf script python: Add more PMU fields to event handler dict") added functionality to report fields like weight, iregs, uregs etc via perf report. That commit predefined buffer size to 512 bytes to print those fields. But incase of powerpc, since we added extended regs support in commits: Commit 068aeea3773a ("perf powerpc: Support exposing Performance Monitor Counter SPRs as part of extended regs") Commit d735599a069f ("powerpc/perf: Add extended regs support for power10 platform") Now iregs can carry more bytes of data and this predefined buffer size can result to data loss in perf script output. Patch resolve this issue by making buffer size dynamic based on number of registers needed to print. It also changed return type for function "regs_map" from int to void, as the return value is not being used by the caller function "set_regs_in_dict". Fixes: 068aeea3773a ("perf powerpc: Support exposing Performance Monitor Counter SPRs as part of extended regs") Signed-off-by: Kajol Jain <redacted> --- .../util/scripting-engines/trace-event-python.c | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-)diff --git a/tools/perf/util/scripting-engines/trace-event-python.c b/tools/perf/util/scripting-engines/trace-event-python.c index 4e4aa4c97ac5..c8c9706b4643 100644 --- a/tools/perf/util/scripting-engines/trace-event-python.c +++ b/tools/perf/util/scripting-engines/trace-event-python.c
[...]
quoted hunk ↗ jump to hunk
@@ -713,7 +711,16 @@ static void set_regs_in_dict(PyObject *dict, struct evsel *evsel) { struct perf_event_attr *attr = &evsel->core.attr; - char bf[512]; + + /* + * Here value 28 is a constant size which can be used to print + * one register value and its corresponds to: + * 16 chars is to specify 64 bit register in hexadecimal. + * 2 chars is for appending "0x" to the hexadecimal value and + * 10 chars is for register name. + */ + int size = __sw_hweight64(attr->sample_regs_intr) * 28; + char bf[size];
I propose using a template rather than a magic number here. Something like:
const char reg_name_tmpl[] = "10 chars ";
const char reg_value_tmpl[] = "0x0123456789abcdef";
const int size = __sw_hweight64(attr->sample_regs_intr) +
sizeof reg_name_tmpl + sizeof reg_value_tmpl;
Pardon my ignorance, but is there no separation/whitespace between the name
and the value? And is there some significance to 10 characters for the
register name, or is that a magic number?
PC