[PATCH v3] tracing/selftests: Add test to test the trace_marker

[PATCH v3] tracing/selftests: Add test to test the trace_marker

[Steven Rostedt]

From: "Steven Rostedt (Google)" <rostedt@goodmis.org>

Add a test that writes longs strings, some over the size of the sub buffer
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/20231212151632.25c9b67d@gandalf.local.home

- Realized with the upcoming change of the dynamic subbuffer sizes, that
  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 subbuffer
  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/trace_marker.tc

diff --git a/tools/testing/selftests/ftrace/test.d/00basic/trace_marker.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/header_page
+}
+
+get_buffer_data_offset() {
+	sed -ne 's/^.*data.*offset:\([0-9][0-9]*\).*/\1/p' events/header_page
+}
+
+get_event_header_size() {
+	type_len=`sed -ne 's/^.*type_len.*:[^0-9]*\([0-9][0-9]*\).*/\1/p' events/header_event`
+	time_len=`sed -ne 's/^.*time_delta.*:[^0-9]*\([0-9][0-9]*\).*/\1/p' events/header_event`
+	array_len=`sed -ne 's/^.*array.*:[^0-9]*\([0-9][0-9]*\).*/\1/p' events/header_event`
+	total_bits=$((type_len+time_len+array_len))
+	total_bits=$((total_bits+7))
+	echo $((total_bits/8))
+}
+
+get_print_event_buf_offset() {
+	sed -ne 's/^.*buf.*offset:\([0-9][0-9]*\).*/\1/p' events/ftrace/print/format
+}
+
+event_header_size=`get_event_header_size`
+print_header_size=`get_print_event_buf_offset`
+
+data_offset=`get_buffer_data_offset`
+
+marker_meta=$((event_header_size+print_header_size))
+
+make_str() {
+        cnt=$1
+	# subtract two for \n\0 as marker adds these
+	cnt=$((cnt-2))
+	printf -- 'X%.0s' $(seq $cnt)
+}
+
+write_buffer() {
+	size=$1
+
+	str=`make_str $size`
+
+	# clear the buffer
+	echo > trace
+
+	# write the string into the marker
+	echo -n $str > trace_marker
+
+	echo $str
+}
+
+test_buffer() {
+
+	size=`get_buffer_data_size`
+	oneline_size=$((size-marker_meta))
+	echo size = $size
+	echo meta size = $marker_meta
+
+	# Now add a little more the meta data overhead will overflow
+
+	str=`write_buffer $size`
+
+	# Make sure the line was broken
+	new_str=`awk ' /tracing_mark_write:/ { sub(/^.*tracing_mark_write: /,"");printf "%s", $0; exit}' trace`
+
+	if [ "$new_str" = "$str" ]; then
+		exit fail;
+	fi
+
+	# Make sure the entire line can be found
+	new_str=`awk ' /tracing_mark_write:/ { sub(/^.*tracing_mark_write: */,"");printf "%s", $0; }' trace`
+
+	if [ "$new_str" != "$str" ]; then
+		exit fail;
+	fi
+}
+
+test_buffer
-- 
2.42.0

Re: [PATCH v3] tracing/selftests: Add test to test the trace_marker

[Alexander Kapshuk]

The REs used in the sed commands below may be simplified as shown if desired.

On Wed, Dec 13, 2023 at 2:22 AM 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 buffer
> 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/20231212151632.25c9b67d@gandalf.local.home
>
> - Realized with the upcoming change of the dynamic subbuffer sizes, that
>   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 subbuffer
>   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/trace_marker.tc
>
> diff --git a/tools/testing/selftests/ftrace/test.d/00basic/trace_marker.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/header_page
            sed -n 's!.*data.*size:\([^;]*\).*!\1!p' events/header_page
> +}
> +
> +get_buffer_data_offset() {
> +       sed -ne 's/^.*data.*offset:\([0-9][0-9]*\).*/\1/p' events/header_page
            sed -n 's!.*data.*offset:\([^;]*\).*!\1!p' events/header_page
> +}
> +
> +get_event_header_size() {
> +       type_len=`sed -ne 's/^.*type_len.*:[^0-9]*\([0-9][0-9]*\).*/\1/p' events/header_event`
            type_len=`sed -n '/type_len.*bits/s![^0-9]*!!gp'
events/header_event`

> +       time_len=`sed -ne 's/^.*time_delta.*:[^0-9]*\([0-9][0-9]*\).*/\1/p' events/header_event`
            time_len=`sed -n '/time_delta/s![^0-9]*!!gp' events/header_event`

> +       array_len=`sed -ne 's/^.*array.*:[^0-9]*\([0-9][0-9]*\).*/\1/p' events/header_event`
            array_len=`sed -n '/array/s![^0-9]*!!gp' events/header_event`

> +       total_bits=$((type_len+time_len+array_len))
> +       total_bits=$((total_bits+7))
> +       echo $((total_bits/8))
> +}
> +
> +get_print_event_buf_offset() {
> +       sed -ne 's/^.*buf.*offset:\([0-9][0-9]*\).*/\1/p' events/ftrace/print/format
            sed -n 's!.*buf.*offset:\([^;]*\).*!\1!p' events/ftrace/print/format
> +}
> +
> +event_header_size=`get_event_header_size`
> +print_header_size=`get_print_event_buf_offset`
> +
> +data_offset=`get_buffer_data_offset`
> +
> +marker_meta=$((event_header_size+print_header_size))
> +
> +make_str() {
> +        cnt=$1
> +       # subtract two for \n\0 as marker adds these
> +       cnt=$((cnt-2))
> +       printf -- 'X%.0s' $(seq $cnt)
> +}
> +
> +write_buffer() {
> +       size=$1
> +
> +       str=`make_str $size`
> +
> +       # clear the buffer
> +       echo > trace
> +
> +       # write the string into the marker
> +       echo -n $str > trace_marker
> +
> +       echo $str
> +}
> +
> +test_buffer() {
> +
> +       size=`get_buffer_data_size`
> +       oneline_size=$((size-marker_meta))
> +       echo size = $size
> +       echo meta size = $marker_meta
> +
> +       # Now add a little more the meta data overhead will overflow
> +
> +       str=`write_buffer $size`
> +
> +       # Make sure the line was broken
> +       new_str=`awk ' /tracing_mark_write:/ { sub(/^.*tracing_mark_write: /,"");printf "%s", $0; exit}' trace`
> +
> +       if [ "$new_str" = "$str" ]; then
> +               exit fail;
> +       fi
> +
> +       # Make sure the entire line can be found
> +       new_str=`awk ' /tracing_mark_write:/ { sub(/^.*tracing_mark_write: */,"");printf "%s", $0; }' trace`
> +
> +       if [ "$new_str" != "$str" ]; then
> +               exit fail;
> +       fi
> +}
> +
> +test_buffer
> --
> 2.42.0
>

Re: [PATCH v3] tracing/selftests: Add test to test the trace_marker

[Steven Rostedt]

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 desired.
> 
> On Wed, Dec 13, 2023 at 2:22 AM 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 buffer
> > 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/20231212151632.25c9b67d@gandalf.local.home
> >
> > - Realized with the upcoming change of the dynamic subbuffer sizes, that
> >   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 subbuffer
> >   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/trace_marker.tc
> >
> > diff --git a/tools/testing/selftests/ftrace/test.d/00basic/trace_marker.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/header_page  
>             sed -n 's!.*data.*size:\([^;]*\).*!\1!p' events/header_page

Not sure the above change can be considered simpler, but it also loses out
showing what exactly is being done.

With the original, I have:

       sed -ne 's/^.*data.*size:\([0-9][0-9]*\).*/\1/p' events/header_page

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/header_page  
>             sed -n 's!.*data.*offset:\([^;]*\).*!\1!p' events/header_page

Same here.

> > +}
> > +
> > +get_event_header_size() {
> > +       type_len=`sed -ne 's/^.*type_len.*:[^0-9]*\([0-9][0-9]*\).*/\1/p' events/header_event`  
>             type_len=`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 the
format field.

> 
> > +       time_len=`sed -ne 's/^.*time_delta.*:[^0-9]*\([0-9][0-9]*\).*/\1/p' events/header_event`  
>             time_len=`sed -n '/time_delta/s![^0-9]*!!gp' events/header_event`
> 
> > +       array_len=`sed -ne 's/^.*array.*:[^0-9]*\([0-9][0-9]*\).*/\1/p' events/header_event`  
>             array_len=`sed -n '/array/s![^0-9]*!!gp' events/header_event`
> 
> > +       total_bits=$((type_len+time_len+array_len))
> > +       total_bits=$((total_bits+7))
> > +       echo $((total_bits/8))
> > +}
> > +
> > +get_print_event_buf_offset() {
> > +       sed -ne 's/^.*buf.*offset:\([0-9][0-9]*\).*/\1/p' events/ftrace/print/format  
>             sed -n 's!.*buf.*offset:\([^;]*\).*!\1!p' events/ftrace/print/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

Re: [PATCH v3] tracing/selftests: Add test to test the trace_marker

[Alexander Kapshuk]

On Wed, Dec 13, 2023 at 4:57 PM 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 desired.
> >
> > On Wed, Dec 13, 2023 at 2:22 AM 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 buffer
> > > 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/20231212151632.25c9b67d@gandalf.local.home
> > >
> > > - Realized with the upcoming change of the dynamic subbuffer sizes, that
> > >   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 subbuffer
> > >   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/trace_marker.tc
> > >
> > > diff --git a/tools/testing/selftests/ftrace/test.d/00basic/trace_marker.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/header_page
> >             sed -n 's!.*data.*size:\([^;]*\).*!\1!p' events/header_page
>
> Not sure the above change can be considered simpler, but it also loses out
> showing what exactly is being done.
>
> With the original, I have:
>
>        sed -ne 's/^.*data.*size:\([0-9][0-9]*\).*/\1/p' events/header_page
>
> 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/header_page
> >             sed -n 's!.*data.*offset:\([^;]*\).*!\1!p' events/header_page
>
> Same here.
>
> > > +}
> > > +
> > > +get_event_header_size() {
> > > +       type_len=`sed -ne 's/^.*type_len.*:[^0-9]*\([0-9][0-9]*\).*/\1/p' events/header_event`
> >             type_len=`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 the
> format field.
>
> >
> > > +       time_len=`sed -ne 's/^.*time_delta.*:[^0-9]*\([0-9][0-9]*\).*/\1/p' events/header_event`
> >             time_len=`sed -n '/time_delta/s![^0-9]*!!gp' events/header_event`
> >
> > > +       array_len=`sed -ne 's/^.*array.*:[^0-9]*\([0-9][0-9]*\).*/\1/p' events/header_event`
> >             array_len=`sed -n '/array/s![^0-9]*!!gp' events/header_event`
> >
> > > +       total_bits=$((type_len+time_len+array_len))
> > > +       total_bits=$((total_bits+7))
> > > +       echo $((total_bits/8))
> > > +}
> > > +
> > > +get_print_event_buf_offset() {
> > > +       sed -ne 's/^.*buf.*offset:\([0-9][0-9]*\).*/\1/p' events/ftrace/print/format
> >             sed -n 's!.*buf.*offset:\([^;]*\).*!\1!p' events/ftrace/print/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.