Skip to content

Commit f234ceb

Browse files
tracing/histogram: Covert expr to const if both operands are constants
JIRA: https://issues.redhat.com/browse/RHEL-67679 commit f47716b Author: Kalesh Singh <kaleshsingh@google.com> Date: Mon Oct 25 13:08:37 2021 -0700 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> Signed-off-by: Tomas Glozar <tglozar@redhat.com>
1 parent 65b7fbb commit f234ceb

File tree

1 file changed

+74
-30
lines changed

1 file changed

+74
-30
lines changed

kernel/trace/trace_events_hist.c

Lines changed: 74 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -2405,9 +2405,15 @@ static struct hist_field *parse_unary(struct hist_trigger_data *hist_data,
24052405
return ERR_PTR(ret);
24062406
}
24072407

2408+
/*
2409+
* If the operands are var refs, return pointers the
2410+
* variable(s) referenced in var1 and var2, else NULL.
2411+
*/
24082412
static int check_expr_operands(struct trace_array *tr,
24092413
struct hist_field *operand1,
2410-
struct hist_field *operand2)
2414+
struct hist_field *operand2,
2415+
struct hist_field **var1,
2416+
struct hist_field **var2)
24112417
{
24122418
unsigned long operand1_flags = operand1->flags;
24132419
unsigned long operand2_flags = operand2->flags;
@@ -2420,6 +2426,7 @@ static int check_expr_operands(struct trace_array *tr,
24202426
if (!var)
24212427
return -EINVAL;
24222428
operand1_flags = var->flags;
2429+
*var1 = var;
24232430
}
24242431

24252432
if ((operand2_flags & HIST_FIELD_FL_VAR_REF) ||
@@ -2430,6 +2437,7 @@ static int check_expr_operands(struct trace_array *tr,
24302437
if (!var)
24312438
return -EINVAL;
24322439
operand2_flags = var->flags;
2440+
*var2 = var;
24332441
}
24342442

24352443
if ((operand1_flags & HIST_FIELD_FL_TIMESTAMP_USECS) !=
@@ -2447,9 +2455,12 @@ static struct hist_field *parse_expr(struct hist_trigger_data *hist_data,
24472455
char *var_name, unsigned int *n_subexprs)
24482456
{
24492457
struct hist_field *operand1 = NULL, *operand2 = NULL, *expr = NULL;
2450-
unsigned long operand_flags;
2458+
struct hist_field *var1 = NULL, *var2 = NULL;
2459+
unsigned long operand_flags, operand2_flags;
24512460
int field_op, ret = -EINVAL;
24522461
char *sep, *operand1_str;
2462+
hist_field_fn_t op_fn;
2463+
bool combine_consts;
24532464

24542465
if (*n_subexprs > 3) {
24552466
hist_err(file->tr, HIST_ERR_TOO_MANY_SUBEXPR, errpos(str));
@@ -2506,11 +2517,38 @@ static struct hist_field *parse_expr(struct hist_trigger_data *hist_data,
25062517
goto free;
25072518
}
25082519

2509-
ret = check_expr_operands(file->tr, operand1, operand2);
2520+
switch (field_op) {
2521+
case FIELD_OP_MINUS:
2522+
op_fn = hist_field_minus;
2523+
break;
2524+
case FIELD_OP_PLUS:
2525+
op_fn = hist_field_plus;
2526+
break;
2527+
case FIELD_OP_DIV:
2528+
op_fn = hist_field_div;
2529+
break;
2530+
case FIELD_OP_MULT:
2531+
op_fn = hist_field_mult;
2532+
break;
2533+
default:
2534+
ret = -EINVAL;
2535+
goto free;
2536+
}
2537+
2538+
ret = check_expr_operands(file->tr, operand1, operand2, &var1, &var2);
25102539
if (ret)
25112540
goto free;
25122541

2513-
flags |= HIST_FIELD_FL_EXPR;
2542+
operand_flags = var1 ? var1->flags : operand1->flags;
2543+
operand2_flags = var2 ? var2->flags : operand2->flags;
2544+
2545+
/*
2546+
* If both operands are constant, the expression can be
2547+
* collapsed to a single constant.
2548+
*/
2549+
combine_consts = operand_flags & operand2_flags & HIST_FIELD_FL_CONST;
2550+
2551+
flags |= combine_consts ? HIST_FIELD_FL_CONST : HIST_FIELD_FL_EXPR;
25142552

25152553
flags |= operand1->flags &
25162554
(HIST_FIELD_FL_TIMESTAMP | HIST_FIELD_FL_TIMESTAMP_USECS);
@@ -2527,37 +2565,43 @@ static struct hist_field *parse_expr(struct hist_trigger_data *hist_data,
25272565
expr->operands[0] = operand1;
25282566
expr->operands[1] = operand2;
25292567

2530-
/* The operand sizes should be the same, so just pick one */
2531-
expr->size = operand1->size;
2568+
if (combine_consts) {
2569+
if (var1)
2570+
expr->operands[0] = var1;
2571+
if (var2)
2572+
expr->operands[1] = var2;
25322573

2533-
expr->operator = field_op;
2534-
expr->name = expr_str(expr, 0);
2535-
expr->type = kstrdup_const(operand1->type, GFP_KERNEL);
2536-
if (!expr->type) {
2537-
ret = -ENOMEM;
2538-
goto free;
2539-
}
2574+
expr->constant = op_fn(expr, NULL, NULL, NULL, NULL);
25402575

2541-
switch (field_op) {
2542-
case FIELD_OP_MINUS:
2543-
expr->fn = hist_field_minus;
2544-
break;
2545-
case FIELD_OP_PLUS:
2546-
expr->fn = hist_field_plus;
2547-
break;
2548-
case FIELD_OP_DIV:
2549-
expr->fn = hist_field_div;
2550-
break;
2551-
case FIELD_OP_MULT:
2552-
expr->fn = hist_field_mult;
2553-
break;
2554-
default:
2555-
ret = -EINVAL;
2556-
goto free;
2576+
expr->operands[0] = NULL;
2577+
expr->operands[1] = NULL;
2578+
2579+
/*
2580+
* var refs won't be destroyed immediately
2581+
* See: destroy_hist_field()
2582+
*/
2583+
destroy_hist_field(operand2, 0);
2584+
destroy_hist_field(operand1, 0);
2585+
2586+
expr->name = expr_str(expr, 0);
2587+
} else {
2588+
expr->fn = op_fn;
2589+
2590+
/* The operand sizes should be the same, so just pick one */
2591+
expr->size = operand1->size;
2592+
2593+
expr->operator = field_op;
2594+
expr->type = kstrdup_const(operand1->type, GFP_KERNEL);
2595+
if (!expr->type) {
2596+
ret = -ENOMEM;
2597+
goto free;
2598+
}
2599+
2600+
expr->name = expr_str(expr, 0);
25572601
}
25582602

25592603
return expr;
2560-
free:
2604+
free:
25612605
destroy_hist_field(operand1, 0);
25622606
destroy_hist_field(operand2, 0);
25632607
destroy_hist_field(expr, 0);

0 commit comments

Comments
 (0)