linux-kernel - Re: [PATCH v13 03/16] perf record: Introduce thread specific data array

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]
Date:   Mon, 31 Jan 2022 18:39:39 -0300
From:   Arnaldo Carvalho de Melo <acme@...nel.org>
To:     Alexey Bayduraev <alexey.v.bayduraev@...ux.intel.com>
Cc:     Jiri Olsa <jolsa@...hat.com>, Namhyung Kim <namhyung@...nel.org>,
        Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...hat.com>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        Andi Kleen <ak@...ux.intel.com>,
        Adrian Hunter <adrian.hunter@...el.com>,
        Alexander Antonov <alexander.antonov@...ux.intel.com>,
        Alexei Budankov <abudankov@...wei.com>,
        Riccardo Mancini <rickyman7@...il.com>
Subject: Re: [PATCH v13 03/16] perf record: Introduce thread specific data
 array

Em Mon, Jan 17, 2022 at 09:34:23PM +0300, Alexey Bayduraev escreveu:
> Introduce thread specific data object and array of such objects
> to store and manage thread local data. Implement functions to
> allocate, initialize, finalize and release thread specific data.
> 
> Thread local maps and overwrite_maps arrays keep pointers to
> mmap buffer objects to serve according to maps thread mask.
> Thread local pollfd array keeps event fds connected to mmaps
> buffers according to maps thread mask.
> 
> Thread control commands are delivered via thread local comm pipes
> and ctlfd_pos fd. External control commands (--control option)
> are delivered via evlist ctlfd_pos fd and handled by the main
> tool thread.
> 
> Acked-by: Namhyung Kim <namhyung@...il.com>
> Reviewed-by: Riccardo Mancini <rickyman7@...il.com>
> Tested-by: Riccardo Mancini <rickyman7@...il.com>
> Signed-off-by: Alexey Bayduraev <alexey.v.bayduraev@...ux.intel.com>

Some changes to reduce patch size, I have them in my local tree, will
publish later.

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 1645b40540b870aa..a8c7118a95c6a3fa 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -924,7 +924,7 @@ static void record__thread_data_close_pipes(struct record_thread *thread_data)
 
 static int record__thread_data_init_maps(struct record_thread *thread_data, struct evlist *evlist)
 {
-	int m, tm, nr_mmaps = evlist->core.nr_mmaps;
+	int nr_mmaps = evlist->core.nr_mmaps;
 	struct mmap *mmap = evlist->mmap;
 	struct mmap *overwrite_mmap = evlist->overwrite_mmap;
 	struct perf_cpu_map *cpus = evlist->core.cpus;
@@ -946,7 +946,7 @@ static int record__thread_data_init_maps(struct record_thread *thread_data, stru
 	pr_debug2("thread_data[%p]: nr_mmaps=%d, maps=%p, ow_maps=%p\n", thread_data,
 		 thread_data->nr_mmaps, thread_data->maps, thread_data->overwrite_maps);
 
-	for (m = 0, tm = 0; m < nr_mmaps && tm < thread_data->nr_mmaps; m++) {
+	for (int m = 0, tm = 0; m < nr_mmaps && tm < thread_data->nr_mmaps; m++) {
 		if (test_bit(cpus->map[m].cpu, thread_data->mask->maps.bits)) {
 			if (thread_data->maps) {
 				thread_data->maps[tm] = &mmap[m];
@@ -967,21 +967,18 @@ static int record__thread_data_init_maps(struct record_thread *thread_data, stru
 
 static int record__thread_data_init_pollfd(struct record_thread *thread_data, struct evlist *evlist)
 {
-	int f, tm, pos;
-	struct mmap *map, *overwrite_map;
-
 	fdarray__init(&thread_data->pollfd, 64);
 
-	for (tm = 0; tm < thread_data->nr_mmaps; tm++) {
-		map = thread_data->maps ? thread_data->maps[tm] : NULL;
-		overwrite_map = thread_data->overwrite_maps ?
-				thread_data->overwrite_maps[tm] : NULL;
+	for (int tm = 0; tm < thread_data->nr_mmaps; tm++) {
+		struct mmap *map = thread_data->maps ? thread_data->maps[tm] : NULL,
+			    *overwrite_map = thread_data->overwrite_maps ?
+						thread_data->overwrite_maps[tm] : NULL;
 
-		for (f = 0; f < evlist->core.pollfd.nr; f++) {
+		for (int f = 0; f < evlist->core.pollfd.nr; f++) {
 			void *ptr = evlist->core.pollfd.priv[f].ptr;
 
 			if ((map && ptr == map) || (overwrite_map && ptr == overwrite_map)) {
-				pos = fdarray__dup_entry_from(&thread_data->pollfd, f,
+				int pos = fdarray__dup_entry_from(&thread_data->pollfd, f,
 							      &evlist->core.pollfd);
 				if (pos < 0)
 					return pos;
@@ -996,13 +993,12 @@ static int record__thread_data_init_pollfd(struct record_thread *thread_data, st
 
 static void record__free_thread_data(struct record *rec)
 {
-	int t;
 	struct record_thread *thread_data = rec->thread_data;
 
 	if (thread_data == NULL)
 		return;
 
-	for (t = 0; t < rec->nr_threads; t++) {
+	for (int t = 0; t < rec->nr_threads; t++) {
 		record__thread_data_close_pipes(&thread_data[t]);
 		zfree(&thread_data[t].maps);
 		zfree(&thread_data[t].overwrite_maps);
@@ -1014,20 +1010,18 @@ static void record__free_thread_data(struct record *rec)
 
 static int record__alloc_thread_data(struct record *rec, struct evlist *evlist)
 {
-	int t, ret;
-	struct record_thread *thread_data;
+	struct record_thread *thread_data = rec->thread_data = zalloc(rec->nr_threads * sizeof(*(rec->thread_data)));
+	int ret;
 
-	rec->thread_data = zalloc(rec->nr_threads * sizeof(*(rec->thread_data)));
 	if (!rec->thread_data) {
 		pr_err("Failed to allocate thread data\n");
 		return -ENOMEM;
 	}
-	thread_data = rec->thread_data;
 
-	for (t = 0; t < rec->nr_threads; t++)
+	for (int t = 0; t < rec->nr_threads; t++)
 		record__thread_data_init_pipes(&thread_data[t]);
 
-	for (t = 0; t < rec->nr_threads; t++) {
+	for (int t = 0; t < rec->nr_threads; t++) {
 		thread_data[t].rec = rec;
 		thread_data[t].mask = &rec->thread_masks[t];
 		ret = record__thread_data_init_maps(&thread_data[t], evlist);