From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 59892C433FE for ; Wed, 20 Oct 2021 15:48:11 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 430E56137C for ; Wed, 20 Oct 2021 15:48:11 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230413AbhJTPuY (ORCPT ); Wed, 20 Oct 2021 11:50:24 -0400 Received: from mail.kernel.org ([198.145.29.99]:59372 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230369AbhJTPuY (ORCPT ); Wed, 20 Oct 2021 11:50:24 -0400 Received: from gandalf.local.home (cpe-66-24-58-225.stny.res.rr.com [66.24.58.225]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 887DD61212; Wed, 20 Oct 2021 15:48:08 +0000 (UTC) Date: Wed, 20 Oct 2021 11:48:05 -0400 From: Steven Rostedt To: Kalesh Singh Cc: surenb@google.com, hridya@google.com, namhyung@kernel.org, kernel-team@android.com, Jonathan Corbet , Ingo Molnar , Shuah Khan , Masami Hiramatsu , Tom Zanussi , linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org Subject: Re: [PATCH v2 3/5] tracing: Fix operator precedence for hist triggers expression Message-ID: <20211020114805.3fbb7d94@gandalf.local.home> In-Reply-To: <20211020013153.4106001-4-kaleshsingh@google.com> References: <20211020013153.4106001-1-kaleshsingh@google.com> <20211020013153.4106001-4-kaleshsingh@google.com> X-Mailer: Claws Mail 3.17.8 (GTK+ 2.24.33; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-doc@vger.kernel.org On Tue, 19 Oct 2021 18:31:40 -0700 Kalesh Singh 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