Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.35 as permitted sender) client-ip=23.128.96.35;
MIME-Version: 1.0
References: <20231212192317.0fb6b101@gandalf.local.home> <CAJ1xhMXuxGHfD8oRfbna_drpHmNv0RUXFrvuVv6rtt-KeCM9fQ@mail.gmail.com>
 <20231213095749.5ff64569@gandalf.local.home>
In-Reply-To: <20231213095749.5ff64569@gandalf.local.home>
From:   Alexander Kapshuk <alexander.kapshuk@gmail.com>
Date:   Wed, 13 Dec 2023 17:08:00 +0200
Message-ID: <CAJ1xhMX-gH+WgQnXd-bAQLEoipjZ_Fcbj=PRnpY64Kth1iFryg@mail.gmail.com>
Subject: Re: [PATCH v3] tracing/selftests: Add test to test the trace_marker
To:     Steven Rostedt <rostedt@goodmis.org>
Cc:     LKML <linux-kernel@vger.kernel.org>,
        Linux Trace Kernel <linux-trace-kernel@vger.kernel.org>,
        Masami Hiramatsu <mhiramat@kernel.org>,
        Mark Rutland <mark.rutland@arm.com>,
        Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
        Shuah Khan <shuah@kernel.org>, linux-kselftest@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Wed, Dec 13, 2023 at 4:57=E2=80=AFPM Steven Rostedt <rostedt@goodmis.org=
> wrote:
>
> On Wed, 13 Dec 2023 10:09:50 +0200
> Alexander Kapshuk <alexander.kapshuk@gmail.com> wrote:
>
> > The REs used in the sed commands below may be simplified as shown if de=
sired.
> >
> > On Wed, Dec 13, 2023 at 2:22=E2=80=AFAM Steven Rostedt <rostedt@goodmis=
.org> wrote:
> > >
> > > From: "Steven Rostedt (Google)" <rostedt@goodmis.org>
> > >
> > > Add a test that writes longs strings, some over the size of the sub b=
uffer
> > > and make sure that the entire content is there.
> > >
> > > Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
> > > ---
> > > Changes since v2: https://lore.kernel.org/linux-trace-kernel/20231212=
151632.25c9b67d@gandalf.local.home
> > >
> > > - Realized with the upcoming change of the dynamic subbuffer sizes, t=
hat
> > >   this test will fail if the subbuffer is bigger than what the trace_=
seq
> > >   can hold. Now the trace_marker does not always utilize the full sub=
buffer
> > >   but the size of the trace_seq instead. As that size isn't available=
 to
> > >   user space, we can only just make sure all content is there.
> > >
> > >  .../ftrace/test.d/00basic/trace_marker.tc     | 82 +++++++++++++++++=
++
> > >  1 file changed, 82 insertions(+)
> > >  create mode 100755 tools/testing/selftests/ftrace/test.d/00basic/tra=
ce_marker.tc
> > >
> > > diff --git a/tools/testing/selftests/ftrace/test.d/00basic/trace_mark=
er.tc b/tools/testing/selftests/ftrace/test.d/00basic/trace_marker.tc
> > > new file mode 100755
> > > index 000000000000..b24aff5807df
> > > --- /dev/null
> > > +++ b/tools/testing/selftests/ftrace/test.d/00basic/trace_marker.tc
> > > @@ -0,0 +1,82 @@
> > > +#!/bin/sh
> > > +# SPDX-License-Identifier: GPL-2.0
> > > +# description: Basic tests on writing to trace_marker
> > > +# requires: trace_marker
> > > +# flags: instance
> > > +
> > > +get_buffer_data_size() {
> > > +       sed -ne 's/^.*data.*size:\([0-9][0-9]*\).*/\1/p' events/heade=
r_page
> >             sed -n 's!.*data.*size:\([^;]*\).*!\1!p' events/header_page
>
> Not sure the above change can be considered simpler, but it also loses ou=
t
> showing what exactly is being done.
>
> With the original, I have:
>
>        sed -ne 's/^.*data.*size:\([0-9][0-9]*\).*/\1/p' events/header_pag=
e
>
> Which is obvious that I'm grabbing a number for the size field.
>
>        sed -n 's!.*data.*size:\([^;]*\).*!\1!p' events/header_page
>
> Shows that I'm grabbing something after size.
>
>
>
> > > +}
> > > +
> > > +get_buffer_data_offset() {
> > > +       sed -ne 's/^.*data.*offset:\([0-9][0-9]*\).*/\1/p' events/hea=
der_page
> >             sed -n 's!.*data.*offset:\([^;]*\).*!\1!p' events/header_pa=
ge
>
> Same here.
>
> > > +}
> > > +
> > > +get_event_header_size() {
> > > +       type_len=3D`sed -ne 's/^.*type_len.*:[^0-9]*\([0-9][0-9]*\).*=
/\1/p' events/header_event`
> >             type_len=3D`sed -n '/type_len.*bits/s![^0-9]*!!gp'
> > events/header_event`
>
> Honestly, the above may be "simplier" but I can't make out what exactly
> that line is doing without going back and looking at the text that's in t=
he
> format field.
>
> >
> > > +       time_len=3D`sed -ne 's/^.*time_delta.*:[^0-9]*\([0-9][0-9]*\)=
.*/\1/p' events/header_event`
> >             time_len=3D`sed -n '/time_delta/s![^0-9]*!!gp' events/heade=
r_event`
> >
> > > +       array_len=3D`sed -ne 's/^.*array.*:[^0-9]*\([0-9][0-9]*\).*/\=
1/p' events/header_event`
> >             array_len=3D`sed -n '/array/s![^0-9]*!!gp' events/header_ev=
ent`
> >
> > > +       total_bits=3D$((type_len+time_len+array_len))
> > > +       total_bits=3D$((total_bits+7))
> > > +       echo $((total_bits/8))
> > > +}
> > > +
> > > +get_print_event_buf_offset() {
> > > +       sed -ne 's/^.*buf.*offset:\([0-9][0-9]*\).*/\1/p' events/ftra=
ce/print/format
> >             sed -n 's!.*buf.*offset:\([^;]*\).*!\1!p' events/ftrace/pri=
nt/format
> > > +}
> > > +
>
> Yeah, thanks for the suggestions, but I rather have it be more readable
> than "simplified". I write perl code the same way. I do not write it like
> any perl developer would, because I write it like C code. I want my code =
to
> be easily understandable.
>
> RE can become extremely obfuscated. So, even though my REs are not the
> simplest, they tend to be rather easy to understand what they are doing,
> and why.
>
> Cheers,
>
> -- Steve

Fair enough.
Thanks.