From nobody Fri Dec 19 04:02:25 2025 Received: from mail-yw1-f201.google.com (mail-yw1-f201.google.com [209.85.128.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id B4D2313C813 for ; Mon, 29 Apr 2024 18:46:42 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.201 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1714416404; cv=none; b=h2IO29pUIpdrSICFKwNL0qqelfGcgedrsW7qmkg3aLetmcWluJMfZVbohZqDHgaSvbju02ZA2Shfa+P1VunGCnybVuTtksdrjS8Lg2F30VnVsacVhxy32qxnEi0PGo+ixh198UEuiLb9qe3OnXbeaAzJvvB42MA7xtxH/mz+KYk= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1714416404; c=relaxed/simple; bh=EdPl+cF+Uacqv5Nfnm9cA4AOJEVckbieQ5lajSHXHVg=; h=Date:In-Reply-To:Message-Id:Mime-Version:References:Subject:From: To:Content-Type; b=geOnIft/uQ+QsqHfZWV5W0PA1UyF+QLNrX3txzNiuVhGFPyNQ42xrOOR8FZCjeQEmpWbvZlxC+rSdkQYdriDFrTi+7mcXwyzERN/7A6XMWiZgovMY4NcInHSvrgB5YWnzfIAXSF1gZYjx+HoDkXgqHVxOb6JxcC369khm6Jtovw= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com; spf=pass smtp.mailfrom=flex--irogers.bounces.google.com; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b=rJTwMPzc; arc=none smtp.client-ip=209.85.128.201 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=flex--irogers.bounces.google.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="rJTwMPzc" Received: by mail-yw1-f201.google.com with SMTP id 00721157ae682-61bbd6578f9so38425177b3.1 for ; Mon, 29 Apr 2024 11:46:42 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1714416402; x=1715021202; darn=vger.kernel.org; h=to:from:subject:references:mime-version:message-id:in-reply-to:date :from:to:cc:subject:date:message-id:reply-to; bh=HJdwWYWTyzE2O801RQGxgyFCeT+SeHyEmr9sJLF5pAo=; b=rJTwMPzcXGsoqyP9+32DZXAtXU+Uzkh978co5c1HnpcXIUGEegqm9Xyk+aVgpzGSpl TYFCGK+vyUGHmVOu8wRdPJHoFW6TjlY/EoSL7IGCWphYvLbaDNn/f4Utcnvz5f3Enm/R gT+6it8XBiUhNlplIQdQCJaiPDMeJP0apXNQm91/CdPTjNVvt1jnfpVCC70+ukkV3bEE RRrtg0IJXeDgyJcHePFZrDklLii6y0uWiOin5g9pIUjEFfJ0oqj0981Xbzgxu0CIhHYW 7yTtP1ZJWwKCP8xnTRndUlYTOL/NYN/asNKkPrHS0IT/HbnU3pBZCF7QeeuatN8tqQMK C0jw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1714416402; x=1715021202; h=to:from:subject:references:mime-version:message-id:in-reply-to:date :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=HJdwWYWTyzE2O801RQGxgyFCeT+SeHyEmr9sJLF5pAo=; b=oJnNfCwK63WGFq8Al8urGiefuTltuy18KZLHvmMkbdvtuvrC49/u7gGEiRNRb/I7lL djnyEHh5pCZf1lz8aEnrOe0bjJA9b1rqxDcb8k+1RFPYzPnibCmB4shm7wP57kMremIJ OzYjwtj/KJW5hSAn4Nt72nppJqNvukhdM57sLjJQj/TibyspSNTdAcGxw+TlczS/ycpT G7gkLGzsFgTxMB6LvVndzRy2M3LWnl28CD09u3sUaUfJjYqklYwptwjcRBKKok3nhuZ4 dbvzOuo5V9zbd0D7D1RqUA9A4268GiX+mmOLTghzREGIEY2/UPylyr1YkVtUkDHuOIDC JwbA== X-Forwarded-Encrypted: i=1; AJvYcCU12OefYiVz/7zysxpQc6wad2GN7n4vQXPQNTAvugu2ePTmA0CIpOKmzLP1LyBgBP19kwPh4DeZgEmGyPikhVqybVzAz8rvzoJEeM3w X-Gm-Message-State: AOJu0YxB6Z/KGUvRrefGlSFdkbPAuGSzq3KrKvF2Mya2zsw+sSYpaFsV QERJgSGcs91uR3Uf2OjV/e4ze+ffqYIQ/yCoZzS2zd39u5V0AyPsQFoCq7QQxKDMFtuvEIwnhA2 ybU1LvA== X-Google-Smtp-Source: AGHT+IFTnJ1QBKgxyv7v6YihXd3g1cL0STprVlZ/yglnrpeHKZmqQX2o2hA/SPAowLr/vyvoNEUiCnDUzvew X-Received: from irogers.svl.corp.google.com ([2620:15c:2a3:200:c137:aa10:25e1:8f1e]) (user=irogers job=sendgmr) by 2002:a05:6902:18ca:b0:dcc:c57c:8873 with SMTP id ck10-20020a05690218ca00b00dccc57c8873mr3736949ybb.9.1714416401555; Mon, 29 Apr 2024 11:46:41 -0700 (PDT) Date: Mon, 29 Apr 2024 11:46:14 -0700 In-Reply-To: <20240429184614.1224041-1-irogers@google.com> Message-Id: <20240429184614.1224041-8-irogers@google.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 References: <20240429184614.1224041-1-irogers@google.com> X-Mailer: git-send-email 2.44.0.769.g3c40516874-goog Subject: [PATCH v5 7/7] perf dso: Use container_of to avoid a pointer in dso_data From: Ian Rogers To: Peter Zijlstra , Ingo Molnar , Arnaldo Carvalho de Melo , Namhyung Kim , Mark Rutland , Alexander Shishkin , Jiri Olsa , Ian Rogers , Adrian Hunter , Kan Liang , James Clark , Athira Rajeev , Colin Ian King , nabijaczleweli@nabijaczleweli.xyz, Leo Yan , Song Liu , Ilkka Koskinen , Ben Gainey , K Prateek Nayak , Yanteng Si , Sun Haiyong , Changbin Du , Andi Kleen , Thomas Richter , Masami Hiramatsu , Dima Kogan , zhaimingbing , Paran Lee , Li Dong , Tiezhu Yang , Yang Jihong , Chengen Du , linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" The dso pointer in dso_data is necessary for reference count checking to account for the dso_data forming a global list of open dso's with references to the dso. The dso pointer also allows for the indirection that reference count checking needs. Outside of reference count checking the indirection isn't needed and container_of is more efficient and saves space. The reference count won't be increased by placing items onto the global list, matching how things were before the reference count checking change, but we assert the dso is in dsos holding it live (and that the set of open dsos is a subset of all dsos for the machine). Update the DSO data tests so that they use a dsos struct to make the invariant true. Signed-off-by: Ian Rogers --- tools/perf/tests/dso-data.c | 60 ++++++++++++++++++------------------- tools/perf/util/dso.c | 16 +++++++++- tools/perf/util/dso.h | 2 ++ 3 files changed, 46 insertions(+), 32 deletions(-) diff --git a/tools/perf/tests/dso-data.c b/tools/perf/tests/dso-data.c index fde4eca84b6f..5286ae8bd2d7 100644 --- a/tools/perf/tests/dso-data.c +++ b/tools/perf/tests/dso-data.c @@ -10,6 +10,7 @@ #include #include #include "dso.h" +#include "dsos.h" #include "machine.h" #include "symbol.h" #include "tests.h" @@ -123,9 +124,10 @@ static int test__dso_data(struct test_suite *test __ma= ybe_unused, int subtest __ TEST_ASSERT_VAL("No test file", file); =20 memset(&machine, 0, sizeof(machine)); + dsos__init(&machine.dsos); =20 - dso =3D dso__new((const char *)file); - + dso =3D dso__new(file); + TEST_ASSERT_VAL("Failed to add dso", !dsos__add(&machine.dsos, dso)); TEST_ASSERT_VAL("Failed to access to dso", dso__data_fd(dso, &machine) >=3D 0); =20 @@ -170,6 +172,7 @@ static int test__dso_data(struct test_suite *test __may= be_unused, int subtest __ } =20 dso__put(dso); + dsos__exit(&machine.dsos); unlink(file); return 0; } @@ -199,41 +202,35 @@ static long open_files_cnt(void) return nr - 1; } =20 -static struct dso **dsos; - -static int dsos__create(int cnt, int size) +static int dsos__create(int cnt, int size, struct dsos *dsos) { int i; =20 - dsos =3D malloc(sizeof(*dsos) * cnt); - TEST_ASSERT_VAL("failed to alloc dsos array", dsos); + dsos__init(dsos); =20 for (i =3D 0; i < cnt; i++) { - char *file; + struct dso *dso; + char *file =3D test_file(size); =20 - file =3D test_file(size); TEST_ASSERT_VAL("failed to get dso file", file); - - dsos[i] =3D dso__new(file); - TEST_ASSERT_VAL("failed to get dso", dsos[i]); + dso =3D dso__new(file); + TEST_ASSERT_VAL("failed to get dso", dso); + TEST_ASSERT_VAL("failed to add dso", !dsos__add(dsos, dso)); + dso__put(dso); } =20 return 0; } =20 -static void dsos__delete(int cnt) +static void dsos__delete(struct dsos *dsos) { - int i; - - for (i =3D 0; i < cnt; i++) { - struct dso *dso =3D dsos[i]; + for (unsigned int i =3D 0; i < dsos->cnt; i++) { + struct dso *dso =3D dsos->dsos[i]; =20 dso__data_close(dso); unlink(dso__name(dso)); - dso__put(dso); } - - free(dsos); + dsos__exit(dsos); } =20 static int set_fd_limit(int n) @@ -267,10 +264,10 @@ static int test__dso_data_cache(struct test_suite *te= st __maybe_unused, int subt /* and this is now our dso open FDs limit */ dso_cnt =3D limit / 2; TEST_ASSERT_VAL("failed to create dsos\n", - !dsos__create(dso_cnt, TEST_FILE_SIZE)); + !dsos__create(dso_cnt, TEST_FILE_SIZE, &machine.dsos)); =20 for (i =3D 0; i < (dso_cnt - 1); i++) { - struct dso *dso =3D dsos[i]; + struct dso *dso =3D machine.dsos.dsos[i]; =20 /* * Open dsos via dso__data_fd(), it opens the data @@ -290,17 +287,17 @@ static int test__dso_data_cache(struct test_suite *te= st __maybe_unused, int subt } =20 /* verify the first one is already open */ - TEST_ASSERT_VAL("dsos[0] is not open", dso__data(dsos[0])->fd !=3D -1); + TEST_ASSERT_VAL("dsos[0] is not open", dso__data(machine.dsos.dsos[0])->f= d !=3D -1); =20 /* open +1 dso to reach the allowed limit */ - fd =3D dso__data_fd(dsos[i], &machine); + fd =3D dso__data_fd(machine.dsos.dsos[i], &machine); TEST_ASSERT_VAL("failed to get fd", fd > 0); =20 /* should force the first one to be closed */ - TEST_ASSERT_VAL("failed to close dsos[0]", dso__data(dsos[0])->fd =3D=3D = -1); + TEST_ASSERT_VAL("failed to close dsos[0]", dso__data(machine.dsos.dsos[0]= )->fd =3D=3D -1); =20 /* cleanup everything */ - dsos__delete(dso_cnt); + dsos__delete(&machine.dsos); =20 /* Make sure we did not leak any file descriptor. */ nr_end =3D open_files_cnt(); @@ -325,9 +322,9 @@ static int test__dso_data_reopen(struct test_suite *tes= t __maybe_unused, int sub long nr_end, nr =3D open_files_cnt(), lim =3D new_limit(3); int fd, fd_extra; =20 -#define dso_0 (dsos[0]) -#define dso_1 (dsos[1]) -#define dso_2 (dsos[2]) +#define dso_0 (machine.dsos.dsos[0]) +#define dso_1 (machine.dsos.dsos[1]) +#define dso_2 (machine.dsos.dsos[2]) =20 /* Rest the internal dso open counter limit. */ reset_fd_limit(); @@ -347,7 +344,8 @@ static int test__dso_data_reopen(struct test_suite *tes= t __maybe_unused, int sub TEST_ASSERT_VAL("failed to set file limit", !set_fd_limit((lim))); =20 - TEST_ASSERT_VAL("failed to create dsos\n", !dsos__create(3, TEST_FILE_SIZ= E)); + TEST_ASSERT_VAL("failed to create dsos\n", + !dsos__create(3, TEST_FILE_SIZE, &machine.dsos)); =20 /* open dso_0 */ fd =3D dso__data_fd(dso_0, &machine); @@ -386,7 +384,7 @@ static int test__dso_data_reopen(struct test_suite *tes= t __maybe_unused, int sub =20 /* cleanup everything */ close(fd_extra); - dsos__delete(3); + dsos__delete(&machine.dsos); =20 /* Make sure we did not leak any file descriptor. */ nr_end =3D open_files_cnt(); diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c index 27db65e96e04..dde706b71da7 100644 --- a/tools/perf/util/dso.c +++ b/tools/perf/util/dso.c @@ -497,14 +497,20 @@ static pthread_mutex_t dso__data_open_lock =3D PTHREA= D_MUTEX_INITIALIZER; static void dso__list_add(struct dso *dso) { list_add_tail(&dso__data(dso)->open_entry, &dso__data_open); +#ifdef REFCNT_CHECKING dso__data(dso)->dso =3D dso__get(dso); +#endif + /* Assume the dso is part of dsos, hence the optional reference count abo= ve. */ + assert(dso__dsos(dso)); dso__data_open_cnt++; } =20 static void dso__list_del(struct dso *dso) { list_del_init(&dso__data(dso)->open_entry); +#ifdef REFCNT_CHECKING dso__put(dso__data(dso)->dso); +#endif WARN_ONCE(dso__data_open_cnt <=3D 0, "DSO data fd counter out of bounds."); dso__data_open_cnt--; @@ -654,9 +660,15 @@ static void close_dso(struct dso *dso) static void close_first_dso(void) { struct dso_data *dso_data; + struct dso *dso; =20 dso_data =3D list_first_entry(&dso__data_open, struct dso_data, open_entr= y); - close_dso(dso_data->dso); +#ifdef REFCNT_CHECKING + dso =3D dso_data->dso; +#else + dso =3D container_of(dso_data, struct dso, data); +#endif + close_dso(dso); } =20 static rlim_t get_fd_limit(void) @@ -1449,7 +1461,9 @@ struct dso *dso__new_id(const char *name, struct dso_= id *id) data->fd =3D -1; data->status =3D DSO_DATA_STATUS_UNKNOWN; INIT_LIST_HEAD(&data->open_entry); +#ifdef REFCNT_CHECKING data->dso =3D NULL; /* Set when on the open_entry list. */ +#endif } return res; } diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h index f9689dd60de3..df2c98402af3 100644 --- a/tools/perf/util/dso.h +++ b/tools/perf/util/dso.h @@ -147,7 +147,9 @@ struct dso_cache { struct dso_data { struct rb_root cache; struct list_head open_entry; +#ifdef REFCNT_CHECKING struct dso *dso; +#endif int fd; int status; u32 status_seen; --=20 2.44.0.769.g3c40516874-goog