linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Ian Rogers <irogers@google.com>
To: Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@redhat.com>,
	 Arnaldo Carvalho de Melo <acme@kernel.org>,
	Namhyung Kim <namhyung@kernel.org>,
	 Mark Rutland <mark.rutland@arm.com>,
	 Alexander Shishkin <alexander.shishkin@linux.intel.com>,
	Jiri Olsa <jolsa@kernel.org>,  Ian Rogers <irogers@google.com>,
	Adrian Hunter <adrian.hunter@intel.com>,
	 Oliver Upton <oliver.upton@linux.dev>,
	Yang Jihong <yangjihong1@huawei.com>,
	 linux-kernel@vger.kernel.org, linux-perf-users@vger.kernel.org,
	 bpf@vger.kernel.org
Subject: [PATCH v1 2/6] perf trace: Ignore thread hashing in summary
Date: Tue, 13 Feb 2024 22:37:04 -0800	[thread overview]
Message-ID: <20240214063708.972376-3-irogers@google.com> (raw)
In-Reply-To: <20240214063708.972376-1-irogers@google.com>

Commit 91e467bc568f ("perf machine: Use hashtable for machine
threads") made the iteration of thread tids unordered. The perf trace
--summary output sorts and prints each hash bucket, rather than all
threads globally. Change this behavior by turn all threads into a
list, sort the list by number of trace events then by tids, finally
print the list. This also allows the rbtree in threads to be not
accessed outside of machine.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/builtin-trace.c  | 41 +++++++++++++++++++++----------------
 tools/perf/util/rb_resort.h |  5 -----
 2 files changed, 23 insertions(+), 23 deletions(-)

diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
index 109b8e64fe69..90eaff8c0f6e 100644
--- a/tools/perf/builtin-trace.c
+++ b/tools/perf/builtin-trace.c
@@ -74,6 +74,7 @@
 #include <linux/err.h>
 #include <linux/filter.h>
 #include <linux/kernel.h>
+#include <linux/list_sort.h>
 #include <linux/random.h>
 #include <linux/stringify.h>
 #include <linux/time64.h>
@@ -4312,34 +4313,38 @@ static unsigned long thread__nr_events(struct thread_trace *ttrace)
 	return ttrace ? ttrace->nr_events : 0;
 }
 
-DEFINE_RESORT_RB(threads,
-		(thread__nr_events(thread__priv(a->thread)) <
-		 thread__nr_events(thread__priv(b->thread))),
-	struct thread *thread;
-)
+static int trace_nr_events_cmp(void *priv __maybe_unused,
+			       const struct list_head *la,
+			       const struct list_head *lb)
 {
-	entry->thread = rb_entry(nd, struct thread_rb_node, rb_node)->thread;
+	struct thread_list *a = list_entry(la, struct thread_list, list);
+	struct thread_list *b = list_entry(lb, struct thread_list, list);
+	unsigned long a_nr_events = thread__nr_events(thread__priv(a->thread));
+	unsigned long b_nr_events = thread__nr_events(thread__priv(b->thread));
+
+	if (a_nr_events != b_nr_events)
+		return a_nr_events < b_nr_events ? -1 : 1;
+
+	/* Identical number of threads, place smaller tids first. */
+	return thread__tid(a->thread) < thread__tid(b->thread)
+		? -1
+		: (thread__tid(a->thread) > thread__tid(b->thread) ? 1 : 0);
 }
 
 static size_t trace__fprintf_thread_summary(struct trace *trace, FILE *fp)
 {
 	size_t printed = trace__fprintf_threads_header(fp);
-	struct rb_node *nd;
-	int i;
-
-	for (i = 0; i < THREADS__TABLE_SIZE; i++) {
-		DECLARE_RESORT_RB_MACHINE_THREADS(threads, trace->host, i);
+	LIST_HEAD(threads);
 
-		if (threads == NULL) {
-			fprintf(fp, "%s", "Error sorting output by nr_events!\n");
-			return 0;
-		}
+	if (machine__thread_list(trace->host, &threads) == 0) {
+		struct thread_list *pos;
 
-		resort_rb__for_each_entry(nd, threads)
-			printed += trace__fprintf_thread(fp, threads_entry->thread, trace);
+		list_sort(NULL, &threads, trace_nr_events_cmp);
 
-		resort_rb__delete(threads);
+		list_for_each_entry(pos, &threads, list)
+			printed += trace__fprintf_thread(fp, pos->thread, trace);
 	}
+	thread_list__delete(&threads);
 	return printed;
 }
 
diff --git a/tools/perf/util/rb_resort.h b/tools/perf/util/rb_resort.h
index 376e86cb4c3c..d927a0d25052 100644
--- a/tools/perf/util/rb_resort.h
+++ b/tools/perf/util/rb_resort.h
@@ -143,9 +143,4 @@ struct __name##_sorted *__name = __name##_sorted__new
 	DECLARE_RESORT_RB(__name)(&__ilist->rblist.entries.rb_root,		\
 				  __ilist->rblist.nr_entries)
 
-/* For 'struct machine->threads' */
-#define DECLARE_RESORT_RB_MACHINE_THREADS(__name, __machine, hash_bucket)    \
- DECLARE_RESORT_RB(__name)(&__machine->threads[hash_bucket].entries.rb_root, \
-			   __machine->threads[hash_bucket].nr)
-
 #endif /* _PERF_RESORT_RB_H_ */
-- 
2.43.0.687.g38aa6559b0-goog


  parent reply	other threads:[~2024-02-14  6:37 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-14  6:37 [PATCH v1 0/6] Thread memory improvements and fixes Ian Rogers
2024-02-14  6:37 ` [PATCH v1 1/6] perf report: Sort child tasks by tid Ian Rogers
2024-02-14 17:24   ` Arnaldo Carvalho de Melo
2024-02-14 17:42     ` Ian Rogers
2024-02-16 20:25       ` Namhyung Kim
2024-02-27  6:39   ` Namhyung Kim
2024-02-27  7:12     ` Ian Rogers
2024-02-28  6:11       ` Namhyung Kim
2024-02-28  7:05         ` Ian Rogers
2024-02-28 22:45           ` Namhyung Kim
2024-02-14  6:37 ` Ian Rogers [this message]
2024-02-14 17:25   ` [PATCH v1 2/6] perf trace: Ignore thread hashing in summary Arnaldo Carvalho de Melo
2024-02-14 18:27     ` Ian Rogers
2024-02-14 21:15       ` Ian Rogers
2024-02-14 21:36         ` Ian Rogers
2024-02-14 21:42           ` Ian Rogers
2024-02-16 14:57           ` Arnaldo Carvalho de Melo
2024-02-27  6:55   ` Namhyung Kim
2024-02-14  6:37 ` [PATCH v1 3/6] perf machine: Move fprintf to for_each loop and a callback Ian Rogers
2024-02-14  6:37 ` [PATCH v1 4/6] perf threads: Move threads to its own files Ian Rogers
2024-02-27  7:07   ` Namhyung Kim
2024-02-27  7:24     ` Ian Rogers
2024-02-27 17:31       ` Namhyung Kim
2024-02-27 19:02         ` Ian Rogers
2024-02-27 19:17         ` Arnaldo Carvalho de Melo
2024-02-27 21:42           ` Ian Rogers
2024-02-28  6:39             ` Namhyung Kim
2024-02-28  7:24               ` Ian Rogers
2024-02-28 23:43                 ` Namhyung Kim
2024-02-29  0:31                   ` Ian Rogers
2024-02-29 21:59       ` David Laight
2024-03-01  0:19         ` Ian Rogers
2024-02-14  6:37 ` [PATCH v1 5/6] perf threads: Switch from rbtree to hashmap Ian Rogers
2024-02-14  6:37 ` [PATCH v1 6/6] perf threads: Reduce table size from 256 to 8 Ian Rogers
2024-02-25 18:50 ` [PATCH v1 0/6] Thread memory improvements and fixes Ian Rogers

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=20240214063708.972376-3-irogers@google.com \
    --to=irogers@google.com \
    --cc=acme@kernel.org \
    --cc=adrian.hunter@intel.com \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=bpf@vger.kernel.org \
    --cc=jolsa@kernel.org \
    --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=oliver.upton@linux.dev \
    --cc=peterz@infradead.org \
    --cc=yangjihong1@huawei.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;
as well as URLs for NNTP newsgroup(s).