From patchwork Mon Oct 25 19:23:12 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kalesh Singh X-Patchwork-Id: 12582727 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 212B4C4332F for ; Mon, 25 Oct 2021 19:54:07 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 0B752604DA for ; Mon, 25 Oct 2021 19:54:07 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S238228AbhJYT40 (ORCPT ); Mon, 25 Oct 2021 15:56:26 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:57940 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S239234AbhJYTyY (ORCPT ); Mon, 25 Oct 2021 15:54:24 -0400 Received: from mail-yb1-xb4a.google.com (mail-yb1-xb4a.google.com [IPv6:2607:f8b0:4864:20::b4a]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 648D7C110F12 for ; Mon, 25 Oct 2021 12:23:52 -0700 (PDT) Received: by mail-yb1-xb4a.google.com with SMTP id s7-20020a25aa07000000b005bfb84d2315so19073715ybi.0 for ; Mon, 25 Oct 2021 12:23:52 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=date:in-reply-to:message-id:mime-version:references:subject:from:cc; bh=loxzsYhDjTebX67ME+FCIKC4vQSRTq2WjXhQBNyrLj8=; b=RJoFoecv8w3cy7UeAFaKPMZCZ47mxIugAnyZXHWGVlgWk5lDlGL5Oi/stF8Xv4HcIS 7v21WcAkL9URcnMQ/zKB+cDssX6yLDBCNwxl2k0eH9vefctFha+C/jPnST2BmPEIqu22 hC8+WyFfVBA+zWncFrrSoTCul45BPOwXn+ec1GiAGlTgsibhGCMNpjRbxgwVStV3odlW zNu4lb4Uj1y4aPs86mb30zzCvqGIzplH2UUco56oWJRO/CwgU/CPXHYBeCjcENNm/Z64 qrZ5AjbyC42ByDdWnxWmeHIwnjd0ybw9j4efBSxC9++CS1yZNWwSUCpli8ooCOwmWilP o6bw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:in-reply-to:message-id:mime-version :references:subject:from:cc; bh=loxzsYhDjTebX67ME+FCIKC4vQSRTq2WjXhQBNyrLj8=; b=niotkM8rgMKHa6cmOfKfVY7wZSDlGwM3fdkbb9zk8XbmGzIZSshhkalo+wE5yqLzfF WXBrPA4VIKkUEJz80/nN5UHjNFtGUcTaN4E9j/15p73DnhBmFxGV8OTqwNV3sf1lATQ7 7AUtv0SLHJqSpEH0ZGh9En92hAyZxT5PJsZlw19GJBjoaU/EWWYsxgMHbjA9mMcohkui QKQ8afeA5KgeOszLqzaBJHE7DarQwXXGRetCChbcAweILrww0ZBFkG1t9edwv72TiMEn QKjN+jkdx+NYQ4+ARpXT6IlIqS7lPwgF42far6PYn2ynccruxZM/iZvkup014aI/X3Sw vetQ== X-Gm-Message-State: AOAM533+sDlZWWIQraK9zl57SzkM8DsOKYyMiOd9sxFmZ4Fn2AIJuk/3 pNdZOsAQq5A4KSK4scgk0q4M7geg2i3I/K1CMg== X-Google-Smtp-Source: ABdhPJydJvl3PC03wLHBI/sDcDMdjPhmIoqIJeNbRR3IGq1cA8fGfxZtEiuFMiAWHLE1skqG2pTkYV1aYIAcDKhP0g== X-Received: from kaleshsingh.mtv.corp.google.com ([2620:15c:211:200:b783:5702:523e:d435]) (user=kaleshsingh job=sendgmr) by 2002:a25:f50b:: with SMTP id a11mr17568377ybe.241.1635189831607; Mon, 25 Oct 2021 12:23:51 -0700 (PDT) Date: Mon, 25 Oct 2021 12:23:12 -0700 In-Reply-To: <20211025192330.2992076-1-kaleshsingh@google.com> Message-Id: <20211025192330.2992076-2-kaleshsingh@google.com> Mime-Version: 1.0 References: <20211025192330.2992076-1-kaleshsingh@google.com> X-Mailer: git-send-email 2.33.0.1079.g6e70778dc9-goog Subject: [PATCH v3 1/8] tracing: Add support for creating hist trigger variables from literal From: Kalesh Singh Cc: surenb@google.com, hridya@google.com, namhyung@kernel.org, kernel-team@android.com, Kalesh Singh , Jonathan Corbet , Steven Rostedt , Ingo Molnar , Shuah Khan , Masami Hiramatsu , Tom Zanussi , linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org To: unlisted-recipients:; (no To-header on input) Precedence: bulk List-ID: X-Mailing-List: linux-kselftest@vger.kernel.org Currently hist trigger expressions don't support the use of numeric literals: e.g. echo 'hist:keys=common_pid:x=$y-1234' --> is not valid expression syntax Having the ability to use numeric constants in hist triggers supports a wider range of expressions for creating variables. Add support for creating trace event histogram variables from numeric literals. e.g. echo 'hist:keys=common_pid:x=1234,y=size-1024' >> event/trigger A negative numeric constant is created, using unary minus operator (parentheses are required). e.g. echo 'hist:keys=common_pid:z=-(2)' >> event/trigger Constants can be used with division/multiplication (added in the next patch in this series) to implement granularity filters for frequent trace events. For instance we can limit emitting the rss_stat trace event to when there is a 512KB cross over in the rss size: # Create a synthetic event to monitor instead of the high frequency # rss_stat event echo 'rss_stat_throttled unsigned int mm_id; unsigned int curr; int member; long size' >> tracing/synthetic_events # Create a hist trigger that emits the synthetic rss_stat_throttled # event only when the rss size crosses a 512KB boundary. echo 'hist:keys=keys=mm_id,member:bucket=size/0x80000:onchange($bucket) .rss_stat_throttled(mm_id,curr,member,size)' >> events/kmem/rss_stat/trigger A use case for using constants with addition/subtraction is not yet known, but for completeness the use of constants are supported for all operators. Signed-off-by: Kalesh Singh Change-Id: I142121d28dc3475dfbc3a882e7b2368d833474eb --- Changes in v3: - Remove the limit on the number of constants that can be created, per Steven Rostedt Changes in v2: - Add description of use case for constants in arithmetic operations in commit message, per Steven Rostedt - Add Namhyung's Reviewed-by kernel/trace/trace_events_hist.c | 71 +++++++++++++++++++++++++++++++- 1 file changed, 70 insertions(+), 1 deletion(-) diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c index f01e442716e2..28f711224944 100644 --- a/kernel/trace/trace_events_hist.c +++ b/kernel/trace/trace_events_hist.c @@ -66,7 +66,8 @@ C(EMPTY_SORT_FIELD, "Empty sort field"), \ C(TOO_MANY_SORT_FIELDS, "Too many sort fields (Max = 2)"), \ C(INVALID_SORT_FIELD, "Sort field must be a key or a val"), \ - C(INVALID_STR_OPERAND, "String type can not be an operand in expression"), + C(INVALID_STR_OPERAND, "String type can not be an operand in expression"), \ + C(EXPECT_NUMBER, "Expecting numeric literal"), #undef C #define C(a, b) HIST_ERR_##a @@ -89,6 +90,7 @@ typedef u64 (*hist_field_fn_t) (struct hist_field *field, #define HIST_FIELD_OPERANDS_MAX 2 #define HIST_FIELDS_MAX (TRACING_MAP_FIELDS_MAX + TRACING_MAP_VARS_MAX) #define HIST_ACTIONS_MAX 8 +#define HIST_CONST_DIGITS_MAX 21 enum field_op_id { FIELD_OP_NONE, @@ -152,6 +154,9 @@ struct hist_field { bool read_once; unsigned int var_str_idx; + + /* Numeric literals are represented as u64 */ + u64 constant; }; static u64 hist_field_none(struct hist_field *field, @@ -163,6 +168,15 @@ static u64 hist_field_none(struct hist_field *field, return 0; } +static u64 hist_field_const(struct hist_field *field, + struct tracing_map_elt *elt, + struct trace_buffer *buffer, + struct ring_buffer_event *rbe, + void *event) +{ + return field->constant; +} + static u64 hist_field_counter(struct hist_field *field, struct tracing_map_elt *elt, struct trace_buffer *buffer, @@ -341,6 +355,7 @@ enum hist_field_flags { HIST_FIELD_FL_CPU = 1 << 15, HIST_FIELD_FL_ALIAS = 1 << 16, HIST_FIELD_FL_BUCKET = 1 << 17, + HIST_FIELD_FL_CONST = 1 << 18, }; struct var_defs { @@ -1516,6 +1531,12 @@ static void expr_field_str(struct hist_field *field, char *expr) { if (field->flags & HIST_FIELD_FL_VAR_REF) strcat(expr, "$"); + else if (field->flags & HIST_FIELD_FL_CONST) { + char str[HIST_CONST_DIGITS_MAX]; + + snprintf(str, HIST_CONST_DIGITS_MAX, "%llu", field->constant); + strcat(expr, str); + } strcat(expr, hist_field_name(field, 0)); @@ -1689,6 +1710,15 @@ static struct hist_field *create_hist_field(struct hist_trigger_data *hist_data, goto out; } + if (flags & HIST_FIELD_FL_CONST) { + hist_field->fn = hist_field_const; + hist_field->size = sizeof(u64); + hist_field->type = kstrdup("u64", GFP_KERNEL); + if (!hist_field->type) + goto free; + goto out; + } + if (flags & HIST_FIELD_FL_STACKTRACE) { hist_field->fn = hist_field_none; goto out; @@ -2090,6 +2120,29 @@ static struct hist_field *create_alias(struct hist_trigger_data *hist_data, return alias; } +static struct hist_field *parse_const(struct hist_trigger_data *hist_data, + char *str, char *var_name, + unsigned long *flags) +{ + struct trace_array *tr = hist_data->event_file->tr; + struct hist_field *field = NULL; + u64 constant; + + if (kstrtoull(str, 0, &constant)) { + hist_err(tr, HIST_ERR_EXPECT_NUMBER, errpos(str)); + return NULL; + } + + *flags |= HIST_FIELD_FL_CONST; + field = create_hist_field(hist_data, NULL, *flags, var_name); + if (!field) + return NULL; + + field->constant = constant; + + return field; +} + static struct hist_field *parse_atom(struct hist_trigger_data *hist_data, struct trace_event_file *file, char *str, unsigned long *flags, char *var_name) @@ -2100,6 +2153,15 @@ static struct hist_field *parse_atom(struct hist_trigger_data *hist_data, unsigned long buckets = 0; int ret = 0; + if (isdigit(str[0])) { + hist_field = parse_const(hist_data, str, var_name, flags); + if (!hist_field) { + ret = -EINVAL; + goto out; + } + return hist_field; + } + s = strchr(str, '.'); if (s) { s = strchr(++s, '.'); @@ -4950,6 +5012,8 @@ static void hist_field_debug_show_flags(struct seq_file *m, if (flags & HIST_FIELD_FL_ALIAS) seq_puts(m, " HIST_FIELD_FL_ALIAS\n"); + else if (flags & HIST_FIELD_FL_CONST) + seq_puts(m, " HIST_FIELD_FL_CONST\n"); } static int hist_field_debug_show(struct seq_file *m, @@ -4971,6 +5035,9 @@ static int hist_field_debug_show(struct seq_file *m, field->var.idx); } + if (field->flags & HIST_FIELD_FL_CONST) + seq_printf(m, " constant: %llu\n", field->constant); + if (field->flags & HIST_FIELD_FL_ALIAS) seq_printf(m, " var_ref_idx (into hist_data->var_refs[]): %u\n", field->var_ref_idx); @@ -5213,6 +5280,8 @@ static void hist_field_print(struct seq_file *m, struct hist_field *hist_field) if (hist_field->flags & HIST_FIELD_FL_CPU) seq_puts(m, "common_cpu"); + else if (hist_field->flags & HIST_FIELD_FL_CONST) + seq_printf(m, "%llu", hist_field->constant); else if (field_name) { if (hist_field->flags & HIST_FIELD_FL_VAR_REF || hist_field->flags & HIST_FIELD_FL_ALIAS) From patchwork Mon Oct 25 19:23:13 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kalesh Singh X-Patchwork-Id: 12582729 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 015FFC433F5 for ; Mon, 25 Oct 2021 19:57:03 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id D002060E75 for ; Mon, 25 Oct 2021 19:57:02 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S237355AbhJYT7X (ORCPT ); Mon, 25 Oct 2021 15:59:23 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:57940 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S238216AbhJYT4Z (ORCPT ); Mon, 25 Oct 2021 15:56:25 -0400 Received: from mail-yb1-xb4a.google.com (mail-yb1-xb4a.google.com [IPv6:2607:f8b0:4864:20::b4a]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 1198AC110F25 for ; Mon, 25 Oct 2021 12:24:09 -0700 (PDT) Received: by mail-yb1-xb4a.google.com with SMTP id u17-20020a25ab11000000b005c1620952bfso9166319ybi.14 for ; Mon, 25 Oct 2021 12:24:09 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=date:in-reply-to:message-id:mime-version:references:subject:from:cc; bh=+/27iLcVwsoRtZBbebC5PI/8sCrYV9o5nqqwoDvFwLQ=; b=iJyBPNByASMp+w6u1OkWWEXPmlY+xuxhMQCry00NkR6CZi5U11wQfoXyJgPVnrdsN5 l1jlpID4yAeJ6DA9F5o77Qmt4Csbolwyo+aERKpL88HbbqOxBzalYfHIK6EUn34ingcM 9ulqgmaEh9+s8F/utYRhLzwDCF4f7Vhbd/MkFGip8zQUYpVh+9+IKxarCXs1aecowlls uDja4f+k3tDJ3CTj/p4GefB9//lhLSLWJr/t8VwkLvNLZbOkqBmJ7pmvCWpSIVKkpcN0 lC9d5sRZYXXm/r8DN7C9JO8C5G7Q3ApBIa4JlH9p/h1E9mbny8IvZVVQ2FqzyJBZ3lyr YX3A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:in-reply-to:message-id:mime-version :references:subject:from:cc; bh=+/27iLcVwsoRtZBbebC5PI/8sCrYV9o5nqqwoDvFwLQ=; b=5kGUyMW3dVYLuxHEjhna9EP9FAeHfnN7Q8vVnuGkGHnVYA3t/+H/mbM8RJ7OaMlEHV nbkdZWO8VVcNuBbfB6WSrc3Je4KeQZU+gMZY3HHMRN1XNtqZN1BIbACq6Ee13S2bdFPx IeEd10/60bGV/Jai29QZGz5LTLUCJNALqF5WiI8I/z4dQQgoi9Mop+zEy57h8w94icJc XZmg2vn9LCzAX/TiIZcUQ/JZkzZwNtfNuT1Hvv0yFI9xH91IzzL8nIyQUX6JWGX24fxp LGNs1g0yTIhiDViav8uXcxlIO6VuiU5A4CL09YT5qZWmgdOHAhJsvoxOZWe+A7fgAY+L y/Ow== X-Gm-Message-State: AOAM533F991Ke3Bn/H5N3UD3K/fZjQ80XCcoC5Y+K6GHn1OQJXQdwhmF VU0bihs9qFcqofE3WFqvSDwf7uzAz90V+ckjTQ== X-Google-Smtp-Source: ABdhPJzGkE0zCUemaF+iHnCTNiduZE54B6+Q+I95bSiWKG4W1OfAzxxaOmrl4UL79SWQbI9iCBMUB9r9pO5DM4x0nw== X-Received: from kaleshsingh.mtv.corp.google.com ([2620:15c:211:200:b783:5702:523e:d435]) (user=kaleshsingh job=sendgmr) by 2002:a05:6902:501:: with SMTP id x1mr19798810ybs.217.1635189848312; Mon, 25 Oct 2021 12:24:08 -0700 (PDT) Date: Mon, 25 Oct 2021 12:23:13 -0700 In-Reply-To: <20211025192330.2992076-1-kaleshsingh@google.com> Message-Id: <20211025192330.2992076-3-kaleshsingh@google.com> Mime-Version: 1.0 References: <20211025192330.2992076-1-kaleshsingh@google.com> X-Mailer: git-send-email 2.33.0.1079.g6e70778dc9-goog Subject: [PATCH v3 2/8] tracing: Add division and multiplication support for hist triggers From: Kalesh Singh Cc: surenb@google.com, hridya@google.com, namhyung@kernel.org, kernel-team@android.com, Kalesh Singh , Jonathan Corbet , Steven Rostedt , Ingo Molnar , Shuah Khan , Masami Hiramatsu , Tom Zanussi , linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org To: unlisted-recipients:; (no To-header on input) Precedence: bulk List-ID: X-Mailing-List: linux-kselftest@vger.kernel.org Adds basic support for division and multiplication operations for hist trigger variable expressions. For simplicity this patch only supports, division and multiplication for a single operation expression (e.g. x=$a/$b), as currently expressions are always evaluated right to left. This can lead to some incorrect results: e.g. echo 'hist:keys=common_pid:x=8-4-2' >> event/trigger 8-4-2 should evaluate to 2 i.e. (8-4)-2 but currently x evaluate to 6 i.e. 8-(4-2) Multiplication and division in sub-expressions will work correctly, once correct operator precedence support is added (See next patch in this series). For the undefined case of division by 0, the histogram expression evaluates to (u64)(-1). Since this cannot be detected when the expression is created, it is the responsibility of the user to be aware and account for this possibility. Examples: echo 'hist:keys=common_pid:a=8,b=4,x=$a/$b' \ >> event/trigger echo 'hist:keys=common_pid:y=5*$b' \ >> event/trigger Signed-off-by: Kalesh Singh --- Changes in v2: - Use div64 helper in hist_field_div() to avoid faults on x86 32-bit machines, per Steven Rostedt kernel/trace/trace_events_hist.c | 72 +++++++++++++++++++++++++++++++- 1 file changed, 71 insertions(+), 1 deletion(-) diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c index 28f711224944..522355a06f58 100644 --- a/kernel/trace/trace_events_hist.c +++ b/kernel/trace/trace_events_hist.c @@ -97,6 +97,8 @@ enum field_op_id { FIELD_OP_PLUS, FIELD_OP_MINUS, FIELD_OP_UNARY_MINUS, + FIELD_OP_DIV, + FIELD_OP_MULT, }; /* @@ -285,6 +287,40 @@ static u64 hist_field_minus(struct hist_field *hist_field, return val1 - val2; } +static u64 hist_field_div(struct hist_field *hist_field, + struct tracing_map_elt *elt, + struct trace_buffer *buffer, + struct ring_buffer_event *rbe, + void *event) +{ + struct hist_field *operand1 = hist_field->operands[0]; + struct hist_field *operand2 = hist_field->operands[1]; + + u64 val1 = operand1->fn(operand1, elt, buffer, rbe, event); + u64 val2 = operand2->fn(operand2, elt, buffer, rbe, event); + + /* Return -1 for the undefined case */ + if (!val2) + return -1; + + return div64_u64(val1, val2); +} + +static u64 hist_field_mult(struct hist_field *hist_field, + struct tracing_map_elt *elt, + struct trace_buffer *buffer, + struct ring_buffer_event *rbe, + void *event) +{ + struct hist_field *operand1 = hist_field->operands[0]; + struct hist_field *operand2 = hist_field->operands[1]; + + u64 val1 = operand1->fn(operand1, elt, buffer, rbe, event); + u64 val2 = operand2->fn(operand2, elt, buffer, rbe, event); + + return val1 * val2; +} + static u64 hist_field_unary_minus(struct hist_field *hist_field, struct tracing_map_elt *elt, struct trace_buffer *buffer, @@ -1592,6 +1628,12 @@ static char *expr_str(struct hist_field *field, unsigned int level) case FIELD_OP_PLUS: strcat(expr, "+"); break; + case FIELD_OP_DIV: + strcat(expr, "/"); + break; + case FIELD_OP_MULT: + strcat(expr, "*"); + break; default: kfree(expr); return NULL; @@ -1607,7 +1649,7 @@ static int contains_operator(char *str) enum field_op_id field_op = FIELD_OP_NONE; char *op; - op = strpbrk(str, "+-"); + op = strpbrk(str, "+-/*"); if (!op) return FIELD_OP_NONE; @@ -1628,6 +1670,12 @@ static int contains_operator(char *str) case '+': field_op = FIELD_OP_PLUS; break; + case '/': + field_op = FIELD_OP_DIV; + break; + case '*': + field_op = FIELD_OP_MULT; + break; default: break; } @@ -2361,10 +2409,26 @@ static struct hist_field *parse_expr(struct hist_trigger_data *hist_data, case FIELD_OP_PLUS: sep = "+"; break; + case FIELD_OP_DIV: + sep = "/"; + break; + case FIELD_OP_MULT: + sep = "*"; + break; default: goto free; } + /* + * Multiplication and division are only supported in single operator + * expressions, since the expression is always evaluated from right + * to left. + */ + if ((field_op == FIELD_OP_DIV || field_op == FIELD_OP_MULT) && level > 0) { + hist_err(file->tr, HIST_ERR_TOO_MANY_SUBEXPR, errpos(str)); + return ERR_PTR(-EINVAL); + } + operand1_str = strsep(&str, sep); if (!operand1_str || !str) goto free; @@ -2436,6 +2500,12 @@ static struct hist_field *parse_expr(struct hist_trigger_data *hist_data, 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; From patchwork Mon Oct 25 19:23:14 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kalesh Singh X-Patchwork-Id: 12582731 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 BEB3AC433F5 for ; Mon, 25 Oct 2021 19:57:48 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id A748360E09 for ; Mon, 25 Oct 2021 19:57:48 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S237440AbhJYUAH (ORCPT ); Mon, 25 Oct 2021 16:00:07 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:59132 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S239844AbhJYT6G (ORCPT ); Mon, 25 Oct 2021 15:58:06 -0400 Received: from mail-pj1-x1049.google.com (mail-pj1-x1049.google.com [IPv6:2607:f8b0:4864:20::1049]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id D171FC110F31 for ; Mon, 25 Oct 2021 12:24:28 -0700 (PDT) Received: by mail-pj1-x1049.google.com with SMTP id m4-20020a17090a2c0400b001a1f07cc9c4so125806pjd.8 for ; Mon, 25 Oct 2021 12:24:28 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=date:in-reply-to:message-id:mime-version:references:subject:from:cc; bh=iRuPYXoXcrU+gBqRAGhoDpe/K+LXE+9n+7UkO6cV9+w=; b=C/w3/Y+X9ucU0aKHe7sgjL9qLWwXwIDcQ05xFVMWZLvrVkP4c9emtFp5Uxzs79/crJ yvRsNl6KDxBFprdibyFW3P+jHgiVZDAaN68RjIr8nCgtVzVQ+TZ1gAd/aRmBBuXORpxk VrmFb4cooL3PG+8ZX8M6ESNLSCJF0BUef4Vm5eOHz1PCWTymCZi/AuRl9r78fvXLM1zH VwZAqF5e4mrEGCv74MeMV7Ow5jtoFAJRzwWlVX03W0KlUULI+GTVTM3Geu9OqcoGbUqc lI04VXu/EPGxxRcHR1cVup5CagWGl6NI4OODPdl6iu0nL9nVDaitA/vhLY0N1s7bXOcs cSjA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:in-reply-to:message-id:mime-version :references:subject:from:cc; bh=iRuPYXoXcrU+gBqRAGhoDpe/K+LXE+9n+7UkO6cV9+w=; b=CasZvdE1IbPHCNTQgxTgv1fQntUMrYm3VE4g8/dAuCabE2a2gj4jWuQk/1k6j4XYX1 YDEHf/6slVS0bo0bG9InGB9Hyjdgav4OibF4eCteJE2hzEPbh3LesxSwDUecmkV3B8zW K2m8whIxkPHofKoKX9wDVMD8RBAiHeIom5uAhKFnwZsnldmMIiDLBJE8WA9/1kenaTW2 m9nwJ/mqlN4qmlpP6rXiQDprWLXLjqZtBL0ltxoVyLtsIASHeEhfqCQSTCJNnq+kYuYe /faqTQTdx5jSBTE5unZTMcCLoBHInlRzep7+20AhoeLdD9qCDNkr2MrbVZCl9ip/wYkK /vHw== X-Gm-Message-State: AOAM531SLP7w4tk3Z1mRlVsC2E9U6mDZDYjAqTpquuapmCsAjZ+FHwHh 4DMKa9BBTQxj3SYW5is7hOpAhj5N/YD/1UtGBw== X-Google-Smtp-Source: ABdhPJyOaT7f+j9Oif9wq26bb+2Vo9pdLPQEuCnvGRv9tJLI2vMGL2UwUS06rSguFhA58+v2E0HYvItJoLSR56xIwQ== X-Received: from kaleshsingh.mtv.corp.google.com ([2620:15c:211:200:b783:5702:523e:d435]) (user=kaleshsingh job=sendgmr) by 2002:a17:902:db02:b0:140:581e:6eb5 with SMTP id m2-20020a170902db0200b00140581e6eb5mr7017995plx.46.1635189868271; Mon, 25 Oct 2021 12:24:28 -0700 (PDT) Date: Mon, 25 Oct 2021 12:23:14 -0700 In-Reply-To: <20211025192330.2992076-1-kaleshsingh@google.com> Message-Id: <20211025192330.2992076-4-kaleshsingh@google.com> Mime-Version: 1.0 References: <20211025192330.2992076-1-kaleshsingh@google.com> X-Mailer: git-send-email 2.33.0.1079.g6e70778dc9-goog Subject: [PATCH v3 3/8] tracing: Fix operator precedence for hist triggers expression From: Kalesh Singh Cc: surenb@google.com, hridya@google.com, namhyung@kernel.org, kernel-team@android.com, Kalesh Singh , Jonathan Corbet , Steven Rostedt , Ingo Molnar , Shuah Khan , Masami Hiramatsu , Tom Zanussi , linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org To: unlisted-recipients:; (no To-header on input) Precedence: bulk List-ID: X-Mailing-List: linux-kselftest@vger.kernel.org The current histogram expression evaluation logic evaluates the expression from right to left. This can lead to incorrect results if the operations are not associative (as is the case for subtraction and, the now added, division operators). e.g. 16-8-4-2 should be 2 not 10 --> 16-8-4-2 = ((16-8)-4)-2 64/8/4/2 should be 1 not 16 --> 64/8/4/2 = ((64/8)/4)/2 Division and multiplication are currently limited to single operation expression due to operator precedence support not yet implemented. Rework the expression parsing to support the correct evaluation of expressions containing operators of different precedences; and fix the associativity error by evaluating expressions with operators of the same precedence from left to right. Examples: (1) echo 'hist:keys=common_pid:a=8,b=4,c=2,d=1,w=$a-$b-$c-$d' \ >> event/trigger (2) echo 'hist:keys=common_pid:x=$a/$b/3/2' >> event/trigger (3) echo 'hist:keys=common_pid:y=$a+10/$c*1024' >> event/trigger (4) echo 'hist:keys=common_pid:z=$a/$b+$c*$d' >> event/trigger Signed-off-by: Kalesh Singh Reviewed-by: Namhyung Kim --- Changed in v2: - Add Namhyung's Reviewed-by kernel/trace/trace_events_hist.c | 210 ++++++++++++++++++++----------- 1 file changed, 140 insertions(+), 70 deletions(-) diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c index 522355a06f58..e10c7d9611e5 100644 --- a/kernel/trace/trace_events_hist.c +++ b/kernel/trace/trace_events_hist.c @@ -67,7 +67,9 @@ C(TOO_MANY_SORT_FIELDS, "Too many sort fields (Max = 2)"), \ C(INVALID_SORT_FIELD, "Sort field must be a key or a val"), \ C(INVALID_STR_OPERAND, "String type can not be an operand in expression"), \ - C(EXPECT_NUMBER, "Expecting numeric literal"), + C(EXPECT_NUMBER, "Expecting numeric literal"), \ + C(UNARY_MINUS_SUBEXPR, "Unary minus not supported in sub-expressions"), \ + C(SYM_OFFSET_SUBEXPR, ".sym-offset not supported in sub-expressions"), #undef C #define C(a, b) HIST_ERR_##a @@ -1644,40 +1646,96 @@ static char *expr_str(struct hist_field *field, unsigned int level) return expr; } -static int contains_operator(char *str) +/* + * If field_op != FIELD_OP_NONE, *sep points to the root operator + * of the expression tree to be evaluated. + */ +static int contains_operator(char *str, char **sep) { enum field_op_id field_op = FIELD_OP_NONE; - char *op; + char *minus_op, *plus_op, *div_op, *mult_op; + + + /* + * Report the last occurrence of the operators first, so that the + * expression is evaluated left to right. This is important since + * subtraction and division are not associative. + * + * e.g + * 64/8/4/2 is 1, i.e 64/8/4/2 = ((64/8)/4)/2 + * 14-7-5-2 is 0, i.e 14-7-5-2 = ((14-7)-5)-2 + */ - op = strpbrk(str, "+-/*"); - if (!op) - return FIELD_OP_NONE; + /* + * First, find lower precedence addition and subtraction + * since the expression will be evaluated recursively. + */ + minus_op = strrchr(str, '-'); + if (minus_op) { + /* Unfortunately, the modifier ".sym-offset" can confuse things. */ + if (minus_op - str >= 4 && !strncmp(minus_op - 4, ".sym-offset", 11)) + goto out; - switch (*op) { - case '-': /* - * Unfortunately, the modifier ".sym-offset" - * can confuse things. + * Unary minus is not supported in sub-expressions. If + * present, it is always the next root operator. */ - if (op - str >= 4 && !strncmp(op - 4, ".sym-offset", 11)) - return FIELD_OP_NONE; - - if (*str == '-') + if (minus_op == str) { field_op = FIELD_OP_UNARY_MINUS; - else - field_op = FIELD_OP_MINUS; - break; - case '+': - field_op = FIELD_OP_PLUS; - break; - case '/': + goto out; + } + + field_op = FIELD_OP_MINUS; + } + + plus_op = strrchr(str, '+'); + if (plus_op || minus_op) { + /* + * For operators of the same precedence use to rightmost as the + * root, so that the expression is evaluated left to right. + */ + if (plus_op > minus_op) + field_op = FIELD_OP_PLUS; + goto out; + } + + /* + * Multiplication and division have higher precedence than addition and + * subtraction. + */ + div_op = strrchr(str, '/'); + if (div_op) field_op = FIELD_OP_DIV; - break; - case '*': + + mult_op = strrchr(str, '*'); + /* + * For operators of the same precedence use to rightmost as the + * root, so that the expression is evaluated left to right. + */ + if (mult_op > div_op) field_op = FIELD_OP_MULT; - break; - default: - break; + +out: + if (sep) { + switch (field_op) { + case FIELD_OP_UNARY_MINUS: + case FIELD_OP_MINUS: + *sep = minus_op; + break; + case FIELD_OP_PLUS: + *sep = plus_op; + break; + case FIELD_OP_DIV: + *sep = div_op; + break; + case FIELD_OP_MULT: + *sep = mult_op; + break; + case FIELD_OP_NONE: + default: + *sep = NULL; + break; + } } return field_op; @@ -2003,7 +2061,7 @@ static char *field_name_from_var(struct hist_trigger_data *hist_data, if (strcmp(var_name, name) == 0) { field = hist_data->attrs->var_defs.expr[i]; - if (contains_operator(field) || is_var_ref(field)) + if (contains_operator(field, NULL) || is_var_ref(field)) continue; return field; } @@ -2266,21 +2324,24 @@ static struct hist_field *parse_atom(struct hist_trigger_data *hist_data, 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); static struct hist_field *parse_unary(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, *expr = NULL; unsigned long operand_flags; int ret = 0; char *s; + /* Unary minus operator, increment n_subexprs */ + ++*n_subexprs; + /* we support only -(xxx) i.e. explicit parens required */ - if (level > 3) { + if (*n_subexprs > 3) { hist_err(file->tr, HIST_ERR_TOO_MANY_SUBEXPR, errpos(str)); ret = -EINVAL; goto free; @@ -2297,8 +2358,16 @@ static struct hist_field *parse_unary(struct hist_trigger_data *hist_data, } s = strrchr(str, ')'); - if (s) + if (s) { + /* unary minus not supported in sub-expressions */ + if (*(s+1) != '\0') { + hist_err(file->tr, HIST_ERR_UNARY_MINUS_SUBEXPR, + errpos(str)); + ret = -EINVAL; + goto free; + } *s = '\0'; + } else { ret = -EINVAL; /* no closing ')' */ goto free; @@ -2312,7 +2381,7 @@ static struct hist_field *parse_unary(struct hist_trigger_data *hist_data, } operand_flags = 0; - operand1 = parse_expr(hist_data, file, str, operand_flags, NULL, ++level); + operand1 = parse_expr(hist_data, file, str, operand_flags, NULL, n_subexprs); if (IS_ERR(operand1)) { ret = PTR_ERR(operand1); goto free; @@ -2382,60 +2451,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) { hist_err(file->tr, HIST_ERR_TOO_MANY_SUBEXPR, errpos(str)); return ERR_PTR(-EINVAL); } - field_op = contains_operator(str); + /* + * ".sym-offset" in expressions has no effect on their evaluation, + * but can confuse operator parsing. + */ + if (*n_subexprs == 0) { + sep = strstr(str, ".sym-offset"); + if (sep) { + *sep = '\0'; + if (strpbrk(str, "+-/*") || strpbrk(sep + 11, "+-/*")) { + *sep = '.'; + hist_err(file->tr, HIST_ERR_SYM_OFFSET_SUBEXPR, + errpos(sep)); + return ERR_PTR(-EINVAL); + } + *sep = '.'; + } + } + + field_op = contains_operator(str, &sep); if (field_op == FIELD_OP_NONE) return parse_atom(hist_data, file, str, &flags, var_name); if (field_op == FIELD_OP_UNARY_MINUS) - return parse_unary(hist_data, file, str, flags, var_name, ++level); + return parse_unary(hist_data, file, str, flags, var_name, n_subexprs); - switch (field_op) { - case FIELD_OP_MINUS: - sep = "-"; - break; - case FIELD_OP_PLUS: - sep = "+"; - break; - case FIELD_OP_DIV: - sep = "/"; - break; - case FIELD_OP_MULT: - sep = "*"; - break; - default: - goto free; - } + /* Binary operator found, increment n_subexprs */ + ++*n_subexprs; - /* - * Multiplication and division are only supported in single operator - * expressions, since the expression is always evaluated from right - * to left. - */ - if ((field_op == FIELD_OP_DIV || field_op == FIELD_OP_MULT) && level > 0) { - hist_err(file->tr, HIST_ERR_TOO_MANY_SUBEXPR, errpos(str)); - return ERR_PTR(-EINVAL); - } + /* Split the expression string at the root operator */ + if (!sep) + goto free; + *sep = '\0'; + operand1_str = str; + str = sep+1; - operand1_str = strsep(&str, sep); if (!operand1_str || !str) goto free; operand_flags = 0; - operand1 = parse_atom(hist_data, file, operand1_str, - &operand_flags, NULL); + + /* 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; @@ -2447,9 +2517,9 @@ static struct hist_field *parse_expr(struct hist_trigger_data *hist_data, goto free; } - /* rest of string could be another expression e.g. b+c in a+b+c */ + /* RHS of string is another expression e.g. c in a+b+c */ operand_flags = 0; - operand2 = parse_expr(hist_data, file, str, operand_flags, NULL, ++level); + operand2 = parse_expr(hist_data, file, str, operand_flags, NULL, n_subexprs); if (IS_ERR(operand2)) { ret = PTR_ERR(operand2); operand2 = NULL; @@ -3883,9 +3953,9 @@ static int __create_val_field(struct hist_trigger_data *hist_data, unsigned long flags) { struct hist_field *hist_field; - int ret = 0; + int ret = 0, n_subexprs = 0; - hist_field = parse_expr(hist_data, file, field_str, flags, var_name, 0); + hist_field = parse_expr(hist_data, file, field_str, flags, var_name, &n_subexprs); if (IS_ERR(hist_field)) { ret = PTR_ERR(hist_field); goto out; @@ -4026,7 +4096,7 @@ static int create_key_field(struct hist_trigger_data *hist_data, struct hist_field *hist_field = NULL; unsigned long flags = 0; unsigned int key_size; - int ret = 0; + int ret = 0, n_subexprs = 0; if (WARN_ON(key_idx >= HIST_FIELDS_MAX)) return -EINVAL; @@ -4039,7 +4109,7 @@ static int create_key_field(struct hist_trigger_data *hist_data, hist_field = create_hist_field(hist_data, NULL, flags, NULL); } else { hist_field = parse_expr(hist_data, file, field_str, flags, - NULL, 0); + NULL, &n_subexprs); if (IS_ERR(hist_field)) { ret = PTR_ERR(hist_field); goto out; From patchwork Mon Oct 25 19:23:15 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kalesh Singh X-Patchwork-Id: 12582759 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 6BAF8C433EF for ; Mon, 25 Oct 2021 19:59:06 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 51B5B60E09 for ; Mon, 25 Oct 2021 19:59:06 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S236825AbhJYUB1 (ORCPT ); Mon, 25 Oct 2021 16:01:27 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:58546 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S237504AbhJYT70 (ORCPT ); Mon, 25 Oct 2021 15:59:26 -0400 Received: from mail-yb1-xb49.google.com (mail-yb1-xb49.google.com [IPv6:2607:f8b0:4864:20::b49]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 04EEBC07978D for ; Mon, 25 Oct 2021 12:24:51 -0700 (PDT) Received: by mail-yb1-xb49.google.com with SMTP id z2-20020a254c02000000b005b68ef4fe24so18915167yba.11 for ; Mon, 25 Oct 2021 12:24:50 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=date:in-reply-to:message-id:mime-version:references:subject:from:cc; bh=40aYx6z+Qhs6iidkMxzLHXZ7pLr5WPfaLZnNc4JCIAA=; b=D2oramfk/k05LngaIBN5DtDp30x+nHc792NEbG/KPYSaJfYrEB3RS29H6763RfdtTK 2jp4uSN9NAFPr+eMPR0/iZQm5rBySPRST2gawSQXstX2HToQiUfQc5TATmc0HtAnQ9a+ dLsFlTcwznpcNJcYkVkIBJD3fYWPjss5VWDkFmDub98BnoKZybHc6aC+6UnvNCcuCir9 bWKP5smBY0skebQ7IA7jmo5ydquhbrwVxShExGE8ghUmjN7jen8IH9cpdA6Sv6eVPPO1 UbKSs+54w4dA9FiYN87UNisxX/HHekOTs1lw+aRQlnR3D4gdzckLlAAMW1EU0ABOQSIP FqsQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:in-reply-to:message-id:mime-version :references:subject:from:cc; bh=40aYx6z+Qhs6iidkMxzLHXZ7pLr5WPfaLZnNc4JCIAA=; b=Iu4ZG3bEZcTujKVTwh4FSPBrVThxkSX5gpM8Oxznr6GGu8Dtj7plZPqn05LexvP3Zt 3HOp38vpycINPETh+SLqMij/TGwM6CeByJpy/7kzb10+w56GumOfJXYkYtws6J0a1EqK BH763fAHT2sDUj6HRkbJVJoOeGv9nx7qIkxaMPJlkiRtYQ5GWTQ499QCRVguwYYErQlf 5j2YUqRZRo17GzfgwTBNlyewQv6KbRxF1hmmPHz8EAq62RDyHh/aT51EvKjjwcc/WkyX 3ffDqz5cZ6AlytPvRI4PSmhsGqTMQ6qMCdmeZIpaCqHSYCOJd/rwqKSpMdJUITNBnKMs 691A== X-Gm-Message-State: AOAM53175bRJ4ji1zwZ9hEE0oxU4rFhkG3yqYrwYTWk3RWxXGy8APPdv VT8C6sSAnVBdMGFmELNvHy+0IsD3npF3y6cuAA== X-Google-Smtp-Source: ABdhPJzXQSEveozcnHlXEPFOeRJ6Vbt+P+ua6zzBxcmrr6/3uDjpYGgYqu1ys9+QAKkZJ8KtUoN+ocDjdDUpMmF2Tg== X-Received: from kaleshsingh.mtv.corp.google.com ([2620:15c:211:200:b783:5702:523e:d435]) (user=kaleshsingh job=sendgmr) by 2002:a25:9847:: with SMTP id k7mr19933212ybo.170.1635189885470; Mon, 25 Oct 2021 12:24:45 -0700 (PDT) Date: Mon, 25 Oct 2021 12:23:15 -0700 In-Reply-To: <20211025192330.2992076-1-kaleshsingh@google.com> Message-Id: <20211025192330.2992076-5-kaleshsingh@google.com> Mime-Version: 1.0 References: <20211025192330.2992076-1-kaleshsingh@google.com> X-Mailer: git-send-email 2.33.0.1079.g6e70778dc9-goog Subject: [PATCH v3 4/8] tracing/histogram: Simplify handling of .sym-offset in expressions From: Kalesh Singh Cc: surenb@google.com, hridya@google.com, namhyung@kernel.org, kernel-team@android.com, Kalesh Singh , Steven Rostedt , 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 To: unlisted-recipients:; (no To-header on input) Precedence: bulk List-ID: X-Mailing-List: linux-kselftest@vger.kernel.org The '-' in .sym-offset can confuse the hist trigger arithmetic expression parsing. Simplify the handling of this by replacing the 'sym-offset' with 'symXoffset'. This allows us to correctly evaluate expressions where the user may have inadvertently added a .sym-offset modifier to one of the operands in an expression, instead of bailing out. In this case the .sym-offset has no effect on the evaluation of the expression. The only valid use of the .sym-offset is as a hist key modifier. Change-Id: Icef416c67138576718e25394d85f8e991d8850d6 Signed-off-by: Kalesh Singh Suggested-by: Steven Rostedt --- kernel/trace/trace_events_hist.c | 43 +++++++++++++------------------- 1 file changed, 17 insertions(+), 26 deletions(-) diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c index e10c7d9611e5..34aba07d23f8 100644 --- a/kernel/trace/trace_events_hist.c +++ b/kernel/trace/trace_events_hist.c @@ -68,8 +68,7 @@ C(INVALID_SORT_FIELD, "Sort field must be a key or a val"), \ C(INVALID_STR_OPERAND, "String type can not be an operand in expression"), \ C(EXPECT_NUMBER, "Expecting numeric literal"), \ - C(UNARY_MINUS_SUBEXPR, "Unary minus not supported in sub-expressions"), \ - C(SYM_OFFSET_SUBEXPR, ".sym-offset not supported in sub-expressions"), + C(UNARY_MINUS_SUBEXPR, "Unary minus not supported in sub-expressions"), #undef C #define C(a, b) HIST_ERR_##a @@ -1672,10 +1671,6 @@ static int contains_operator(char *str, char **sep) */ minus_op = strrchr(str, '-'); if (minus_op) { - /* Unfortunately, the modifier ".sym-offset" can confuse things. */ - if (minus_op - str >= 4 && !strncmp(minus_op - 4, ".sym-offset", 11)) - goto out; - /* * Unary minus is not supported in sub-expressions. If * present, it is always the next root operator. @@ -2138,7 +2133,11 @@ parse_field(struct hist_trigger_data *hist_data, struct trace_event_file *file, *flags |= HIST_FIELD_FL_HEX; else if (strcmp(modifier, "sym") == 0) *flags |= HIST_FIELD_FL_SYM; - else if (strcmp(modifier, "sym-offset") == 0) + /* + * 'sym-offset' occurrences in the trigger string are modified + * to 'symXoffset' to simplify arithmetic expression parsing. + */ + else if (strcmp(modifier, "symXoffset") == 0) *flags |= HIST_FIELD_FL_SYM_OFFSET; else if ((strcmp(modifier, "execname") == 0) && (strcmp(field_name, "common_pid") == 0)) @@ -2463,24 +2462,6 @@ static struct hist_field *parse_expr(struct hist_trigger_data *hist_data, return ERR_PTR(-EINVAL); } - /* - * ".sym-offset" in expressions has no effect on their evaluation, - * but can confuse operator parsing. - */ - if (*n_subexprs == 0) { - sep = strstr(str, ".sym-offset"); - if (sep) { - *sep = '\0'; - if (strpbrk(str, "+-/*") || strpbrk(sep + 11, "+-/*")) { - *sep = '.'; - hist_err(file->tr, HIST_ERR_SYM_OFFSET_SUBEXPR, - errpos(sep)); - return ERR_PTR(-EINVAL); - } - *sep = '.'; - } - } - field_op = contains_operator(str, &sep); if (field_op == FIELD_OP_NONE) @@ -6004,7 +5985,7 @@ static int event_hist_trigger_func(struct event_command *cmd_ops, struct synth_event *se; const char *se_name; bool remove = false; - char *trigger, *p; + char *trigger, *p, *start; int ret = 0; lockdep_assert_held(&event_mutex); @@ -6052,6 +6033,16 @@ static int event_hist_trigger_func(struct event_command *cmd_ops, trigger = strstrip(trigger); } + /* + * To simplify arithmetic expression parsing, replace occurrences of + * '.sym-offset' modifier with '.symXoffset' + */ + start = strstr(trigger, ".sym-offset"); + while (start) { + *(start + 4) = 'X'; + start = strstr(start + 11, ".sym-offset"); + }; + attrs = parse_hist_trigger_attrs(file->tr, trigger); if (IS_ERR(attrs)) return PTR_ERR(attrs); From patchwork Mon Oct 25 19:23:16 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kalesh Singh X-Patchwork-Id: 12582761 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 E14FDC433EF for ; Mon, 25 Oct 2021 20:02:56 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id C865760F46 for ; Mon, 25 Oct 2021 20:02:56 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S237524AbhJYUFP (ORCPT ); Mon, 25 Oct 2021 16:05:15 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:59860 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S238911AbhJYUB0 (ORCPT ); Mon, 25 Oct 2021 16:01:26 -0400 Received: from mail-yb1-xb4a.google.com (mail-yb1-xb4a.google.com [IPv6:2607:f8b0:4864:20::b4a]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 57A30C036D4E for ; Mon, 25 Oct 2021 12:25:07 -0700 (PDT) Received: by mail-yb1-xb4a.google.com with SMTP id c84-20020a251c57000000b005c131bb8967so17703279ybc.9 for ; Mon, 25 Oct 2021 12:25:07 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=date:in-reply-to:message-id:mime-version:references:subject:from:cc; bh=9Ql4uehadw2j6BVRhe3fDBuv0Q3sPrmeMa11sG3Mghc=; b=CjK3qa+LRnQlrKD32MixlMBHWpmyPuaXfyrhPr29OWVk8ewAxNn+DC0BHIAzDtTjlL 2BqBiqVSHV4UrnVMNHuw7xGeKbjBbAVaeKQ4zA/r+R51XbksUQXHfn+8oH9rcKXFUEty KOWBw1LKNoCUsGt5ybafmz/39ZElM1WB4o21hxObJ5xVP6BGC3ccpLJ2HmVbOySGK5bk VhwlwMDX8U36F7f4xYMeKV6fvCuX3Ii81RXw2zM1+anZxHqhFq4kroM7p62+ckopc+Jq v0a+o4BiKNsJzEtGYyo+cnqe0bA6c+VoORdYVgTEM4vd5S1gs4epYdhTH/SStB7ggfNy q9bg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:in-reply-to:message-id:mime-version :references:subject:from:cc; bh=9Ql4uehadw2j6BVRhe3fDBuv0Q3sPrmeMa11sG3Mghc=; b=4WAocOHKkqF5ydUn7EPEqhMyLmRByo2fZ8s9CC/zV6SFwlOOyGCE8ulbJNOP2a7yDl aiRYGy6CPg0fcJ9aiboC0RKWvltVbZ9MisIXioewbNGsHccB58gdFt+PK8W+7NuOE/eS P04En0XfttHBxkwfs2G5yECW13/kjO7wApHe3fOtMuc+Ws0MlDj/v74MWmybyfXtCKrN ASAktBs+ih8Mz9oCbFwZi9UTABjfJ4j7hCKS1v0I0buayoWRRn1W/9ieY6nunibOxgmG UVnBNH9l3vcBGZVzulUa9cnAIsu+i8FGnmZoA4kkbtZceMkJdXsqVxTKowUSnT2guV08 Ea9Q== X-Gm-Message-State: AOAM530EQXcJj9hpci5AelDsRH+ffDN2YD5DNPTGGaT9gxF5u0TrXfpr VIIjbvsJm0/31KY7JsS2rJ/aQVevnF9pGQ7bCQ== X-Google-Smtp-Source: ABdhPJwEU718QD6CYBzlo0tkEFiliWh68r9hbOSAJN6K80wLyxtEhFO0eCAz5+0x8Zxjhkv8UpJU5Mr+B9UjHtdvgw== X-Received: from kaleshsingh.mtv.corp.google.com ([2620:15c:211:200:b783:5702:523e:d435]) (user=kaleshsingh job=sendgmr) by 2002:a25:86c4:: with SMTP id y4mr17365728ybm.258.1635189906533; Mon, 25 Oct 2021 12:25:06 -0700 (PDT) Date: Mon, 25 Oct 2021 12:23:16 -0700 In-Reply-To: <20211025192330.2992076-1-kaleshsingh@google.com> Message-Id: <20211025192330.2992076-6-kaleshsingh@google.com> Mime-Version: 1.0 References: <20211025192330.2992076-1-kaleshsingh@google.com> X-Mailer: git-send-email 2.33.0.1079.g6e70778dc9-goog Subject: [PATCH v3 5/8] tracing/histogram: Covert expr to const if both operands are constants From: Kalesh Singh Cc: surenb@google.com, hridya@google.com, namhyung@kernel.org, kernel-team@android.com, Kalesh Singh , Steven Rostedt , 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 To: unlisted-recipients:; (no To-header on input) Precedence: bulk List-ID: X-Mailing-List: linux-kselftest@vger.kernel.org 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. Change-Id: I83388bbbc6e109980f08eac437172238576055d5 Signed-off-by: Kalesh Singh Suggested-by: Steven Rostedt --- kernel/trace/trace_events_hist.c | 104 ++++++++++++++++++++++--------- 1 file changed, 74 insertions(+), 30 deletions(-) diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c index 34aba07d23f8..db28bcf976f4 100644 --- a/kernel/trace/trace_events_hist.c +++ b/kernel/trace/trace_events_hist.c @@ -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; @@ -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) || @@ -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) != @@ -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)); @@ -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); @@ -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); From patchwork Mon Oct 25 19:23:17 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kalesh Singh X-Patchwork-Id: 12582763 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 59845C4332F for ; Mon, 25 Oct 2021 20:02:59 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 43FC260E8B for ; Mon, 25 Oct 2021 20:02:59 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S237856AbhJYUFS (ORCPT ); Mon, 25 Oct 2021 16:05:18 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:59216 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S237790AbhJYUCV (ORCPT ); Mon, 25 Oct 2021 16:02:21 -0400 Received: from mail-yb1-xb49.google.com (mail-yb1-xb49.google.com [IPv6:2607:f8b0:4864:20::b49]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 16C1EC0432F5 for ; Mon, 25 Oct 2021 12:25:30 -0700 (PDT) Received: by mail-yb1-xb49.google.com with SMTP id z2-20020a254c02000000b005b68ef4fe24so18917247yba.11 for ; Mon, 25 Oct 2021 12:25:30 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=date:in-reply-to:message-id:mime-version:references:subject:from:cc; bh=zTwkaja/2JWSasZvIIm+cYcdHeXbXK6HRxptMu06TGw=; b=es0PIzo5QRe/gNbKP/B9WOwdGUjl3l9nyAvVGwTkzf/E0vFbRT43d5JLLX+0lCXYrj IuV1P8y4Zn0oRJfSwiaEGxa/UiAZz8da4/F8a5EHSpnbwDi+6C0PA/Lnnyz29lNBruSh BEGlpTgEM0+76Ouvm072oqAOIMtPb9D7p+/gImv+17JnzOioFBRX0d4dWauFZr5olSzv UhAfDvqixN7XGXm2cyzJHyg8HKcwCorh+H8p1i15l4RoGG8g0z94TwyT/Skl05dI3NSQ h9ac+zm++4zhUn2z0dbq4Z7N72HhEcHJQV0eDJka8cAB4CrPam/tgATotv94uLIo0ENt s3lA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:in-reply-to:message-id:mime-version :references:subject:from:cc; bh=zTwkaja/2JWSasZvIIm+cYcdHeXbXK6HRxptMu06TGw=; b=UpniewnyVsqOIdGZJz8ww5qwxYy9GpfTgY5C1gAGnq2CjpN7wPrvsLq+xtws878KzQ 0k/hgfnoU10JvqNoyaIBRkdcAALdwNDI1Vvv36T1YfvOUtjZqAnypxuUnnpsoKf/YdIf oRby3WeCauhCUJqivOrCDZjXxHvVvv/jhR6/2TOHf4qjmRa5vFB5ABcyCyMf0duDUPS7 87qXdTM3T5VAMyX9EIF7Aa3pk3noUU+faY9HDk8gpWAESajAFvtO6b1mkRyHa4RvtUgQ p8iqj3QeYpMIM2Uf0sJiVmzpkM31dMNjJ+LGJROpe2J2hqa7aUyKyZEgx4wmw6yRekBI kHnA== X-Gm-Message-State: AOAM530L87kHMPejcCgg8Y7ReRLEkAwHu7RS0uvGxKO8zaU5witofZCS 37KoHMi+sBIIOQIeGHW8zKY2n1sG3DRy8o+1nQ== X-Google-Smtp-Source: ABdhPJynb9pf2F0DjhOaAmMkHMZpmL6519gTydu0ZsQZU9+8HWvfo2CFsZGDTuGlxXOTmpzBjpkKiYIVVkTGlU3T6g== X-Received: from kaleshsingh.mtv.corp.google.com ([2620:15c:211:200:b783:5702:523e:d435]) (user=kaleshsingh job=sendgmr) by 2002:a25:400f:: with SMTP id n15mr21239484yba.497.1635189929323; Mon, 25 Oct 2021 12:25:29 -0700 (PDT) Date: Mon, 25 Oct 2021 12:23:17 -0700 In-Reply-To: <20211025192330.2992076-1-kaleshsingh@google.com> Message-Id: <20211025192330.2992076-7-kaleshsingh@google.com> Mime-Version: 1.0 References: <20211025192330.2992076-1-kaleshsingh@google.com> X-Mailer: git-send-email 2.33.0.1079.g6e70778dc9-goog Subject: [PATCH v3 6/8] tracing/histogram: Optimize division by a power of 2 From: Kalesh Singh Cc: surenb@google.com, hridya@google.com, namhyung@kernel.org, kernel-team@android.com, Kalesh Singh , Steven Rostedt , 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 To: unlisted-recipients:; (no To-header on input) Precedence: bulk List-ID: X-Mailing-List: linux-kselftest@vger.kernel.org The division is a slow operation. If the divisor is a power of 2, use a shift instead. Results were obtained using Android's version of perf (simpleperf[1]) as described below: 1. hist_field_div() is modified to call 2 test functions: test_hist_field_div_[not]_optimized(); passing them the same args. Use noinline and volatile to ensure these are not optimized out by the compiler. 2. Create a hist event trigger that uses division: events/kmem/rss_stat$ echo 'hist:keys=common_pid:x=size/' >> trigger events/kmem/rss_stat$ echo 'hist:keys=common_pid:vals=$x' >> trigger 3. Run Android's lmkd_test[2] to generate rss_stat events, and record CPU samples with Android's simpleperf: simpleperf record -a --exclude-perf --post-unwind=yes -m 16384 -g -f 2000 -o perf.data == Results == Divisor is a power of 2 (divisor == 32): test_hist_field_div_not_optimized | 8,717,091 cpu-cycles test_hist_field_div_optimized | 1,643,137 cpu-cycles If the divisor is a power of 2, the optimized version is ~5.3x faster. Divisor is not a power of 2 (divisor == 33): test_hist_field_div_not_optimized | 4,444,324 cpu-cycles test_hist_field_div_optimized | 5,497,958 cpu-cycles If the divisor is not a power of 2, as expected, the optimized version is slightly slower (~24% slower). [1] https://android.googlesource.com/platform/system/extras/+/master/simpleperf/doc/README.md [2] https://cs.android.com/android/platform/superproject/+/master:system/memory/lmkd/tests/lmkd_test.cpp Signed-off-by: Kalesh Singh Suggested-by: Steven Rostedt --- kernel/trace/trace_events_hist.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c index db28bcf976f4..364cb3091789 100644 --- a/kernel/trace/trace_events_hist.c +++ b/kernel/trace/trace_events_hist.c @@ -304,6 +304,10 @@ static u64 hist_field_div(struct hist_field *hist_field, if (!val2) return -1; + /* Use shift if the divisor is a power of 2 */ + if (!(val2 & (val2 - 1))) + return val1 >> __ffs64(val2); + return div64_u64(val1, val2); } From patchwork Mon Oct 25 19:23:18 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kalesh Singh X-Patchwork-Id: 12582775 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 1ECB1C433F5 for ; Mon, 25 Oct 2021 20:04:14 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 03EBC60E96 for ; Mon, 25 Oct 2021 20:04:14 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S236708AbhJYUGd (ORCPT ); Mon, 25 Oct 2021 16:06:33 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:60566 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S240921AbhJYUEf (ORCPT ); Mon, 25 Oct 2021 16:04:35 -0400 Received: from mail-pl1-x649.google.com (mail-pl1-x649.google.com [IPv6:2607:f8b0:4864:20::649]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 75FFFC036D66 for ; Mon, 25 Oct 2021 12:25:48 -0700 (PDT) Received: by mail-pl1-x649.google.com with SMTP id u1-20020a170903124100b0013fd0e97269so4185347plh.10 for ; Mon, 25 Oct 2021 12:25:48 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=date:in-reply-to:message-id:mime-version:references:subject:from:cc; bh=0X4ZzEWqQCI4S+9dMmdUBiiRunDNYVnUP5vvvMfRtDs=; b=bi59lsDZVQms20hwFrTJJM3A/hTBiNa7tq1pLsKiZNL6niHp8iT9X1jIprUqUp56p4 kaEdhbjPBAuGwiry1ymNlKFWkZP2Cp9AayRKUjjLVHK8hevBvbkMlMJT/4cEzSnsIIs8 uPmcHafy1IQXWqKXBrB8w9AZwKdiTrh4Ff0H9vjxkWqAI0qOYF8giDkLgh+s+ZozRO3B wyecMadGlWFxZ26lpG2oAhPZPc/ooNC2pOHiaShcSLTCC2g5clVah9MT1LDQ7trjUMrP F+I7gI3hUD1FBJIeCpmMiXv5Lq2ON1AkLjfH4qZYlZhmYlbtyBu6ylJkRXCUOs9DAWcO NlCg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:in-reply-to:message-id:mime-version :references:subject:from:cc; bh=0X4ZzEWqQCI4S+9dMmdUBiiRunDNYVnUP5vvvMfRtDs=; b=Si9evj6vxDw7YIEmH/7B7tUFyzeJ81hYvyRwrg0whoqV3RsaYpR9TXT3UZpIOCajpu /oWQ8h8vzHUiIFebgilKpKMdkUxScO19WVWmdmeSsFVvJYTe2Y++hEPuAjCJ3S7U64Hd Lr09y76TKYeHJSlqlpiQU2V8B57wTbQNnCxp5KKFXbl2sHZ/cLIRWTuHN3rb0UeCn59d DH7Z5o+yPAhYQacGQUVEcYySbJGhHcDQmZ82lU7H0P5AahIG3C8kKVCmuid1agCfvzIc NQMEpKbYgUmSnAtnXZ7Gim+tOX0fmQJmo8imRINjyreDFu2JHPJfdO1POVgGIoIvlqZd clzg== X-Gm-Message-State: AOAM530gattarJEfJYnMNnlJaHhWrtNWvolnnJ5on1cKAf5vwetP9irg egPyFrIwLemKrNHSKxGiJwCnhVLTYRTDV2+XvQ== X-Google-Smtp-Source: ABdhPJzXXPfI/4dfShXeTCziC9Z90/UVK1Pqw4rSjtJJCZfTyDZT4yuAF4sUhZ1BdorC8BCekgQgTIrfpmbx+Kceog== X-Received: from kaleshsingh.mtv.corp.google.com ([2620:15c:211:200:b783:5702:523e:d435]) (user=kaleshsingh job=sendgmr) by 2002:a17:90b:3ecc:: with SMTP id rm12mr23043839pjb.48.1635189947934; Mon, 25 Oct 2021 12:25:47 -0700 (PDT) Date: Mon, 25 Oct 2021 12:23:18 -0700 In-Reply-To: <20211025192330.2992076-1-kaleshsingh@google.com> Message-Id: <20211025192330.2992076-8-kaleshsingh@google.com> Mime-Version: 1.0 References: <20211025192330.2992076-1-kaleshsingh@google.com> X-Mailer: git-send-email 2.33.0.1079.g6e70778dc9-goog Subject: [PATCH v3 7/8] tracing/selftests: Add tests for hist trigger expression parsing From: Kalesh Singh Cc: surenb@google.com, hridya@google.com, namhyung@kernel.org, kernel-team@android.com, Kalesh Singh , Jonathan Corbet , Steven Rostedt , Ingo Molnar , Shuah Khan , Masami Hiramatsu , Tom Zanussi , linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org To: unlisted-recipients:; (no To-header on input) Precedence: bulk List-ID: X-Mailing-List: linux-kselftest@vger.kernel.org Add tests for the parsing of hist trigger expressions; and to validate expression evaluation. Signed-off-by: Kalesh Singh Reviewed-by: Namhyung Kim Change-Id: Id3871697eb2d7537c8185494f5c1d347077798bc --- Changes in v3: - Remove .sym-offset error check tests Changes in v2: - Add Namhyung's Reviewed-by - Update comment to clarify err_pos in "Too many subexpressions" test .../testing/selftests/ftrace/test.d/functions | 4 +- .../trigger/trigger-hist-expressions.tc | 72 +++++++++++++++++++ 2 files changed, 74 insertions(+), 2 deletions(-) create mode 100644 tools/testing/selftests/ftrace/test.d/trigger/trigger-hist-expressions.tc diff --git a/tools/testing/selftests/ftrace/test.d/functions b/tools/testing/selftests/ftrace/test.d/functions index 000fd05e84b1..1855a63559ad 100644 --- a/tools/testing/selftests/ftrace/test.d/functions +++ b/tools/testing/selftests/ftrace/test.d/functions @@ -16,13 +16,13 @@ reset_tracer() { # reset the current tracer reset_trigger_file() { # remove action triggers first - grep -H ':on[^:]*(' $@ | + grep -H ':on[^:]*(' $@ | tac | while read line; do cmd=`echo $line | cut -f2- -d: | cut -f1 -d"["` file=`echo $line | cut -f1 -d:` echo "!$cmd" >> $file done - grep -Hv ^# $@ | + grep -Hv ^# $@ | tac | while read line; do cmd=`echo $line | cut -f2- -d: | cut -f1 -d"["` file=`echo $line | cut -f1 -d:` diff --git a/tools/testing/selftests/ftrace/test.d/trigger/trigger-hist-expressions.tc b/tools/testing/selftests/ftrace/test.d/trigger/trigger-hist-expressions.tc new file mode 100644 index 000000000000..e715641c54d3 --- /dev/null +++ b/tools/testing/selftests/ftrace/test.d/trigger/trigger-hist-expressions.tc @@ -0,0 +1,72 @@ +#!/bin/sh +# SPDX-License-Identifier: GPL-2.0 +# description: event trigger - test histogram expression parsing +# requires: set_event events/sched/sched_process_fork/trigger events/sched/sched_process_fork/hist error_log + + +fail() { #msg + echo $1 + exit_fail +} + +get_hist_var() { #var_name hist_path + hist_output=`grep -m1 "$1: " $2` + hitcount=`echo $hist_output | awk '{ for (i=1; i<=NF; ++i) { if ($i ~ "hitcount:") print $(i+1)} }'` + var_sum=`echo $hist_output | awk '{ for (i=1; i<=NF; ++i) { if ($i ~ "'$1':") print $(i+1)} }'` + var_val=$(( var_sum / hitcount )) + echo $var_val +} + +test_hist_expr() { # test_name expression expected_val + reset_trigger + + echo "Test hist trigger expressions - $1" + + echo "hist:keys=common_pid:x=$2" > events/sched/sched_process_fork/trigger + echo 'hist:keys=common_pid:vals=$x' >> events/sched/sched_process_fork/trigger + for i in `seq 1 10` ; do ( echo "forked" > /dev/null); done + + actual=`get_hist_var x events/sched/sched_process_fork/hist` + + if [ $actual != $3 ]; then + fail "Failed hist trigger expression evaluation: Expression: $2 Expected: $3, Actual: $actual" + fi + + reset_trigger +} + +check_error() { # test_name command-with-error-pos-by-^ + reset_trigger + + echo "Test hist trigger expressions - $1" + ftrace_errlog_check 'hist:sched:sched_process_fork' "$2" 'events/sched/sched_process_fork/trigger' + + reset_trigger +} + +test_hist_expr "Variable assignment" "123" "123" + +test_hist_expr "Subtraction not associative" "16-8-4-2" "2" + +test_hist_expr "Division not associative" "64/8/4/2" "1" + +test_hist_expr "Same precedence operators (+,-) evaluated left to right" "16-8+4+2" "14" + +test_hist_expr "Same precedence operators (*,/) evaluated left to right" "4*3/2*2" "12" + +test_hist_expr "Multiplication evaluated before addition/subtraction" "4+3*2-2" "8" + +test_hist_expr "Division evaluated before addition/subtraction" "4+6/2-2" "5" + +# Division by zero returns -1 +test_hist_expr "Handles division by zero" "3/0" "-1" + +# err pos for "too many subexpressions" is dependent on where +# the last subexpression was detected. This can vary depending +# on how the expression tree was generated. +check_error "Too many subexpressions" 'hist:keys=common_pid:x=32+^10*3/20-4' +check_error "Too many subexpressions" 'hist:keys=common_pid:x=^1+2+3+4+5' + +check_error "Unary minus not supported in subexpression" 'hist:keys=common_pid:x=-(^1)+2' + +exit 0 From patchwork Mon Oct 25 19:23:19 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kalesh Singh X-Patchwork-Id: 12582777 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 4E3F4C433F5 for ; Mon, 25 Oct 2021 20:05:32 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 2965360E0B for ; Mon, 25 Oct 2021 20:05:32 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S236954AbhJYUHw (ORCPT ); Mon, 25 Oct 2021 16:07:52 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:60426 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S234944AbhJYUGD (ORCPT ); Mon, 25 Oct 2021 16:06:03 -0400 Received: from mail-qv1-xf49.google.com (mail-qv1-xf49.google.com [IPv6:2607:f8b0:4864:20::f49]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 29937C036D6F for ; Mon, 25 Oct 2021 12:26:04 -0700 (PDT) Received: by mail-qv1-xf49.google.com with SMTP id el19-20020ad459d3000000b00384a5ef8979so11677562qvb.18 for ; Mon, 25 Oct 2021 12:26:04 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=date:in-reply-to:message-id:mime-version:references:subject:from:cc; bh=kE3Nv1lKwT793xJwq+iC3RoCHqpYdQS3Tip3semgyOw=; b=gbn9eg9008NdXKZ1/pxplWbq1W+HOEpi3W3HkUpPW4ebJu/REMlQr1QF8rl6mQIeob euAGzvS+dOzo0OLG8Dvw/ls/OC6sl5N7rY/bFIaEZ6MMC/DFS3XjRDLrySKoPcl0Onwf TR3XD9K2yr65MzjufGS4Maq/H7ORSIxQQeGN/v+vTFrEH7F7Uo1xD6t4IqUj10V2a6GA RwWq00Ic7WNkanKG1SVkDRvdAP398xy1awfqdfjX+8zrnFqsgafTBYlGR+ZD+rxq8olq Qlsg3CtIduhiPG9cLfEDQ56Rujasnsz4EppRxtlTl7M8mxnvBNOn73lnj4mIxM1QQ5/s TgbA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:in-reply-to:message-id:mime-version :references:subject:from:cc; bh=kE3Nv1lKwT793xJwq+iC3RoCHqpYdQS3Tip3semgyOw=; b=1H48bKbWB2QbCWPgwYGNGpK/d3wzVxBnMSFj9lMKQwQ9My366PucnpQx/yAVpT7X3c rlwOp+CEKb6PEdB2u/lxLzynEpvubLz5EBqMXdxOQ+F5wH3P3o9HxvCE6pry9aNJPI71 /pGwalc0de/Gu1g4E9uHoJSepJZlTbGja/bLTsLwtx0xI2CE4GyJdjrcAF/cwed+7mSH UurZy1iI2DAahuxrkPjqFErpDwzroyMhFgcvrgEhNHZEXu1oZL793k5cFQCofTBLikqW GcEYAe0M/ndm0ZqWuZRzukXkv95i8Abt4TCOMCmrEkkhUA5UbCelAnnJp97djpv9wIrW 5L7Q== X-Gm-Message-State: AOAM531zQN3AnsHfX9bPWS4irkKnZWltLEtXBdkst01R4Z8SMM/Vby9P bU8U6L8xmRO8xT1p2ADM43pTBNf/vXTxnL1RKg== X-Google-Smtp-Source: ABdhPJz0CM9pT2MDiX5onT9xQHkFssm++CsGBJiIKiL0pdTsJ/7A8JCL2uHFhTnN92yeJZ01BIcWoG5+gyc3y2cP1Q== X-Received: from kaleshsingh.mtv.corp.google.com ([2620:15c:211:200:b783:5702:523e:d435]) (user=kaleshsingh job=sendgmr) by 2002:a05:622a:1a0b:: with SMTP id f11mr19780649qtb.133.1635189963298; Mon, 25 Oct 2021 12:26:03 -0700 (PDT) Date: Mon, 25 Oct 2021 12:23:19 -0700 In-Reply-To: <20211025192330.2992076-1-kaleshsingh@google.com> Message-Id: <20211025192330.2992076-9-kaleshsingh@google.com> Mime-Version: 1.0 References: <20211025192330.2992076-1-kaleshsingh@google.com> X-Mailer: git-send-email 2.33.0.1079.g6e70778dc9-goog Subject: [PATCH v3 8/8] tracing/histogram: Document expression arithmetic and constants From: Kalesh Singh Cc: surenb@google.com, hridya@google.com, namhyung@kernel.org, kernel-team@android.com, Kalesh Singh , Jonathan Corbet , Steven Rostedt , Ingo Molnar , Shuah Khan , Masami Hiramatsu , Tom Zanussi , linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org To: unlisted-recipients:; (no To-header on input) Precedence: bulk List-ID: X-Mailing-List: linux-kselftest@vger.kernel.org Histogram expressions now support division, and multiplication in addition to the already supported subtraction and addition operators. Numeric constants can also be used in a hist trigger expressions or assigned to a variable and used by refernce in an expression. Signed-off-by: Kalesh Singh Reviewed-by: Namhyung Kim --- Changes in v2: - Add Namhyung's Reviewed-by Documentation/trace/histogram.rst | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/Documentation/trace/histogram.rst b/Documentation/trace/histogram.rst index 533415644c54..e12699abaee8 100644 --- a/Documentation/trace/histogram.rst +++ b/Documentation/trace/histogram.rst @@ -1763,6 +1763,20 @@ using the same key and variable from yet another event:: # echo 'hist:key=pid:wakeupswitch_lat=$wakeup_lat+$switchtime_lat ...' >> event3/trigger +Expressions support the use of addition, subtraction, multiplication and +division operators (+-*/). + +Note that division by zero always returns -1. + +Numeric constants can also be used directly in an expression:: + + # echo 'hist:keys=next_pid:timestamp_secs=common_timestamp/1000000 ...' >> event/trigger + +or assigned to a variable and referenced in a subsequent expression:: + + # echo 'hist:keys=next_pid:us_per_sec=1000000 ...' >> event/trigger + # echo 'hist:keys=next_pid:timestamp_secs=common_timestamp/$us_per_sec ...' >> event/trigger + 2.2.2 Synthetic Events ----------------------