`On Tue, 19 Oct 2021 18:31:40 -0700Kalesh 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 ofrecursion. We allow 3 levels of recursion, but we could have more than 3sub expressions.If we have:  a * b + c / d - e * f / hIt would break down into:              -       +            /   *       /     *     h a   b   c  d  e  fWhich I believe is 6 "sub expressions", but never goes more than three deepin 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 andoperand2 are both constants?Just perform the function, and convert it into a constant as well.-- Steve`