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 11/15] tracing/histogram: Covert expr to const if both operands are constants
Date: Wed, 27 Oct 2021 12:09:51 -0400 [thread overview]
Message-ID: <20211027161001.781380582@goodmis.org> (raw)
In-Reply-To: 20211027160940.084904334@goodmis.org
From: Kalesh Singh <kaleshsingh@google.com>
If both operands of a hist trigger expression are constants, convert the
expression to a constant. This optimization avoids having to perform the
same calculation multiple times and also saves on memory since the
merged constants are represented by a single struct hist_field instead
or multiple.
Link: https://lkml.kernel.org/r/20211025200852.3002369-6-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 | 104 ++++++++++++++++++++++---------
1 file changed, 74 insertions(+), 30 deletions(-)
diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index bbaf2e16b7ae..71b453576d85 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -2411,9 +2411,15 @@ static struct hist_field *parse_unary(struct hist_trigger_data *hist_data,
return ERR_PTR(ret);
}
+/*
+ * If the operands are var refs, return pointers the
+ * variable(s) referenced in var1 and var2, else NULL.
+ */
static int check_expr_operands(struct trace_array *tr,
struct hist_field *operand1,
- struct hist_field *operand2)
+ struct hist_field *operand2,
+ struct hist_field **var1,
+ struct hist_field **var2)
{
unsigned long operand1_flags = operand1->flags;
unsigned long operand2_flags = operand2->flags;
@@ -2426,6 +2432,7 @@ static int check_expr_operands(struct trace_array *tr,
if (!var)
return -EINVAL;
operand1_flags = var->flags;
+ *var1 = var;
}
if ((operand2_flags & HIST_FIELD_FL_VAR_REF) ||
@@ -2436,6 +2443,7 @@ static int check_expr_operands(struct trace_array *tr,
if (!var)
return -EINVAL;
operand2_flags = var->flags;
+ *var2 = var;
}
if ((operand1_flags & HIST_FIELD_FL_TIMESTAMP_USECS) !=
@@ -2453,9 +2461,12 @@ static struct hist_field *parse_expr(struct hist_trigger_data *hist_data,
char *var_name, unsigned int *n_subexprs)
{
struct hist_field *operand1 = NULL, *operand2 = NULL, *expr = NULL;
- unsigned long operand_flags;
+ struct hist_field *var1 = NULL, *var2 = NULL;
+ unsigned long operand_flags, operand2_flags;
int field_op, ret = -EINVAL;
char *sep, *operand1_str;
+ hist_field_fn_t op_fn;
+ bool combine_consts;
if (*n_subexprs > 3) {
hist_err(file->tr, HIST_ERR_TOO_MANY_SUBEXPR, errpos(str));
@@ -2512,11 +2523,38 @@ static struct hist_field *parse_expr(struct hist_trigger_data *hist_data,
goto free;
}
- ret = check_expr_operands(file->tr, operand1, operand2);
+ switch (field_op) {
+ case FIELD_OP_MINUS:
+ op_fn = hist_field_minus;
+ break;
+ case FIELD_OP_PLUS:
+ op_fn = hist_field_plus;
+ break;
+ case FIELD_OP_DIV:
+ op_fn = hist_field_div;
+ break;
+ case FIELD_OP_MULT:
+ op_fn = hist_field_mult;
+ break;
+ default:
+ ret = -EINVAL;
+ goto free;
+ }
+
+ ret = check_expr_operands(file->tr, operand1, operand2, &var1, &var2);
if (ret)
goto free;
- flags |= HIST_FIELD_FL_EXPR;
+ operand_flags = var1 ? var1->flags : operand1->flags;
+ operand2_flags = var2 ? var2->flags : operand2->flags;
+
+ /*
+ * If both operands are constant, the expression can be
+ * collapsed to a single constant.
+ */
+ combine_consts = operand_flags & operand2_flags & HIST_FIELD_FL_CONST;
+
+ flags |= combine_consts ? HIST_FIELD_FL_CONST : HIST_FIELD_FL_EXPR;
flags |= operand1->flags &
(HIST_FIELD_FL_TIMESTAMP | HIST_FIELD_FL_TIMESTAMP_USECS);
@@ -2533,37 +2571,43 @@ static struct hist_field *parse_expr(struct hist_trigger_data *hist_data,
expr->operands[0] = operand1;
expr->operands[1] = operand2;
- /* The operand sizes should be the same, so just pick one */
- expr->size = operand1->size;
+ if (combine_consts) {
+ if (var1)
+ expr->operands[0] = var1;
+ if (var2)
+ expr->operands[1] = var2;
- expr->operator = field_op;
- expr->name = expr_str(expr, 0);
- expr->type = kstrdup_const(operand1->type, GFP_KERNEL);
- if (!expr->type) {
- ret = -ENOMEM;
- goto free;
- }
+ expr->constant = op_fn(expr, NULL, NULL, NULL, NULL);
- switch (field_op) {
- case FIELD_OP_MINUS:
- expr->fn = hist_field_minus;
- break;
- case FIELD_OP_PLUS:
- expr->fn = hist_field_plus;
- break;
- case FIELD_OP_DIV:
- expr->fn = hist_field_div;
- break;
- case FIELD_OP_MULT:
- expr->fn = hist_field_mult;
- break;
- default:
- ret = -EINVAL;
- goto free;
+ expr->operands[0] = NULL;
+ expr->operands[1] = NULL;
+
+ /*
+ * var refs won't be destroyed immediately
+ * See: destroy_hist_field()
+ */
+ destroy_hist_field(operand2, 0);
+ destroy_hist_field(operand1, 0);
+
+ expr->name = expr_str(expr, 0);
+ } else {
+ expr->fn = op_fn;
+
+ /* The operand sizes should be the same, so just pick one */
+ expr->size = operand1->size;
+
+ expr->operator = field_op;
+ expr->type = kstrdup_const(operand1->type, GFP_KERNEL);
+ if (!expr->type) {
+ ret = -ENOMEM;
+ goto free;
+ }
+
+ expr->name = expr_str(expr, 0);
}
return expr;
- free:
+free:
destroy_hist_field(operand1, 0);
destroy_hist_field(operand2, 0);
destroy_hist_field(expr, 0);
--
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 ` [for-next][PATCH 10/15] tracing/histogram: Simplify handling of .sym-offset in expressions Steven Rostedt
2021-10-27 16:09 ` Steven Rostedt [this message]
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.781380582@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.