Commit ee756ef7 authored by Ian Rogers's avatar Ian Rogers Committed by Arnaldo Carvalho de Melo
Browse files

perf dso: Add reference count checking and accessor functions



Add reference count checking to struct dso, this can help with
implementing correct reference counting discipline. To avoid
RC_CHK_ACCESS everywhere, add accessor functions for the variables in
struct dso.

The majority of the change is mechanical in nature and not easy to
split up.

Committer testing:

'perf test' up to this patch shows no regressions.

But:

  util/symbol.c: In function ‘dso__load_bfd_symbols’:
  util/symbol.c:1683:9: error: too few arguments to function ‘dso__set_adjust_symbols’
   1683 |         dso__set_adjust_symbols(dso);
        |         ^~~~~~~~~~~~~~~~~~~~~~~
  In file included from util/symbol.c:21:
  util/dso.h:268:20: note: declared here
    268 | static inline void dso__set_adjust_symbols(struct dso *dso, bool val)
        |                    ^~~~~~~~~~~~~~~~~~~~~~~
  make[6]: *** [/home/acme/git/perf-tools-next/tools/build/Makefile.build:106: /tmp/tmp.ZWHbQftdN6/util/symbol.o] Error 1
    MKDIR   /tmp/tmp.ZWHbQftdN6/tests/workloads/
  make[6]: *** Waiting for unfinished jobs....

This was updated:

  -       symbols__fixup_end(&dso->symbols, false);
  -       symbols__fixup_duplicate(&dso->symbols);
  -       dso->adjust_symbols = 1;
  +       symbols__fixup_end(dso__symbols(dso), false);
  +       symbols__fixup_duplicate(dso__symbols(dso));
  +       dso__set_adjust_symbols(dso);

But not build tested with BUILD_NONDISTRO and libbfd devel files installed
(binutils-devel on fedora).

Add the missing argument:

   	symbols__fixup_end(dso__symbols(dso), false);
   	symbols__fixup_duplicate(dso__symbols(dso));
  -	dso__set_adjust_symbols(dso);
  +	dso__set_adjust_symbols(dso, true);

Signed-off-by: default avatarIan Rogers <irogers@google.com>
Tested-by: default avatarArnaldo Carvalho de Melo <acme@redhat.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
Cc: Ben Gainey <ben.gainey@arm.com>
Cc: Changbin Du <changbin.du@huawei.com>
Cc: Chengen Du <chengen.du@canonical.com>
Cc: Colin Ian King <colin.i.king@gmail.com>
Cc: Dima Kogan <dima@secretsauce.net>
Cc: Ilkka Koskinen <ilkka@os.amperecomputing.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: James Clark <james.clark@arm.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: K Prateek Nayak <kprateek.nayak@amd.com>
Cc: Kan Liang <kan.liang@linux.intel.com>
Cc: Leo Yan <leo.yan@linux.dev>
Cc: Li Dong <lidong@vivo.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paran Lee <p4ranlee@gmail.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Song Liu <song@kernel.org>
Cc: Sun Haiyong <sunhaiyong@loongson.cn>
Cc: Thomas Richter <tmricht@linux.ibm.com>
Cc: Tiezhu Yang <yangtiezhu@loongson.cn>
Cc: Yanteng Si <siyanteng@loongson.cn>
Cc: zhaimingbing <zhaimingbing@cmss.chinamobile.com>
Link: https://lore.kernel.org/r/20240504213803.218974-6-irogers@google.com


Signed-off-by: default avatarArnaldo Carvalho de Melo <acme@redhat.com>
parent 7a9418cf
Loading
Loading
Loading
Loading
+3 −3
Original line number Diff line number Diff line
@@ -219,7 +219,7 @@ static int process_branch_callback(struct evsel *evsel,
	}

	if (a.map != NULL)
		map__dso(a.map)->hit = 1;
		dso__set_hit(map__dso(a.map));

	hist__account_cycles(sample->branch_stack, al, sample, false, NULL);

@@ -254,7 +254,7 @@ static int evsel__add_sample(struct evsel *evsel, struct perf_sample *sample,
		if (al->sym != NULL) {
			struct dso *dso = map__dso(al->map);

			rb_erase_cached(&al->sym->rb_node, &dso->symbols);
			rb_erase_cached(&al->sym->rb_node, dso__symbols(dso));
			symbol__delete(al->sym);
			dso__reset_find_symbol_cache(dso);
		}
@@ -419,7 +419,7 @@ static void hists__find_annotations(struct hists *hists,
		struct hist_entry *he = rb_entry(nd, struct hist_entry, rb_node);
		struct annotation *notes;

		if (he->ms.sym == NULL || map__dso(he->ms.map)->annotate_warned)
		if (he->ms.sym == NULL || dso__annotate_warned(map__dso(he->ms.map)))
			goto find_next;

		if (ann->sym_hist_filter &&
+1 −1
Original line number Diff line number Diff line
@@ -286,7 +286,7 @@ static bool dso__missing_buildid_cache(struct dso *dso, int parm __maybe_unused)

		pr_warning("Problems with %s file, consider removing it from the cache\n",
			   filename);
	} else if (memcmp(dso->bid.data, bid.data, bid.size)) {
	} else if (memcmp(dso__bid(dso)->data, bid.data, bid.size)) {
		pr_warning("Problems with %s file, consider removing it from the cache\n",
			   filename);
	}
+10 −8
Original line number Diff line number Diff line
@@ -26,16 +26,18 @@ static int buildid__map_cb(struct map *map, void *arg __maybe_unused)
{
	const struct dso *dso = map__dso(map);
	char bid_buf[SBUILD_ID_SIZE];
	const char *dso_long_name = dso__long_name(dso);
	const char *dso_short_name = dso__short_name(dso);

	memset(bid_buf, 0, sizeof(bid_buf));
	if (dso->has_build_id)
		build_id__sprintf(&dso->bid, bid_buf);
	if (dso__has_build_id(dso))
		build_id__sprintf(dso__bid_const(dso), bid_buf);
	printf("%s %16" PRIx64 " %16" PRIx64, bid_buf, map__start(map), map__end(map));
	if (dso->long_name != NULL) {
		printf(" %s", dso->long_name);
	} else if (dso->short_name != NULL) {
		printf(" %s", dso->short_name);
	}
	if (dso_long_name != NULL)
		printf(" %s", dso_long_name);
	else if (dso_short_name != NULL)
		printf(" %s", dso_short_name);

	printf("\n");

	return 0;
@@ -76,7 +78,7 @@ static int filename__fprintf_build_id(const char *name, FILE *fp)

static bool dso__skip_buildid(struct dso *dso, int with_hits)
{
	return with_hits && !dso->hit;
	return with_hits && !dso__hit(dso);
}

static int perf_session__list_build_ids(bool force, bool with_hits)
+35 −36
Original line number Diff line number Diff line
@@ -445,10 +445,9 @@ static struct dso *findnew_dso(int pid, int tid, const char *filename,
	}

	if (dso) {
		mutex_lock(&dso->lock);
		nsinfo__put(dso->nsinfo);
		dso->nsinfo = nsi;
		mutex_unlock(&dso->lock);
		mutex_lock(dso__lock(dso));
		dso__set_nsinfo(dso, nsi);
		mutex_unlock(dso__lock(dso));
	} else
		nsinfo__put(nsi);

@@ -466,8 +465,8 @@ static int perf_event__repipe_buildid_mmap(struct perf_tool *tool,
	dso = findnew_dso(event->mmap.pid, event->mmap.tid,
			  event->mmap.filename, NULL, machine);

	if (dso && !dso->hit) {
		dso->hit = 1;
	if (dso && !dso__hit(dso)) {
		dso__set_hit(dso);
		dso__inject_build_id(dso, tool, machine, sample->cpumode, 0);
	}
	dso__put(dso);
@@ -492,7 +491,7 @@ static int perf_event__repipe_mmap2(struct perf_tool *tool,
				  event->mmap2.filename, NULL, machine);
		if (dso) {
			/* mark it not to inject build-id */
			dso->hit = 1;
			dso__set_hit(dso);
		}
		dso__put(dso);
	}
@@ -544,7 +543,7 @@ static int perf_event__repipe_buildid_mmap2(struct perf_tool *tool,
				  event->mmap2.filename, NULL, machine);
		if (dso) {
			/* mark it not to inject build-id */
			dso->hit = 1;
			dso__set_hit(dso);
		}
		dso__put(dso);
		perf_event__repipe(tool, event, sample, machine);
@@ -554,8 +553,8 @@ static int perf_event__repipe_buildid_mmap2(struct perf_tool *tool,
	dso = findnew_dso(event->mmap2.pid, event->mmap2.tid,
			  event->mmap2.filename, &dso_id, machine);

	if (dso && !dso->hit) {
		dso->hit = 1;
	if (dso && !dso__hit(dso)) {
		dso__set_hit(dso);
		dso__inject_build_id(dso, tool, machine, sample->cpumode,
				     event->mmap2.flags);
	}
@@ -631,24 +630,24 @@ static int dso__read_build_id(struct dso *dso)
{
	struct nscookie nsc;

	if (dso->has_build_id)
	if (dso__has_build_id(dso))
		return 0;

	mutex_lock(&dso->lock);
	nsinfo__mountns_enter(dso->nsinfo, &nsc);
	if (filename__read_build_id(dso->long_name, &dso->bid) > 0)
		dso->has_build_id = true;
	else if (dso->nsinfo) {
		char *new_name = dso__filename_with_chroot(dso, dso->long_name);
	mutex_lock(dso__lock(dso));
	nsinfo__mountns_enter(dso__nsinfo(dso), &nsc);
	if (filename__read_build_id(dso__long_name(dso), dso__bid(dso)) > 0)
		dso__set_has_build_id(dso);
	else if (dso__nsinfo(dso)) {
		char *new_name = dso__filename_with_chroot(dso, dso__long_name(dso));

		if (new_name && filename__read_build_id(new_name, &dso->bid) > 0)
			dso->has_build_id = true;
		if (new_name && filename__read_build_id(new_name, dso__bid(dso)) > 0)
			dso__set_has_build_id(dso);
		free(new_name);
	}
	nsinfo__mountns_exit(&nsc);
	mutex_unlock(&dso->lock);
	mutex_unlock(dso__lock(dso));

	return dso->has_build_id ? 0 : -1;
	return dso__has_build_id(dso) ? 0 : -1;
}

static struct strlist *perf_inject__parse_known_build_ids(
@@ -700,14 +699,14 @@ static bool perf_inject__lookup_known_build_id(struct perf_inject *inject,
		dso_name = strchr(build_id, ' ');
		bid_len = dso_name - pos->s;
		dso_name = skip_spaces(dso_name);
		if (strcmp(dso->long_name, dso_name))
		if (strcmp(dso__long_name(dso), dso_name))
			continue;
		for (int ix = 0; 2 * ix + 1 < bid_len; ++ix) {
			dso->bid.data[ix] = (hex(build_id[2 * ix]) << 4 |
			dso__bid(dso)->data[ix] = (hex(build_id[2 * ix]) << 4 |
						  hex(build_id[2 * ix + 1]));
		}
		dso->bid.size = bid_len / 2;
		dso->has_build_id = 1;
		dso__bid(dso)->size = bid_len / 2;
		dso__set_has_build_id(dso);
		return true;
	}
	return false;
@@ -720,9 +719,9 @@ static int dso__inject_build_id(struct dso *dso, struct perf_tool *tool,
						  tool);
	int err;

	if (is_anon_memory(dso->long_name) || flags & MAP_HUGETLB)
	if (is_anon_memory(dso__long_name(dso)) || flags & MAP_HUGETLB)
		return 0;
	if (is_no_dso_memory(dso->long_name))
	if (is_no_dso_memory(dso__long_name(dso)))
		return 0;

	if (inject->known_build_ids != NULL &&
@@ -730,14 +729,14 @@ static int dso__inject_build_id(struct dso *dso, struct perf_tool *tool,
		return 1;

	if (dso__read_build_id(dso) < 0) {
		pr_debug("no build_id found for %s\n", dso->long_name);
		pr_debug("no build_id found for %s\n", dso__long_name(dso));
		return -1;
	}

	err = perf_event__synthesize_build_id(tool, dso, cpumode,
					      perf_event__repipe, machine);
	if (err) {
		pr_err("Can't synthesize build_id event for %s\n", dso->long_name);
		pr_err("Can't synthesize build_id event for %s\n", dso__long_name(dso));
		return -1;
	}

@@ -763,8 +762,8 @@ int perf_event__inject_buildid(struct perf_tool *tool, union perf_event *event,
	if (thread__find_map(thread, sample->cpumode, sample->ip, &al)) {
		struct dso *dso = map__dso(al.map);

		if (!dso->hit) {
			dso->hit = 1;
		if (!dso__hit(dso)) {
			dso__set_hit(dso);
			dso__inject_build_id(dso, tool, machine,
					     sample->cpumode, map__flags(al.map));
		}
@@ -1146,8 +1145,8 @@ static bool dso__is_in_kernel_space(struct dso *dso)
		return false;

	return dso__is_kcore(dso) ||
	       dso->kernel ||
	       is_kernel_module(dso->long_name, PERF_RECORD_MISC_CPUMODE_UNKNOWN);
	       dso__kernel(dso) ||
	       is_kernel_module(dso__long_name(dso), PERF_RECORD_MISC_CPUMODE_UNKNOWN);
}

static u64 evlist__first_id(struct evlist *evlist)
@@ -1181,7 +1180,7 @@ static int synthesize_build_id(struct perf_inject *inject, struct dso *dso, pid_
	if (!machine)
		return -ENOMEM;

	dso->hit = 1;
	dso__set_hit(dso);

	return perf_event__synthesize_build_id(&inject->tool, dso, cpumode,
					       process_build_id, machine);
@@ -1192,7 +1191,7 @@ static int guest_session__add_build_ids_cb(struct dso *dso, void *data)
	struct guest_session *gs = data;
	struct perf_inject *inject = container_of(gs, struct perf_inject, guest_session);

	if (!dso->has_build_id)
	if (!dso__has_build_id(dso))
		return 0;

	return synthesize_build_id(inject, dso, gs->machine_pid);
+1 −1
Original line number Diff line number Diff line
@@ -38,7 +38,7 @@ static int __cmd_kallsyms(int argc, const char **argv)

		dso = map__dso(map);
		printf("%s: %s %s %#" PRIx64 "-%#" PRIx64 " (%#" PRIx64 "-%#" PRIx64")\n",
			symbol->name, dso->short_name, dso->long_name,
			symbol->name, dso__short_name(dso), dso__long_name(dso),
			map__unmap_ip(map, symbol->start), map__unmap_ip(map, symbol->end),
			symbol->start, symbol->end);
	}
Loading