Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755531AbdIGRBh (ORCPT ); Thu, 7 Sep 2017 13:01:37 -0400 Received: from mga11.intel.com ([192.55.52.93]:42672 "EHLO mga11.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755449AbdIGRBg (ORCPT ); Thu, 7 Sep 2017 13:01:36 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.42,359,1500966000"; d="scan'208";a="1216006095" Message-ID: <1504803677.13310.9.camel@tzanussi-mobl.amr.corp.intel.com> Subject: Re: [PATCH v2 20/40] tracing: Add simple expression support to hist triggers From: Tom Zanussi To: Steven Rostedt Cc: tglx@linutronix.de, mhiramat@kernel.org, namhyung@kernel.org, vedang.patel@intel.com, bigeasy@linutronix.de, joel.opensrc@gmail.com, joelaf@google.com, mathieu.desnoyers@efficios.com, baohong.liu@intel.com, linux-kernel@vger.kernel.org, linux-rt-users@vger.kernel.org Date: Thu, 07 Sep 2017 12:01:17 -0500 In-Reply-To: <20170907124605.01f0025a@gandalf.local.home> References: <20170907124605.01f0025a@gandalf.local.home> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.10.4 (3.10.4-4.fc20) Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2320 Lines: 69 On Thu, 2017-09-07 at 12:46 -0400, Steven Rostedt wrote: > On Tue, 5 Sep 2017 16:57:32 -0500 > Tom Zanussi 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. > Sure, will do as well as the above.. Tom