From: Arnaldo Carvalho de Melo <acme@redhat.com>

Hi Namhyung,

        So this simplifies it greatly, it almost ends up as a one-liner,
but there is an improvement as well to mark the event as consumed to
then parse its sample, because the copy of the whole event was done all
along.

        This is brittle, as the header size can be bigger, than the
space we use and in that case we fail to parse the event by bounds
checking it.

	Supporting larger event payloads can be done on top of this,
possibly by deferring consuming the event in the ring buffer by parsing
it all instead of having pre-allocated space, measurements need to be
made to see whats best. I'd say leave this for when it proves necessary.

        With this series I managed to run it for a long time without
crashes and 'top' says it doesn't seem to be leaking anything, as its
memory usage stays the same for as long as I looked.

        Please consider applying to perf-tools-next,

Best regards

P.S.: In other news, the syscalltbl series from Ian built on all my
containers, I'm now trying to go over it patch by patch.

Arnaldo Carvalho de Melo (6):
  perf python: Fixup description of sample.id event member
  perf python: Remove some unused macros (_PyUnicode_FromString(arg), etc)
  perf python tracepoint.py: Change the COMM using setproctitle if available
  perf python: Decrement the refcount of just created event on failure
  perf python: Don't keep a raw_data pointer to consumed ring buffer space
  perf python: Check if there is space to copy all the event

 tools/perf/python/tracepoint.py |  8 ++++++++
 tools/perf/util/python.c        | 24 ++++++++++++------------
 2 files changed, 20 insertions(+), 12 deletions(-)

-- 
2.48.1

On Wed, 12 Mar 2025 17:31:35 -0300, Arnaldo Carvalho de Melo wrote:
> From: Arnaldo Carvalho de Melo <acme@redhat.com>
> 
> Hi Namhyung,
> 
>         So this simplifies it greatly, it almost ends up as a one-liner,
> but there is an improvement as well to mark the event as consumed to
> then parse its sample, because the copy of the whole event was done all
> along.
> 
> [...]
Applied to perf-tools-next, thanks!

Best regards,
Namhyung

On Wed, Mar 12, 2025 at 1:31 PM Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
>
> From: Arnaldo Carvalho de Melo <acme@redhat.com>
>
> Hi Namhyung,
>
>         So this simplifies it greatly, it almost ends up as a one-liner,
> but there is an improvement as well to mark the event as consumed to
> then parse its sample, because the copy of the whole event was done all
> along.
>
>         This is brittle, as the header size can be bigger, than the
> space we use and in that case we fail to parse the event by bounds
> checking it.
>
>         Supporting larger event payloads can be done on top of this,
> possibly by deferring consuming the event in the ring buffer by parsing
> it all instead of having pre-allocated space, measurements need to be
> made to see whats best. I'd say leave this for when it proves necessary.
>
>         With this series I managed to run it for a long time without
> crashes and 'top' says it doesn't seem to be leaking anything, as its
> memory usage stays the same for as long as I looked.
>
>         Please consider applying to perf-tools-next,
>
> Best regards
>
> P.S.: In other news, the syscalltbl series from Ian built on all my
> containers, I'm now trying to go over it patch by patch.
>
> Arnaldo Carvalho de Melo (6):
>   perf python: Fixup description of sample.id event member
>   perf python: Remove some unused macros (_PyUnicode_FromString(arg), etc)
>   perf python tracepoint.py: Change the COMM using setproctitle if available
>   perf python: Decrement the refcount of just created event on failure
>   perf python: Don't keep a raw_data pointer to consumed ring buffer space
>   perf python: Check if there is space to copy all the event

Reviewed-by: Ian Rogers <irogers@google.com>

Thanks,
Ian

On Mon, Mar 17, 2025 at 10:41:28AM -0700, Ian Rogers wrote:
> On Wed, Mar 12, 2025 at 1:31 PM Arnaldo Carvalho de Melo
> > P.S.: In other news, the syscalltbl series from Ian built on all my
> > containers, I'm now trying to go over it patch by patch.

Doing some more tests on it, the -e case is fixed with Namhyung's fix
for re-reading the sc variable after the realloc.

> > Arnaldo Carvalho de Melo (6):
> >   perf python: Fixup description of sample.id event member
> >   perf python: Remove some unused macros (_PyUnicode_FromString(arg), etc)
> >   perf python tracepoint.py: Change the COMM using setproctitle if available
> >   perf python: Decrement the refcount of just created event on failure
> >   perf python: Don't keep a raw_data pointer to consumed ring buffer space
> >   perf python: Check if there is space to copy all the event
 
> Reviewed-by: Ian Rogers <irogers@google.com>

Thanks!

- Arnaldo