Re: [PATCH v3 1/8] tracing: Add support for creating hist trigger variables from literal

On Mon, Oct 25, 2021 at 12:23 PM Kalesh Singh kaleshsingh@google.com wrote:

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 kaleshsingh@google.com Change-Id: I142121d28dc3475dfbc3a882e7b2368d833474eb

Ahh. Sorry, I forgot to remove these Change-Id tags, I'll resend another version.

---

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)
  

-- 2.33.0.1079.g6e70778dc9-goog