lkml.org 
[lkml]   [2021]   [Oct]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v2 3/5] tracing: Fix operator precedence for hist triggers expression
On Tue, 19 Oct 2021 18:31:40 -0700
Kalesh Singh <kaleshsingh@google.com> wrote:

> @@ -2391,60 +2460,61 @@ static int check_expr_operands(struct trace_array *tr,
> static struct hist_field *parse_expr(struct hist_trigger_data *hist_data,
> struct trace_event_file *file,
> char *str, unsigned long flags,
> - char *var_name, unsigned int level)
> + char *var_name, unsigned int *n_subexprs)
> {
> struct hist_field *operand1 = NULL, *operand2 = NULL, *expr = NULL;
> unsigned long operand_flags;
> int field_op, ret = -EINVAL;
> char *sep, *operand1_str;
>
> - if (level > 3) {
> + if (*n_subexprs > 3) {

Why limit the sub expressions, and not just keep the limit of the level of
recursion. We allow 3 levels of recursion, but we could have more than 3
sub expressions.


If we have: a * b + c / d - e * f / h

It would break down into:
-
+ /
* / * h
a b c d e f


Which I believe is 6 "sub expressions", but never goes more than three deep
in recursion:

"a * b + c / d - e * f / h"

Step 1:

op = "-"
operand1 = "a * b + c / d"
operand2 = "e * f / h"

Process operand1: (recursion level 1)

op = "+"
operand1a = "a * b"
operand2a = "c / d"

Process operand1a: (recursion level 2)

op = "*"
operand1b = "a"
operand2b = "b"

return;

Process operand1b: (recursion level 2)

op = "/"
operand1b = "c"
operand2b = "d"

return;

return;

Process operand2: (recursion level 1)

op = "/"
operand1c = "e * f"
operand2c = "h"

Process operand1c: (recursion level 2)

op = "*"
operand1c = "e"
operand2c = "f"

return;

return;



> +
> + /* LHS of string is an expression e.g. a+b in a+b+c */
> + operand1 = parse_expr(hist_data, file, operand1_str, operand_flags, NULL, n_subexprs);
> if (IS_ERR(operand1)) {
> ret = PTR_ERR(operand1);
> operand1 = NULL;

I wonder if we should look for optimizations, in case of operand1 and
operand2 are both constants?

Just perform the function, and convert it into a constant as well.

-- Steve

\
 
 \ /
  Last update: 2021-10-20 17:49    [W:0.112 / U:2.260 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site