linux-kernel - Re: [PATCH] perf callchain: Remove unused callchain_branch

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Zwa0bfGAaBGTdD33@gallifrey>
Date: Wed, 9 Oct 2024 16:50:53 +0000
From: "Dr. David Alan Gilbert" <linux@...blig.org>
To: "Liang, Kan" <kan.liang@...ux.intel.com>
Cc: Namhyung Kim <namhyung@...nel.org>, peterz@...radead.org,
	mingo@...hat.com, acme@...nel.org, mark.rutland@....com,
	alexander.shishkin@...ux.intel.com, jolsa@...nel.org,
	irogers@...gle.com, adrian.hunter@...el.com,
	linux-perf-users@...r.kernel.org, linux-kernel@...r.kernel.org,
	Andi Kleen <ak@...ux.intel.com>,
	Thomas Falcon <thomas.falcon@...el.com>
Subject: Re: [PATCH] perf callchain: Remove unused callchain_branch_counts

* Liang, Kan (kan.liang@...ux.intel.com) wrote:
> 
> 
> On 2024-10-09 1:30 a.m., Namhyung Kim wrote:
> > On Sun, Oct 06, 2024 at 02:22:04AM +0100, linux@...blig.org wrote:
> >> From: "Dr. David Alan Gilbert" <linux@...blig.org>
> >>
> >> callchain_branch_counts() was added in 2016 by commit
> >> 3dd029ef9401 ("perf report: Calculate and return the branch flag counting")
> >> but unused.
> 
> It seems the original patchset was not well organized. This function is
> actually used by the last patch of the series.
> https://lore.kernel.org/lkml/1477876794-30749-7-git-send-email-yao.jin@linux.intel.com/
> 
> But for some reason, the last patch was not merged. I think Andi gave
> the Acked-by for the whole series. It might be just accidentally missed.

OK, that'll be the 4th time this deadcode series has found missing calls
to functions!

> I think the missed feature is still useful.
> We will redo the test and re-post it.

OK, great.

Dave

> Thanks,
> Kan
> 
> >>
> >> Remove it and it's helpers.
> > 
> > Kan, are you ok with this?
> > 
> > Thanks,
> > Namhyung
> > 
> >>
> >> Signed-off-by: Dr. David Alan Gilbert <linux@...blig.org>
> >> ---
> >>  tools/perf/util/callchain.c | 71 -------------------------------------
> >>  tools/perf/util/callchain.h |  4 ---
> >>  2 files changed, 75 deletions(-)
> >>
> >> diff --git a/tools/perf/util/callchain.c b/tools/perf/util/callchain.c
> >> index 0c7564747a14..11435b72afbe 100644
> >> --- a/tools/perf/util/callchain.c
> >> +++ b/tools/perf/util/callchain.c
> >> @@ -1266,77 +1266,6 @@ int callchain_node__fprintf_value(struct callchain_node *node,
> >>  	return 0;
> >>  }
> >>  
> >> -static void callchain_counts_value(struct callchain_node *node,
> >> -				   u64 *branch_count, u64 *predicted_count,
> >> -				   u64 *abort_count, u64 *cycles_count)
> >> -{
> >> -	struct callchain_list *clist;
> >> -
> >> -	list_for_each_entry(clist, &node->val, list) {
> >> -		if (branch_count)
> >> -			*branch_count += clist->branch_count;
> >> -
> >> -		if (predicted_count)
> >> -			*predicted_count += clist->predicted_count;
> >> -
> >> -		if (abort_count)
> >> -			*abort_count += clist->abort_count;
> >> -
> >> -		if (cycles_count)
> >> -			*cycles_count += clist->cycles_count;
> >> -	}
> >> -}
> >> -
> >> -static int callchain_node_branch_counts_cumul(struct callchain_node *node,
> >> -					      u64 *branch_count,
> >> -					      u64 *predicted_count,
> >> -					      u64 *abort_count,
> >> -					      u64 *cycles_count)
> >> -{
> >> -	struct callchain_node *child;
> >> -	struct rb_node *n;
> >> -
> >> -	n = rb_first(&node->rb_root_in);
> >> -	while (n) {
> >> -		child = rb_entry(n, struct callchain_node, rb_node_in);
> >> -		n = rb_next(n);
> >> -
> >> -		callchain_node_branch_counts_cumul(child, branch_count,
> >> -						   predicted_count,
> >> -						   abort_count,
> >> -						   cycles_count);
> >> -
> >> -		callchain_counts_value(child, branch_count,
> >> -				       predicted_count, abort_count,
> >> -				       cycles_count);
> >> -	}
> >> -
> >> -	return 0;
> >> -}
> >> -
> >> -int callchain_branch_counts(struct callchain_root *root,
> >> -			    u64 *branch_count, u64 *predicted_count,
> >> -			    u64 *abort_count, u64 *cycles_count)
> >> -{
> >> -	if (branch_count)
> >> -		*branch_count = 0;
> >> -
> >> -	if (predicted_count)
> >> -		*predicted_count = 0;
> >> -
> >> -	if (abort_count)
> >> -		*abort_count = 0;
> >> -
> >> -	if (cycles_count)
> >> -		*cycles_count = 0;
> >> -
> >> -	return callchain_node_branch_counts_cumul(&root->node,
> >> -						  branch_count,
> >> -						  predicted_count,
> >> -						  abort_count,
> >> -						  cycles_count);
> >> -}
> >> -
> >>  static int count_pri64_printf(int idx, const char *str, u64 value, char *bf, int bfsize)
> >>  {
> >>  	return scnprintf(bf, bfsize, "%s%s:%" PRId64 "", (idx) ? " " : " (", str, value);
> >> diff --git a/tools/perf/util/callchain.h b/tools/perf/util/callchain.h
> >> index 86ed9e4d04f9..d7741fa9e9de 100644
> >> --- a/tools/perf/util/callchain.h
> >> +++ b/tools/perf/util/callchain.h
> >> @@ -298,10 +298,6 @@ void free_callchain(struct callchain_root *root);
> >>  void decay_callchain(struct callchain_root *root);
> >>  int callchain_node__make_parent_list(struct callchain_node *node);
> >>  
> >> -int callchain_branch_counts(struct callchain_root *root,
> >> -			    u64 *branch_count, u64 *predicted_count,
> >> -			    u64 *abort_count, u64 *cycles_count);
> >> -
> >>  void callchain_param_setup(u64 sample_type, const char *arch);
> >>  
> >>  bool callchain_cnode_matched(struct callchain_node *base_cnode,
> >> -- 
> >> 2.46.2
> >>
> > 
> 
-- 
 -----Open up your eyes, open up your mind, open up your code -------   
/ Dr. David Alan Gilbert    |       Running GNU/Linux       | Happy  \ 
\        dave @ treblig.org |                               | In Hex /
 \ _________________________|_____ http://www.treblig.org   |_______/