On Tue,  5 Sep 2017 16:57:32 -0500
Tom Zanussi <tom.zanu...@linux.intel.com> wrote:
>  #define DEFINE_HIST_FIELD_FN(type)                                   \
>       static u64 hist_field_##type(struct hist_field *hist_field,     \
>                                    void *event,                       \
> @@ -148,6 +192,7 @@ enum hist_field_flags {
>       HIST_FIELD_FL_TIMESTAMP_USECS   = 2048,
>       HIST_FIELD_FL_VAR               = 4096,
>       HIST_FIELD_FL_VAR_ONLY          = 8192,
> +     HIST_FIELD_FL_EXPR              = 16384,

When numbers get this big, I'm a fan of:

        HIST_FIELD_FL_TIMESTAMP_USECS   = (1 << 11),
        HIST_FIELD_FL_VAR               = (1 << 12),
        HIST_FIELD_FL_VAR_ONLY          = (1 << 13),
        HIST_FIELD_FL_EXPR              = (1 << 14),

It makes it much easier to debug with printk()s and also know how many
bits you are actually using.

>  };
>  
>  struct var_defs {
> @@ -218,6 +263,8 @@ static const char *hist_field_name(struct hist_field 
> *field,
>               field_name = hist_field_name(field->operands[0], ++level);
>       else if (field->flags & HIST_FIELD_FL_TIMESTAMP)
>               field_name = "$common_timestamp";
> +     else if (field->flags & HIST_FIELD_FL_EXPR)
> +             field_name = field->name;
>  
>       if (field_name == NULL)
>               field_name = "";
> @@ -441,6 +488,115 @@ static void hist_trigger_elt_comm_init(struct 
> tracing_map_elt *elt)
>       .elt_init       = hist_trigger_elt_comm_init,
>  };
>  
> +static const char *get_hist_field_flags(struct hist_field *hist_field)
> +{
> +     const char *flags_str = NULL;
> +
> +     if (hist_field->flags & HIST_FIELD_FL_HEX)
> +             flags_str = "hex";
> +     else if (hist_field->flags & HIST_FIELD_FL_SYM)
> +             flags_str = "sym";
> +     else if (hist_field->flags & HIST_FIELD_FL_SYM_OFFSET)
> +             flags_str = "sym-offset";
> +     else if (hist_field->flags & HIST_FIELD_FL_EXECNAME)
> +             flags_str = "execname";
> +     else if (hist_field->flags & HIST_FIELD_FL_SYSCALL)
> +             flags_str = "syscall";
> +     else if (hist_field->flags & HIST_FIELD_FL_LOG2)
> +             flags_str = "log2";
> +     else if (hist_field->flags & HIST_FIELD_FL_TIMESTAMP_USECS)
> +             flags_str = "usecs";
> +
> +     return flags_str;
> +}

Also, can you make the moving of the code a separate patch before this
patch. It makes git blame and such nicer.

-- Steve

Reply via email to