| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <Z9DpcfWqV7_Za3uu@google.com>
Date: Tue, 11 Mar 2025 18:54:57 -0700
From: Namhyung Kim <namhyung@...nel.org>
To: Arnaldo Carvalho de Melo <acme@...nel.org>
Cc: Ian Rogers <irogers@...gle.com>, Peter Zijlstra <peterz@...radead.org>,
Ingo Molnar <mingo@...hat.com>, 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>,
Yicong Yang <yangyicong@...ilicon.com>,
James Clark <james.clark@...aro.org>,
"Dr. David Alan Gilbert" <linux@...blig.org>,
Levi Yun <yeoreum.yun@....com>, Ze Gao <zegao2021@...il.com>,
Weilin Wang <weilin.wang@...el.com>, Xu Yang <xu.yang_2@....com>,
linux-perf-users@...r.kernel.org, linux-kernel@...r.kernel.org,
Howard Chu <howardchu95@...il.com>
Subject: Re: [PATCH v2 11/11] perf python tracepoint: Switch to using
parse_events
On Tue, Mar 11, 2025 at 06:52:45PM -0300, Arnaldo Carvalho de Melo wrote:
> On Tue, Mar 11, 2025 at 03:49:37PM -0300, Arnaldo Carvalho de Melo wrote:
> > So it seems to be something just in the python binding, as perf trace
> > seems to handle it well:
> >
> > ( field 'prev_comm' ret=0x7f7c31f65110, raw_size=68 ) ( field 'prev_pid' ret=0x7f7c23b1bed0, raw_size=68 ) ( field 'prev_prio' ret=0x7f7c239c0030, raw_size=68 ) ( field 'prev_state' ret=0x7f7c239c0250, raw_size=68 ) time 14771421785867 prev_comm= prev_pid=1919907691 prev_prio=796026219 prev_state=0x303a32313175 ==>
> > ( XXX '��' len=16, raw_size=68) ( field 'next_comm' ret=(nil), raw_size=68 ) Traceback (most recent call last):
> > File "/home/acme/git/perf-tools-next/tools/perf/python/tracepoint.py", line 51, in <module>
> > main()
> > File "/home/acme/git/perf-tools-next/tools/perf/python/tracepoint.py", line 46, in main
> > event.next_comm,
> > ^^^^^^^^^^^^^^^
> > AttributeError: 'perf.sample_event' object has no attribute 'next_comm'
> > root@...ber:/home/acme/git/perf-tools-next# cat /proc/125355/comm
> > kworker/u112:0-i915
> > root@...ber:/home/acme/git/perf-tools-next#
> > root@...ber:/home/acme/git/perf-tools-next#
> > root@...ber:/home/acme/git/perf-tools-next# perf trace -e sched:sched_switch -p 125355
> > 0.000 sched:sched_switch(prev_comm: "kworker/u112:0", prev_pid: 125355 (kworker/u112:0-), prev_prio: 120, prev_state: 128, next_comm: "swapper/6", next_prio: 120)
> > ^Croot@...ber:/home/acme/git/perf-tools-next#
> >
> > I.e. it chops up the prev_comm size to what is specified in the
> > tracepoint format.
> >
> > And that sample->raw_size is the same accross the sched:sched_switch
> > raw_datas (seemingly suboptimal, most are less than 16 bytes, but
> > probably its not guaranteed that the \0 will be there, so copy all the
> > 16 bytes).
> >
> > Now to try to figure out why simply using PyUnicode_FromStringAndSize
> > doesn't work...
>
> Didn't manage to make progress on this, I spent more time than I
> expected as I think this could be some sort of canary on some coal mine,
> but with the patch below, that gives up and just avoids touching the
> COMM fields and don't switch from string to bytearray in the binding, it
> runs forever, this is just a data point in case somebody wants to
> pursue.
>
> That flipping from string to not string based on just one entry not
> being acceptable is questionable, and I think it should go away, but why
> when COMM fields are bigger what is alloted to them in the tracepoint
> ends up tripping up just the python binding is something I couldn't
> grasp in today's session.
>
> Namhyung, this is something open, but not caused by Ian's patchset, for
> which I give my:
>
> Tested-by: Arnaldo Carvalho de Melo <acme@...hat.com>
Thanks for the analysis!
>
> In addition to the tags I provided patch by patch.
Ian, can you check if it works well for you?
Thanks,
Namhyung
> diff --git a/tools/perf/python/tracepoint.py b/tools/perf/python/tracepoint.py
> index 38b2b6d11f64566a..965b50afbdafeeb2 100755
> --- a/tools/perf/python/tracepoint.py
> +++ b/tools/perf/python/tracepoint.py
> @@ -33,15 +33,12 @@ def main():
> if not isinstance(event, perf.sample_event):
> continue
>
> - print("time %u prev_comm=%s prev_pid=%d prev_prio=%d prev_state=0x%x ==> next_comm=%s next_pid=%d next_prio=%d" % (
> - event.sample_time,
> - event.prev_comm,
> - event.prev_pid,
> - event.prev_prio,
> - event.prev_state,
> - event.next_comm,
> - event.next_pid,
> - event.next_prio))
> + try:
> + print("time %u prev_comm=%s prev_pid=%d prev_prio=%d prev_state=0x%x ==> next_comm=%s next_pid=%d next_prio=%d" % (
> + event.sample_time, event.prev_comm, event.prev_pid, event.prev_prio, event.prev_state, event.next_comm, event.next_pid, event.next_prio))
> + except:
> + print("time %u prev_pid=%d prev_prio=%d prev_state=0x%x ==> next_pid=%d next_prio=%d" % (
> + event.sample_time, event.prev_pid, event.prev_prio, event.prev_state, event.next_pid, event.next_prio))
>
> if __name__ == '__main__':
> main()
> diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c
> index 6c5bb5e8893998ae..3eb77bd270077cb3 100644
> --- a/tools/perf/util/python.c
> +++ b/tools/perf/util/python.c
> @@ -318,13 +318,10 @@ tracepoint_field(const struct pyrf_event *pe, struct tep_format_field *field)
> if (tep_field_is_relative(field->flags))
> offset += field->offset + field->size;
> }
> - if (field->flags & TEP_FIELD_IS_STRING &&
> - is_printable_array(data + offset, len)) {
> + if (field->flags & TEP_FIELD_IS_STRING)
> ret = PyUnicode_FromString((char *)data + offset);
> - } else {
> + else
> ret = PyByteArray_FromStringAndSize((const char *) data + offset, len);
> - field->flags &= ~TEP_FIELD_IS_STRING;
> - }
> } else {
> val = tep_read_number(pevent, data + field->offset,
> field->size);
Powered by blists - more mailing lists