tracing/histogram: Covert expr to const if both operands are constants

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>

diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index bbaf2e16b7ae73b108dc39de85621ba9004ce3e3..71b453576d85d2aeac26c5e44bbc2c55b1457369 100644 (file)
--- 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);