perf dso: Use container_of() to avoid a pointer in 'struct dso_data'

The dso pointer in 'struct 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 <irogers@google.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
Cc: Changbin Du <changbin.du@huawei.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Kan Liang <kan.liang@linux.intel.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Tiezhu Yang <yangtiezhu@loongson.cn>
Link: https://lore.kernel.org/r/20240506180104.485674-5-irogers@google.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

diff --git a/tools/perf/tests/dso-data.c b/tools/perf/tests/dso-data.c
index fde4eca84b6f77cb4eaa4ef3d71b99710c9da2ad..5286ae8bd2d70d0628fca11c2b1b812def132f48 100644 (file)
--- a/tools/perf/tests/dso-data.c
+++ b/tools/perf/tests/dso-data.c
@@ -10,6 +10,7 @@
 #include <sys/resource.h>
 #include <api/fs/fs.h>
 #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 __maybe_unused, int subtest __
        TEST_ASSERT_VAL("No test file", file);
 
        memset(&machine, 0, sizeof(machine));
+       dsos__init(&machine.dsos);
 
-       dso = dso__new((const char *)file);
-
+       dso = 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) >= 0);
 
@@ -170,6 +172,7 @@ static int test__dso_data(struct test_suite *test __maybe_unused, int subtest __
        }
 
        dso__put(dso);
+       dsos__exit(&machine.dsos);
        unlink(file);
        return 0;
 }
@@ -199,41 +202,35 @@ static long open_files_cnt(void)
        return nr - 1;
 }
 
-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;
 
-       dsos = malloc(sizeof(*dsos) * cnt);
-       TEST_ASSERT_VAL("failed to alloc dsos array", dsos);
+       dsos__init(dsos);
 
        for (i = 0; i < cnt; i++) {
-               char *file;
+               struct dso *dso;
+               char *file = test_file(size);
 
-               file = test_file(size);
                TEST_ASSERT_VAL("failed to get dso file", file);
-
-               dsos[i] = dso__new(file);
-               TEST_ASSERT_VAL("failed to get dso", dsos[i]);
+               dso = 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);
        }
 
        return 0;
 }
 
-static void dsos__delete(int cnt)
+static void dsos__delete(struct dsos *dsos)
 {
-       int i;
-
-       for (i = 0; i < cnt; i++) {
-               struct dso *dso = dsos[i];
+       for (unsigned int i = 0; i < dsos->cnt; i++) {
+               struct dso *dso = dsos->dsos[i];
 
                dso__data_close(dso);
                unlink(dso__name(dso));
-               dso__put(dso);
        }
-
-       free(dsos);
+       dsos__exit(dsos);
 }
 
 static int set_fd_limit(int n)
@@ -267,10 +264,10 @@ static int test__dso_data_cache(struct test_suite *test __maybe_unused, int subt
        /* and this is now our dso open FDs limit */
        dso_cnt = 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));
 
        for (i = 0; i < (dso_cnt - 1); i++) {
-               struct dso *dso = dsos[i];
+               struct dso *dso = machine.dsos.dsos[i];
 
                /*
                 * Open dsos via dso__data_fd(), it opens the data
@@ -290,17 +287,17 @@ static int test__dso_data_cache(struct test_suite *test __maybe_unused, int subt
        }
 
        /* verify the first one is already open */
-       TEST_ASSERT_VAL("dsos[0] is not open", dso__data(dsos[0])->fd != -1);
+       TEST_ASSERT_VAL("dsos[0] is not open", dso__data(machine.dsos.dsos[0])->fd != -1);
 
        /* open +1 dso to reach the allowed limit */
-       fd = dso__data_fd(dsos[i], &machine);
+       fd = dso__data_fd(machine.dsos.dsos[i], &machine);
        TEST_ASSERT_VAL("failed to get fd", fd > 0);
 
        /* should force the first one to be closed */
-       TEST_ASSERT_VAL("failed to close dsos[0]", dso__data(dsos[0])->fd == -1);
+       TEST_ASSERT_VAL("failed to close dsos[0]", dso__data(machine.dsos.dsos[0])->fd == -1);
 
        /* cleanup everything */
-       dsos__delete(dso_cnt);
+       dsos__delete(&machine.dsos);
 
        /* Make sure we did not leak any file descriptor. */
        nr_end = open_files_cnt();
@@ -325,9 +322,9 @@ static int test__dso_data_reopen(struct test_suite *test __maybe_unused, int sub
        long nr_end, nr = open_files_cnt(), lim = new_limit(3);
        int fd, fd_extra;
 
-#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])
 
        /* Rest the internal dso open counter limit. */
        reset_fd_limit();
@@ -347,7 +344,8 @@ static int test__dso_data_reopen(struct test_suite *test __maybe_unused, int sub
        TEST_ASSERT_VAL("failed to set file limit",
                        !set_fd_limit((lim)));
 
-       TEST_ASSERT_VAL("failed to create dsos\n", !dsos__create(3, TEST_FILE_SIZE));
+       TEST_ASSERT_VAL("failed to create dsos\n",
+                       !dsos__create(3, TEST_FILE_SIZE, &machine.dsos));
 
        /* open dso_0 */
        fd = dso__data_fd(dso_0, &machine);
@@ -386,7 +384,7 @@ static int test__dso_data_reopen(struct test_suite *test __maybe_unused, int sub
 
        /* cleanup everything */
        close(fd_extra);
-       dsos__delete(3);
+       dsos__delete(&machine.dsos);
 
        /* Make sure we did not leak any file descriptor. */
        nr_end = open_files_cnt();

diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
index 27db65e96e0441988ad353a93596443d85046a9d..dde706b71da7b3b9e05bbd9d2c4cc8691c2c113c 100644 (file)
--- a/tools/perf/util/dso.c
+++ b/tools/perf/util/dso.c
@@ -497,14 +497,20 @@ static pthread_mutex_t dso__data_open_lock = PTHREAD_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 = dso__get(dso);
+#endif
+       /* Assume the dso is part of dsos, hence the optional reference count above. */
+       assert(dso__dsos(dso));
        dso__data_open_cnt++;
 }
 
 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 <= 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;
 
        dso_data = list_first_entry(&dso__data_open, struct dso_data, open_entry);
-       close_dso(dso_data->dso);
+#ifdef REFCNT_CHECKING
+       dso = dso_data->dso;
+#else
+       dso = container_of(dso_data, struct dso, data);
+#endif
+       close_dso(dso);
 }
 
 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 = -1;
                data->status = DSO_DATA_STATUS_UNKNOWN;
                INIT_LIST_HEAD(&data->open_entry);
+#ifdef REFCNT_CHECKING
                data->dso = 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 f9689dd60de3e0b4c5d0e6fe6c4834cda21237d0..df2c98402af3e6fafffc65b9c1612eb7701fcb6f 100644 (file)
--- 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;