Skip to content

Commit

Permalink
tracing/histogram: Covert expr to const if both operands are constants
Browse files Browse the repository at this point in the history 
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>
  • Loading branch information
Kalesh Singh authored and Steven Rostedt (VMware) committed Oct 27, 2021
1 parent c5eac6e commit f47716b
Showing 1 changed file with 74 additions and 30 deletions.
104 changes: 74 additions & 30 deletions kernel/trace/trace_events_hist.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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) ||
Expand All @@ -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) !=
Expand All @@ -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));
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand Down

0 comments on commit f47716b

Please sign in to comment.