public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Steven Rostedt <rostedt@goodmis.org>
To: linux-kernel@vger.kernel.org, linux-trace-kernel@vger.kernel.org
Cc: "Masami Hiramatsu" <mhiramat@kernel.org>,
	"Mark Rutland" <mark.rutland@arm.com>,
	"Mathieu Desnoyers" <mathieu.desnoyers@efficios.com>,
	"Andrew Morton" <akpm@linux-foundation.org>,
	"Ville Syrjälä" <ville.syrjala@linux.intel.com>,
	"Rodrigo Vivi" <rodrigo.vivi@intel.com>,
	"Chuck Lever" <chuck.lever@oracle.com>
Subject: [PATCH v2 0/4] tracing: Optimize __string()/__assign_str() processing
Date: Thu, 22 Feb 2024 16:14:15 -0500	[thread overview]
Message-ID: <20240222211415.255659509@goodmis.org> (raw)

The TRACE_EVENT() macro handles dynamic strings by having:

  TP_PROTO(struct some_struct *s),
  TP_ARGS(s),
  TP_STRUCT__entry(
        __string(my_string, s->string)
 ),
 TP_fast_assign(
        __assign_str(my_string, s->string);
 )
 TP_printk("%s", __get_str(my_string))

There's even some code that may call a function helper to find the
s->string value. The problem with the above is that the work to get the
s->string is done twice. Once at the __string() and again in the
__assign_str().

The length of the string is calculated via a strlen(), not once, but
twice (using strcpy). The length is actually already recorded in the data
location from __string() and here's no reason to call strcpy() in
__assign_str() as the length is already known.

The __string() macro uses dynamic_array() which has a helper structure that
is created holding the offsets and length of the string fields. Instead of
finding the string twice, just save it off in another field in that helper
structure, and have __assign_str() use that instead.

Changes since v1: https://lore.kernel.org/linux-trace-kernel/20240222195111.139824528@goodmis.org/

- Both the __dynamic_array() and __rel_dynamic_array() macros end with
  a semicolon and are used by __string() and __rel_string()
  respectively. But __string() doesn't have a semicolon after
  __dynamic_array() but __rel_string does have a semicolon after
  __rel_dynamic_array() which is unneeded. Remove the unnecessary
  semicolon to keep the two consistent.

- Fixed __rel_string_len() that was incorrectly using __get_str() and
  not __get_rel_str().

- Added two cleanup patches. One to use the ?: shortcut and the other
  to use the new macro EVENT_NULL_STR instead of open coding "(null)"
  as the string must be consistent between __string() and __assign_str().

Steven Rostedt (Google) (4):
      tracing: Rework __assign_str() and __string() to not duplicate getting the string
      tracing: Do not calculate strlen() twice for __string() fields
      tracing: Use ? : shortcut in trace macros
      tracing: Use EVENT_NULL_STR macro instead of open coding "(null)"

----
 include/linux/trace_events.h                 |  3 +++
 include/trace/events/sunrpc.h                | 12 ++++++------
 include/trace/stages/stage2_data_offsets.h   |  4 ++--
 include/trace/stages/stage5_get_offsets.h    | 15 ++++++++++-----
 include/trace/stages/stage6_event_callback.h | 12 ++++++++----
 5 files changed, 29 insertions(+), 17 deletions(-)

             reply	other threads:[~2024-02-22 21:12 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-22 21:14 Steven Rostedt [this message]
2024-02-22 21:14 ` [PATCH v2 1/4] tracing: Rework __assign_str() and __string() to not duplicate getting the string Steven Rostedt
2024-02-22 21:14 ` [PATCH v2 2/4] tracing: Do not calculate strlen() twice for __string() fields Steven Rostedt
2024-02-22 21:14 ` [PATCH v2 3/4] tracing: Use ? : shortcut in trace macros Steven Rostedt
2024-02-22 21:14 ` [PATCH v2 4/4] tracing: Use EVENT_NULL_STR macro instead of open coding "(null)" Steven Rostedt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240222211415.255659509@goodmis.org \
    --to=rostedt@goodmis.org \
    --cc=akpm@linux-foundation.org \
    --cc=chuck.lever@oracle.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-trace-kernel@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=mathieu.desnoyers@efficios.com \
    --cc=mhiramat@kernel.org \
    --cc=rodrigo.vivi@intel.com \
    --cc=ville.syrjala@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox