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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox