public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: "Dr. David Alan Gilbert" <linux@treblig.org>
To: "Liang, Kan" <kan.liang@linux.intel.com>
Cc: Namhyung Kim <namhyung@kernel.org>,
	peterz@infradead.org, mingo@redhat.com, acme@kernel.org,
	mark.rutland@arm.com, alexander.shishkin@linux.intel.com,
	jolsa@kernel.org, irogers@google.com, adrian.hunter@intel.com,
	linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org,
	Andi Kleen <ak@linux.intel.com>,
	Thomas Falcon <thomas.falcon@intel.com>
Subject: Re: [PATCH] perf callchain: Remove unused callchain_branch_counts
Date: Wed, 9 Oct 2024 16:50:53 +0000	[thread overview]
Message-ID: <Zwa0bfGAaBGTdD33@gallifrey> (raw)
In-Reply-To: <ba80cbe4-877a-4b5f-a5d6-de16fc1d2d4d@linux.intel.com>

* Liang, Kan (kan.liang@linux.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@treblig.org wrote:
> >> From: "Dr. David Alan Gilbert" <linux@treblig.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@treblig.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   |_______/

  reply	other threads:[~2024-10-09 16:51 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-10-06  1:22 [PATCH] perf callchain: Remove unused callchain_branch_counts linux
2024-10-09  5:30 ` Namhyung Kim
2024-10-09 15:27   ` Liang, Kan
2024-10-09 16:50     ` Dr. David Alan Gilbert [this message]
2024-10-10  0:34       ` Namhyung Kim
2024-10-10  0:33     ` Namhyung Kim
2024-10-10 18:40       ` [PATCH] perf report: Display columns Predicted/Abort/Cycles in --branch-history Thomas Falcon
2024-10-14 16:59         ` Namhyung Kim

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Zwa0bfGAaBGTdD33@gallifrey \
    --to=linux@treblig.org \
    --cc=acme@kernel.org \
    --cc=adrian.hunter@intel.com \
    --cc=ak@linux.intel.com \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=irogers@google.com \
    --cc=jolsa@kernel.org \
    --cc=kan.liang@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=mingo@redhat.com \
    --cc=namhyung@kernel.org \
    --cc=peterz@infradead.org \
    --cc=thomas.falcon@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox