Re: [PATCH 16/40] perf tools: Reducing arguments of hist_entry_iter__add()

[Namhyung Kim <namhyung@kernel.org>]

On Mon, May 18, 2015 at 09:55:41AM -0300, Arnaldo Carvalho de Melo wrote:
> Em Mon, May 18, 2015 at 09:30:31AM +0900, Namhyung Kim escreveu:
> > The evsel and sample arguments are to set iter for later use.  As it
> > also receives an iter as another argument, just set them before
> > calling the function.
> 
> This looks like can be applied sooner, tried but it didn't apply :-\

I'll send it as a separate patch.

Thanks,
Namhyung


> 
> - Arnaldo
>  
> > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> > ---
> >  tools/perf/builtin-report.c       | 9 +++++----
> >  tools/perf/builtin-top.c          | 7 ++++---
> >  tools/perf/tests/hists_cumulate.c | 6 ++++--
> >  tools/perf/tests/hists_filter.c   | 4 +++-
> >  tools/perf/tests/hists_output.c   | 6 ++++--
> >  tools/perf/util/hist.c            | 8 ++------
> >  tools/perf/util/hist.h            | 1 -
> >  7 files changed, 22 insertions(+), 19 deletions(-)
> > 
> > diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
> > index fee770935eab..decd9e8584b5 100644
> > --- a/tools/perf/builtin-report.c
> > +++ b/tools/perf/builtin-report.c
> > @@ -139,8 +139,10 @@ static int process_sample_event(struct perf_tool *tool,
> >  	struct report *rep = container_of(tool, struct report, tool);
> >  	struct addr_location al;
> >  	struct hist_entry_iter iter = {
> > -		.hide_unresolved = rep->hide_unresolved,
> > -		.add_entry_cb = hist_iter__report_callback,
> > +		.evsel 			= evsel,
> > +		.sample 		= sample,
> > +		.hide_unresolved 	= rep->hide_unresolved,
> > +		.add_entry_cb 		= hist_iter__report_callback,
> >  	};
> >  	int ret = 0;
> >  
> > @@ -168,8 +170,7 @@ static int process_sample_event(struct perf_tool *tool,
> >  	if (al.map != NULL)
> >  		al.map->dso->hit = 1;
> >  
> > -	ret = hist_entry_iter__add(&iter, &al, evsel, sample, rep->max_stack,
> > -				   rep);
> > +	ret = hist_entry_iter__add(&iter, &al, rep->max_stack, rep);
> >  	if (ret < 0)
> >  		pr_debug("problem adding hist entry, skipping event\n");
> >  out_put:
> > diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
> > index a19351728f0f..6b987424d015 100644
> > --- a/tools/perf/builtin-top.c
> > +++ b/tools/perf/builtin-top.c
> > @@ -775,7 +775,9 @@ static void perf_event__process_sample(struct perf_tool *tool,
> >  	if (al.sym == NULL || !al.sym->ignore) {
> >  		struct hists *hists = evsel__hists(evsel);
> >  		struct hist_entry_iter iter = {
> > -			.add_entry_cb = hist_iter__top_callback,
> > +			.evsel		= evsel,
> > +			.sample 	= sample,
> > +			.add_entry_cb 	= hist_iter__top_callback,
> >  		};
> >  
> >  		if (symbol_conf.cumulate_callchain)
> > @@ -785,8 +787,7 @@ static void perf_event__process_sample(struct perf_tool *tool,
> >  
> >  		pthread_mutex_lock(&hists->lock);
> >  
> > -		err = hist_entry_iter__add(&iter, &al, evsel, sample,
> > -					   top->max_stack, top);
> > +		err = hist_entry_iter__add(&iter, &al, top->max_stack, top);
> >  		if (err < 0)
> >  			pr_err("Problem incrementing symbol period, skipping event\n");
> >  
> > diff --git a/tools/perf/tests/hists_cumulate.c b/tools/perf/tests/hists_cumulate.c
> > index 620f626e5b35..7d82c8be5e36 100644
> > --- a/tools/perf/tests/hists_cumulate.c
> > +++ b/tools/perf/tests/hists_cumulate.c
> > @@ -87,6 +87,8 @@ static int add_hist_entries(struct hists *hists, struct machine *machine)
> >  			},
> >  		};
> >  		struct hist_entry_iter iter = {
> > +			.evsel = evsel,
> > +			.sample	= &sample,
> >  			.hide_unresolved = false,
> >  		};
> >  
> > @@ -104,8 +106,8 @@ static int add_hist_entries(struct hists *hists, struct machine *machine)
> >  						  &sample) < 0)
> >  			goto out;
> >  
> > -		if (hist_entry_iter__add(&iter, &al, evsel, &sample,
> > -					 PERF_MAX_STACK_DEPTH, NULL) < 0) {
> > +		if (hist_entry_iter__add(&iter, &al, PERF_MAX_STACK_DEPTH,
> > +					 NULL) < 0) {
> >  			addr_location__put(&al);
> >  			goto out;
> >  		}
> > diff --git a/tools/perf/tests/hists_filter.c b/tools/perf/tests/hists_filter.c
> > index 82e1ee52e024..ce48775e6ada 100644
> > --- a/tools/perf/tests/hists_filter.c
> > +++ b/tools/perf/tests/hists_filter.c
> > @@ -63,6 +63,8 @@ static int add_hist_entries(struct perf_evlist *evlist,
> >  				},
> >  			};
> >  			struct hist_entry_iter iter = {
> > +				.evsel = evsel,
> > +				.sample = &sample,
> >  				.ops = &hist_iter_normal,
> >  				.hide_unresolved = false,
> >  			};
> > @@ -81,7 +83,7 @@ static int add_hist_entries(struct perf_evlist *evlist,
> >  							  &sample) < 0)
> >  				goto out;
> >  
> > -			if (hist_entry_iter__add(&iter, &al, evsel, &sample,
> > +			if (hist_entry_iter__add(&iter, &al,
> >  						 PERF_MAX_STACK_DEPTH, NULL) < 0) {
> >  				addr_location__put(&al);
> >  				goto out;
> > diff --git a/tools/perf/tests/hists_output.c b/tools/perf/tests/hists_output.c
> > index fd7ec4f9aeb4..adbebc852cc8 100644
> > --- a/tools/perf/tests/hists_output.c
> > +++ b/tools/perf/tests/hists_output.c
> > @@ -57,6 +57,8 @@ static int add_hist_entries(struct hists *hists, struct machine *machine)
> >  			},
> >  		};
> >  		struct hist_entry_iter iter = {
> > +			.evsel = evsel,
> > +			.sample = &sample,
> >  			.ops = &hist_iter_normal,
> >  			.hide_unresolved = false,
> >  		};
> > @@ -70,8 +72,8 @@ static int add_hist_entries(struct hists *hists, struct machine *machine)
> >  						  &sample) < 0)
> >  			goto out;
> >  
> > -		if (hist_entry_iter__add(&iter, &al, evsel, &sample,
> > -					 PERF_MAX_STACK_DEPTH, NULL) < 0) {
> > +		if (hist_entry_iter__add(&iter, &al, PERF_MAX_STACK_DEPTH,
> > +					 NULL) < 0) {
> >  			addr_location__put(&al);
> >  			goto out;
> >  		}
> > diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
> > index f13993e53e4e..b492968913e1 100644
> > --- a/tools/perf/util/hist.c
> > +++ b/tools/perf/util/hist.c
> > @@ -852,19 +852,15 @@ const struct hist_iter_ops hist_iter_cumulative = {
> >  };
> >  
> >  int hist_entry_iter__add(struct hist_entry_iter *iter, struct addr_location *al,
> > -			 struct perf_evsel *evsel, struct perf_sample *sample,
> >  			 int max_stack_depth, void *arg)
> >  {
> >  	int err, err2;
> >  
> > -	err = sample__resolve_callchain(sample, &iter->parent, evsel, al,
> > -					max_stack_depth);
> > +	err = sample__resolve_callchain(iter->sample, &iter->parent,
> > +					iter->evsel, al, max_stack_depth);
> >  	if (err)
> >  		return err;
> >  
> > -	iter->evsel = evsel;
> > -	iter->sample = sample;
> > -
> >  	err = iter->ops->prepare_entry(iter, al);
> >  	if (err)
> >  		goto out;
> > diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
> > index 43ffe62e25d2..d47f6730f6a4 100644
> > --- a/tools/perf/util/hist.h
> > +++ b/tools/perf/util/hist.h
> > @@ -111,7 +111,6 @@ struct hist_entry *__hists__add_entry(struct hists *hists,
> >  				      u64 weight, u64 transaction,
> >  				      u64 timestamp, bool sample_self);
> >  int hist_entry_iter__add(struct hist_entry_iter *iter, struct addr_location *al,
> > -			 struct perf_evsel *evsel, struct perf_sample *sample,
> >  			 int max_stack_depth, void *arg);
> >  
> >  int64_t hist_entry__cmp(struct hist_entry *left, struct hist_entry *right);
> > -- 
> > 2.4.0
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/