BPF List
 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>,
	"James Clark" <james.clark@arm.com>,
	"Athira Rajeev" <atrajeev@linux.vnet.ibm.com>,
	"Colin Ian King" <colin.i.king@gmail.com>,
	"Ahelenia Ziemiańska" <nabijaczleweli@nabijaczleweli.xyz>,
	"Leo Yan" <leo.yan@linux.dev>, "Song Liu" <song@kernel.org>,
	"Miguel Ojeda" <ojeda@kernel.org>,
	"Liam Howlett" <liam.howlett@oracle.com>,
	"Ilkka Koskinen" <ilkka@os.amperecomputing.com>,
	"Ben Gainey" <ben.gainey@arm.com>,
	"K Prateek Nayak" <kprateek.nayak@amd.com>,
	"Kan Liang" <kan.liang@linux.intel.com>,
	"Yanteng Si" <siyanteng@loongson.cn>,
	"Ravi Bangoria" <ravi.bangoria@amd.com>,
	"Sun Haiyong" <sunhaiyong@loongson.cn>,
	"Changbin Du" <changbin.du@huawei.com>,
	"Masami Hiramatsu" <mhiramat@kernel.org>,
	zhaimingbing <zhaimingbing@cmss.chinamobile.com>,
	"Paran Lee" <p4ranlee@gmail.com>, "Li Dong" <lidong@vivo.com>,
	elfring@users.sourceforge.net, "Andi Kleen" <ak@linux.intel.com>,
	"Markus Elfring" <Markus.Elfring@web.de>,
	"Chengen Du" <chengen.du@canonical.com>,
	linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org,
	bpf@vger.kernel.org
Subject: [PATCH v2 12/13] perf dso: Reference counting related fixes
Date: Thu, 21 Mar 2024 09:02:59 -0700	[thread overview]
Message-ID: <20240321160300.1635121-13-irogers@google.com> (raw)
In-Reply-To: <20240321160300.1635121-1-irogers@google.com>

Ensure gets and puts are better aligned fixing reference couting
checking problems.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/util/machine.c    |  4 +--
 tools/perf/util/map.c        |  1 +
 tools/perf/util/symbol-elf.c | 51 ++++++++++++++++++------------------
 3 files changed, 28 insertions(+), 28 deletions(-)

diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index 4153dc92154d..ca8b1d7e4028 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -683,7 +683,7 @@ static int machine__process_ksymbol_register(struct machine *machine,
 					     struct perf_sample *sample __maybe_unused)
 {
 	struct symbol *sym;
-	struct dso *dso;
+	struct dso *dso = NULL;
 	struct map *map = maps__find(machine__kernel_maps(machine), event->ksymbol.addr);
 	int err = 0;
 
@@ -696,7 +696,6 @@ static int machine__process_ksymbol_register(struct machine *machine,
 		}
 		dso__set_kernel(dso, DSO_SPACE__KERNEL);
 		map = map__new2(0, dso);
-		dso__put(dso);
 		if (!map) {
 			err = -ENOMEM;
 			goto out;
@@ -735,6 +734,7 @@ static int machine__process_ksymbol_register(struct machine *machine,
 	dso__insert_symbol(dso, sym);
 out:
 	map__put(map);
+	dso__put(dso);
 	return err;
 }
 
diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
index 117c4bb78b35..e1d14936a60d 100644
--- a/tools/perf/util/map.c
+++ b/tools/perf/util/map.c
@@ -200,6 +200,7 @@ struct map *map__new(struct machine *machine, u64 start, u64 len,
 				dso__set_build_id(dso, dso__bid(header_bid_dso));
 				dso__set_header_build_id(dso, 1);
 			}
+			dso__put(header_bid_dso);
 		}
 		dso__put(dso);
 	}
diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index 3be5e8d1e278..e398abfd13a0 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -1419,7 +1419,7 @@ void __weak arch__sym_update(struct symbol *s __maybe_unused,
 static int dso__process_kernel_symbol(struct dso *dso, struct map *map,
 				      GElf_Sym *sym, GElf_Shdr *shdr,
 				      struct maps *kmaps, struct kmap *kmap,
-				      struct dso **curr_dsop, struct map **curr_mapp,
+				      struct dso **curr_dsop,
 				      const char *section_name,
 				      bool adjust_kernel_syms, bool kmodule, bool *remap_kernel,
 				      u64 max_text_sh_offset)
@@ -1470,8 +1470,8 @@ static int dso__process_kernel_symbol(struct dso *dso, struct map *map,
 			map__set_pgoff(map, shdr->sh_offset);
 		}
 
-		*curr_mapp = map;
-		*curr_dsop = dso;
+		dso__put(*curr_dsop);
+		*curr_dsop = dso__get(dso);
 		return 0;
 	}
 
@@ -1484,8 +1484,8 @@ static int dso__process_kernel_symbol(struct dso *dso, struct map *map,
 	 */
 	if (kmodule && adjust_kernel_syms && is_exe_text(shdr->sh_flags) &&
 	    shdr->sh_offset <= max_text_sh_offset) {
-		*curr_mapp = map;
-		*curr_dsop = dso;
+		dso__put(*curr_dsop);
+		*curr_dsop = dso__get(dso);
 		return 0;
 	}
 
@@ -1507,10 +1507,10 @@ static int dso__process_kernel_symbol(struct dso *dso, struct map *map,
 		dso__set_binary_type(curr_dso, dso__binary_type(dso));
 		dso__set_adjust_symbols(curr_dso, dso__adjust_symbols(dso));
 		curr_map = map__new2(start, curr_dso);
-		dso__put(curr_dso);
-		if (curr_map == NULL)
+		if (curr_map == NULL) {
+			dso__put(curr_dso);
 			return -1;
-
+		}
 		if (dso__kernel(curr_dso))
 			map__kmap(curr_map)->kmaps = kmaps;
 
@@ -1524,21 +1524,15 @@ static int dso__process_kernel_symbol(struct dso *dso, struct map *map,
 		dso__set_symtab_type(curr_dso, dso__symtab_type(dso));
 		if (maps__insert(kmaps, curr_map))
 			return -1;
-		/*
-		 * Add it before we drop the reference to curr_map, i.e. while
-		 * we still are sure to have a reference to this DSO via
-		 * *curr_map->dso.
-		 */
 		dsos__add(&maps__machine(kmaps)->dsos, curr_dso);
-		/* kmaps already got it */
-		map__put(curr_map);
 		dso__set_loaded(curr_dso);
-		*curr_mapp = curr_map;
+		dso__put(*curr_dsop);
 		*curr_dsop = curr_dso;
 	} else {
-		*curr_dsop = map__dso(curr_map);
-		map__put(curr_map);
+		dso__put(*curr_dsop);
+		*curr_dsop = dso__get(map__dso(curr_map));
 	}
+	map__put(curr_map);
 
 	return 0;
 }
@@ -1549,11 +1543,9 @@ dso__load_sym_internal(struct dso *dso, struct map *map, struct symsrc *syms_ss,
 {
 	struct kmap *kmap = dso__kernel(dso) ? map__kmap(map) : NULL;
 	struct maps *kmaps = kmap ? map__kmaps(map) : NULL;
-	struct map *curr_map = map;
-	struct dso *curr_dso = dso;
+	struct dso *curr_dso = NULL;
 	Elf_Data *symstrs, *secstrs, *secstrs_run, *secstrs_sym;
 	uint32_t nr_syms;
-	int err = -1;
 	uint32_t idx;
 	GElf_Ehdr ehdr;
 	GElf_Shdr shdr;
@@ -1656,6 +1648,7 @@ dso__load_sym_internal(struct dso *dso, struct map *map, struct symsrc *syms_ss,
 	if (kmodule && adjust_kernel_syms)
 		max_text_sh_offset = max_text_section(runtime_ss->elf, &runtime_ss->ehdr);
 
+	curr_dso = dso__get(dso);
 	elf_symtab__for_each_symbol(syms, nr_syms, idx, sym) {
 		struct symbol *f;
 		const char *elf_name = elf_sym__name(&sym, symstrs);
@@ -1744,9 +1737,13 @@ dso__load_sym_internal(struct dso *dso, struct map *map, struct symsrc *syms_ss,
 			--sym.st_value;
 
 		if (dso__kernel(dso)) {
-			if (dso__process_kernel_symbol(dso, map, &sym, &shdr, kmaps, kmap, &curr_dso, &curr_map,
-						       section_name, adjust_kernel_syms, kmodule,
-						       &remap_kernel, max_text_sh_offset))
+			if (dso__process_kernel_symbol(dso, map, &sym, &shdr,
+						       kmaps, kmap, &curr_dso,
+						       section_name,
+						       adjust_kernel_syms,
+						       kmodule,
+						       &remap_kernel,
+						       max_text_sh_offset))
 				goto out_elf_end;
 		} else if ((used_opd && runtime_ss->adjust_symbols) ||
 			   (!used_opd && syms_ss->adjust_symbols)) {
@@ -1795,6 +1792,7 @@ dso__load_sym_internal(struct dso *dso, struct map *map, struct symsrc *syms_ss,
 		__symbols__insert(dso__symbols(curr_dso), f, dso__kernel(dso));
 		nr++;
 	}
+	dso__put(curr_dso);
 
 	/*
 	 * For misannotated, zeroed, ASM function sizes.
@@ -1810,9 +1808,10 @@ dso__load_sym_internal(struct dso *dso, struct map *map, struct symsrc *syms_ss,
 			maps__fixup_end(kmaps);
 		}
 	}
-	err = nr;
+	return nr;
 out_elf_end:
-	return err;
+	dso__put(curr_dso);
+	return -1;
 }
 
 int dso__load_sym(struct dso *dso, struct map *map, struct symsrc *syms_ss,
-- 
2.44.0.396.g6e790dbe36-goog


  parent reply	other threads:[~2024-03-21 16:03 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-03-21 16:02 [PATCH v2 00/13] dso/dsos memory savings and clean up Ian Rogers
2024-03-21 16:02 ` [PATCH v2 01/13] perf dso: Reorder variables to save space in struct dso Ian Rogers
2024-03-21 20:28   ` Arnaldo Carvalho de Melo
2024-03-21 16:02 ` [PATCH v2 02/13] perf dsos: Attempt to better abstract dsos internals Ian Rogers
2024-03-21 16:02 ` [PATCH v2 03/13] perf dsos: Tidy reference counting and locking Ian Rogers
2024-03-21 16:02 ` [PATCH v2 04/13] perf dsos: Add dsos__for_each_dso Ian Rogers
2024-03-22 20:43   ` Namhyung Kim
2024-03-22 20:54     ` Namhyung Kim
2024-03-21 16:02 ` [PATCH v2 05/13] perf dso: Move dso functions out of dsos Ian Rogers
2024-03-21 16:02 ` [PATCH v2 06/13] perf dsos: Switch more loops to dsos__for_each_dso Ian Rogers
2024-03-21 16:02 ` [PATCH v2 07/13] perf dsos: Switch backing storage to array from rbtree/list Ian Rogers
2024-03-21 16:02 ` [PATCH v2 08/13] perf dsos: Remove __dsos__addnew Ian Rogers
2024-03-21 16:02 ` [PATCH v2 09/13] perf dsos: Remove __dsos__findnew_link_by_longname_id Ian Rogers
2024-03-21 16:02 ` [PATCH v2 10/13] perf dsos: Switch hand code to bsearch Ian Rogers
2024-03-21 16:02 ` [PATCH v2 11/13] perf dso: Add reference count checking and accessor functions Ian Rogers
2024-03-21 16:02 ` Ian Rogers [this message]
2024-03-25 17:22   ` [PATCH v2 12/13] perf dso: Reference counting related fixes Markus Elfring
2024-03-21 16:03 ` [PATCH v2 13/13] perf dso: Use container_of to avoid a pointer in dso_data Ian Rogers
2024-03-25 21:03 ` [PATCH v2 00/13] dso/dsos memory savings and clean up 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=20240321160300.1635121-13-irogers@google.com \
    --to=irogers@google.com \
    --cc=Markus.Elfring@web.de \
    --cc=acme@kernel.org \
    --cc=adrian.hunter@intel.com \
    --cc=ak@linux.intel.com \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=atrajeev@linux.vnet.ibm.com \
    --cc=ben.gainey@arm.com \
    --cc=bpf@vger.kernel.org \
    --cc=changbin.du@huawei.com \
    --cc=chengen.du@canonical.com \
    --cc=colin.i.king@gmail.com \
    --cc=elfring@users.sourceforge.net \
    --cc=ilkka@os.amperecomputing.com \
    --cc=james.clark@arm.com \
    --cc=jolsa@kernel.org \
    --cc=kan.liang@linux.intel.com \
    --cc=kprateek.nayak@amd.com \
    --cc=leo.yan@linux.dev \
    --cc=liam.howlett@oracle.com \
    --cc=lidong@vivo.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=mhiramat@kernel.org \
    --cc=mingo@redhat.com \
    --cc=nabijaczleweli@nabijaczleweli.xyz \
    --cc=namhyung@kernel.org \
    --cc=ojeda@kernel.org \
    --cc=p4ranlee@gmail.com \
    --cc=peterz@infradead.org \
    --cc=ravi.bangoria@amd.com \
    --cc=siyanteng@loongson.cn \
    --cc=song@kernel.org \
    --cc=sunhaiyong@loongson.cn \
    --cc=zhaimingbing@cmss.chinamobile.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