From: Steven Rostedt <rostedt@goodmis.org>
To: linux-kernel@vger.kernel.org
Cc: Ingo Molnar <mingo@kernel.org>,
Andrew Morton <akpm@linux-foundation.org>,
Kalesh Singh <kaleshsingh@google.com>
Subject: [for-next][PATCH 10/15] tracing/histogram: Simplify handling of .sym-offset in expressions
Date: Wed, 27 Oct 2021 12:09:50 -0400 [thread overview]
Message-ID: <20211027161001.592319549@goodmis.org> (raw)
In-Reply-To: 20211027160940.084904334@goodmis.org
From: Kalesh Singh <kaleshsingh@google.com>
The '-' in .sym-offset can confuse the hist trigger arithmetic
expression parsing. Simplify the handling of this by replacing the
'sym-offset' with 'symXoffset'. This allows us to correctly evaluate
expressions where the user may have inadvertently added a .sym-offset
modifier to one of the operands in an expression, instead of bailing
out. In this case the .sym-offset has no effect on the evaluation of the
expression. The only valid use of the .sym-offset is as a hist key
modifier.
Link: https://lkml.kernel.org/r/20211025200852.3002369-5-kaleshsingh@google.com
Signed-off-by: Kalesh Singh <kaleshsingh@google.com>
Suggested-by: Steven Rostedt <rostedt@goodmis.org>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
kernel/trace/trace_events_hist.c | 43 +++++++++++++-------------------
1 file changed, 17 insertions(+), 26 deletions(-)
diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index 7a50ea2ac6b1..bbaf2e16b7ae 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -68,8 +68,7 @@
C(INVALID_SORT_FIELD, "Sort field must be a key or a val"), \
C(INVALID_STR_OPERAND, "String type can not be an operand in expression"), \
C(EXPECT_NUMBER, "Expecting numeric literal"), \
- C(UNARY_MINUS_SUBEXPR, "Unary minus not supported in sub-expressions"), \
- C(SYM_OFFSET_SUBEXPR, ".sym-offset not supported in sub-expressions"),
+ C(UNARY_MINUS_SUBEXPR, "Unary minus not supported in sub-expressions"),
#undef C
#define C(a, b) HIST_ERR_##a
@@ -1672,10 +1671,6 @@ static int contains_operator(char *str, char **sep)
*/
minus_op = strrchr(str, '-');
if (minus_op) {
- /* Unfortunately, the modifier ".sym-offset" can confuse things. */
- if (minus_op - str >= 4 && !strncmp(minus_op - 4, ".sym-offset", 11))
- goto out;
-
/*
* Unary minus is not supported in sub-expressions. If
* present, it is always the next root operator.
@@ -2138,7 +2133,11 @@ parse_field(struct hist_trigger_data *hist_data, struct trace_event_file *file,
*flags |= HIST_FIELD_FL_HEX;
else if (strcmp(modifier, "sym") == 0)
*flags |= HIST_FIELD_FL_SYM;
- else if (strcmp(modifier, "sym-offset") == 0)
+ /*
+ * 'sym-offset' occurrences in the trigger string are modified
+ * to 'symXoffset' to simplify arithmetic expression parsing.
+ */
+ else if (strcmp(modifier, "symXoffset") == 0)
*flags |= HIST_FIELD_FL_SYM_OFFSET;
else if ((strcmp(modifier, "execname") == 0) &&
(strcmp(field_name, "common_pid") == 0))
@@ -2463,24 +2462,6 @@ static struct hist_field *parse_expr(struct hist_trigger_data *hist_data,
return ERR_PTR(-EINVAL);
}
- /*
- * ".sym-offset" in expressions has no effect on their evaluation,
- * but can confuse operator parsing.
- */
- if (*n_subexprs == 0) {
- sep = strstr(str, ".sym-offset");
- if (sep) {
- *sep = '\0';
- if (strpbrk(str, "+-/*") || strpbrk(sep + 11, "+-/*")) {
- *sep = '.';
- hist_err(file->tr, HIST_ERR_SYM_OFFSET_SUBEXPR,
- errpos(sep));
- return ERR_PTR(-EINVAL);
- }
- *sep = '.';
- }
- }
-
field_op = contains_operator(str, &sep);
if (field_op == FIELD_OP_NONE)
@@ -5999,7 +5980,7 @@ static int event_hist_trigger_func(struct event_command *cmd_ops,
struct synth_event *se;
const char *se_name;
bool remove = false;
- char *trigger, *p;
+ char *trigger, *p, *start;
int ret = 0;
lockdep_assert_held(&event_mutex);
@@ -6047,6 +6028,16 @@ static int event_hist_trigger_func(struct event_command *cmd_ops,
trigger = strstrip(trigger);
}
+ /*
+ * To simplify arithmetic expression parsing, replace occurrences of
+ * '.sym-offset' modifier with '.symXoffset'
+ */
+ start = strstr(trigger, ".sym-offset");
+ while (start) {
+ *(start + 4) = 'X';
+ start = strstr(start + 11, ".sym-offset");
+ };
+
attrs = parse_hist_trigger_attrs(file->tr, trigger);
if (IS_ERR(attrs))
return PTR_ERR(attrs);
--
2.33.0
next prev parent reply other threads:[~2021-10-27 16:10 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-10-27 16:09 [for-next][PATCH 00/15] tracing: More updates for 5.16 Steven Rostedt
2021-10-27 16:09 ` [for-next][PATCH 01/15] lib/bootconfig: Fix the xbc_get_info kerneldoc Steven Rostedt
2021-10-27 16:09 ` [for-next][PATCH 02/15] samples/kretprobes: Fix return value if register_kretprobe() failed Steven Rostedt
2021-10-27 16:09 ` [for-next][PATCH 03/15] docs, kprobes: Remove invalid URL and add new reference Steven Rostedt
2021-10-27 16:09 ` [for-next][PATCH 04/15] test_kprobes: Move it from kernel/ to lib/ Steven Rostedt
2021-10-27 16:09 ` [for-next][PATCH 05/15] MAINTAINERS: Update KPROBES and TRACING entries Steven Rostedt
2021-10-27 16:09 ` [for-next][PATCH 06/15] selftests/ftrace: Stop tracing while reading the trace file by default Steven Rostedt
2021-10-27 16:09 ` [for-next][PATCH 07/15] tracing: Add support for creating hist trigger variables from literal Steven Rostedt
2021-10-27 16:09 ` [for-next][PATCH 08/15] tracing: Add division and multiplication support for hist triggers Steven Rostedt
2021-10-27 16:09 ` [for-next][PATCH 09/15] tracing: Fix operator precedence for hist triggers expression Steven Rostedt
2021-10-27 16:09 ` Steven Rostedt [this message]
2021-10-27 16:09 ` [for-next][PATCH 11/15] tracing/histogram: Covert expr to const if both operands are constants Steven Rostedt
2021-10-27 16:09 ` [for-next][PATCH 12/15] tracing/histogram: Optimize division by a power of 2 Steven Rostedt
2021-10-27 16:09 ` [for-next][PATCH 13/15] tracing/histogram: Document expression arithmetic and constants Steven Rostedt
2021-10-27 16:09 ` [for-next][PATCH 14/15] ftrace: disable preemption when recursion locked Steven Rostedt
2021-10-27 16:17 ` Steven Rostedt
2021-10-27 16:09 ` [for-next][PATCH 15/15] ftrace: do CPU checking after preemption disabled 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=20211027161001.592319549@goodmis.org \
--to=rostedt@goodmis.org \
--cc=akpm@linux-foundation.org \
--cc=kaleshsingh@google.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@kernel.org \
/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