[PATCH v5 1/6] tracing: Allow multiple hitcount values in histograms

Masami Hiramatsu (Google) posted 6 patches 3 years, 7 months ago
There is a newer version of this series
[PATCH v5 1/6] tracing: Allow multiple hitcount values in histograms
Posted by Masami Hiramatsu (Google) 3 years, 7 months ago
From: Tom Zanussi <zanussi@kernel.org>

The hitcount is treated specially in the histograms - since it's
always expected to be there regardless of whether the user specified
anything or not, it's always added as the first histogram value.

Currently the code doesn't allow it to be added more than once as a
value, which is inconsistent with all the other possible values.  It
would seem to be a pointless thing to want to do, but other features
being added such as percent and graph modifiers don't work properly
with the current hitcount restrictions.

Fix this by allowing multiple hitcounts to be added.

Signed-off-by: Tom Zanussi <zanussi@kernel.org>

---
 Changes in v4:
  - Initialize n_hitcount.
---
 kernel/trace/trace_events_hist.c |   12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index 92fbd72b6408..e80a41e380bb 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -1356,6 +1356,8 @@ static const char *hist_field_name(struct hist_field *field,
 			field_name = field->name;
 	} else if (field->flags & HIST_FIELD_FL_TIMESTAMP)
 		field_name = "common_timestamp";
+	else if (field->flags & HIST_FIELD_FL_HITCOUNT)
+		field_name = "hitcount";
 
 	if (field_name == NULL)
 		field_name = "";
@@ -2328,6 +2330,8 @@ parse_field(struct hist_trigger_data *hist_data, struct trace_event_file *file,
 			hist_data->attrs->ts_in_usecs = true;
 	} else if (strcmp(field_name, "common_cpu") == 0)
 		*flags |= HIST_FIELD_FL_CPU;
+	else if (strcmp(field_name, "hitcount") == 0)
+		*flags |= HIST_FIELD_FL_HITCOUNT;
 	else {
 		field = trace_find_event_field(file->event_call, field_name);
 		if (!field || !field->size) {
@@ -4328,8 +4332,8 @@ static int create_var_field(struct hist_trigger_data *hist_data,
 static int create_val_fields(struct hist_trigger_data *hist_data,
 			     struct trace_event_file *file)
 {
+	unsigned int i, j = 1, n_hitcount = 0;
 	char *fields_str, *field_str;
-	unsigned int i, j = 1;
 	int ret;
 
 	ret = create_hitcount_val(hist_data);
@@ -4346,8 +4350,10 @@ static int create_val_fields(struct hist_trigger_data *hist_data,
 		if (!field_str)
 			break;
 
-		if (strcmp(field_str, "hitcount") == 0)
-			continue;
+		if (strcmp(field_str, "hitcount") == 0) {
+			if (!n_hitcount++)
+				continue;
+		}
 
 		ret = create_val_field(hist_data, j++, file, field_str);
 		if (ret)
Re: [PATCH v5 1/6] tracing: Allow multiple hitcount values in histograms
Posted by Steven Rostedt 3 years, 7 months ago
On Sun,  4 Sep 2022 13:12:20 +0900
"Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote:

> From: Tom Zanussi <zanussi@kernel.org>
> 
> The hitcount is treated specially in the histograms - since it's
> always expected to be there regardless of whether the user specified
> anything or not, it's always added as the first histogram value.
> 
> Currently the code doesn't allow it to be added more than once as a
> value, which is inconsistent with all the other possible values.  It
> would seem to be a pointless thing to want to do, but other features
> being added such as percent and graph modifiers don't work properly
> with the current hitcount restrictions.
> 
> Fix this by allowing multiple hitcounts to be added.
> 
> Signed-off-by: Tom Zanussi <zanussi@kernel.org>

Hi Masami,

When posting patches from other people, you still need to add your
Signed-off-by, as that denotes the people who processed the patch before it
made it into git.

-- Steve
Re: [PATCH v5 1/6] tracing: Allow multiple hitcount values in histograms
Posted by Masami Hiramatsu (Google) 3 years, 7 months ago
On Tue, 6 Sep 2022 22:03:46 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> On Sun,  4 Sep 2022 13:12:20 +0900
> "Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote:
> 
> > From: Tom Zanussi <zanussi@kernel.org>
> > 
> > The hitcount is treated specially in the histograms - since it's
> > always expected to be there regardless of whether the user specified
> > anything or not, it's always added as the first histogram value.
> > 
> > Currently the code doesn't allow it to be added more than once as a
> > value, which is inconsistent with all the other possible values.  It
> > would seem to be a pointless thing to want to do, but other features
> > being added such as percent and graph modifiers don't work properly
> > with the current hitcount restrictions.
> > 
> > Fix this by allowing multiple hitcounts to be added.
> > 
> > Signed-off-by: Tom Zanussi <zanussi@kernel.org>
> 
> Hi Masami,
> 
> When posting patches from other people, you still need to add your
> Signed-off-by, as that denotes the people who processed the patch before it
> made it into git.

Oops, I got it.

Please add my signed-off-by;

Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>

Thank you,

> 
> -- Steve


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>