From nobody Wed Oct 8 21:34:42 2025 Received: from mail-pf1-f202.google.com (mail-pf1-f202.google.com [209.85.210.202]) (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 5C7702E9ECC for ; Tue, 24 Jun 2025 19:03:37 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.210.202 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1750791819; cv=none; b=gbfr3kSVtj+3B8MtzoF5iQESfPutALEHgloG5O4GQgM/RwnvlTbW86QWEUPFgxGyY4Nzr1jyQKOBl5IlbGqef0/GFngUH+2LKDzHRo6RWekYsQQ6LpfUYdHFKO36jMQI/Hty8Wb0/E51WewKcfCXAp5SbjZ/ifA6bFMj0TACpGc= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1750791819; c=relaxed/simple; bh=ktXu6VcAdkGJ2v1xOhOgufhv3qxhgg3zbylT0vutCvY=; h=Date:In-Reply-To:Mime-Version:References:Message-ID:Subject:From: To:Content-Type; b=X8Y9syza4HPkVfjeeyUTx2QlJQdxoCs5afr+BDzLvUgjSK47UehWi+ufZejSintmFWH3flJCnbbOon71U9wQ6d728kDG9Q4VWu8P4PTUNyf/N/oUhl7V5qsx4efUEeumrzNJcFLtX9+RX6vTXyjMApSj8tEmzjwUIrgFQMVQuXE= 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=xKmvhRRa; arc=none smtp.client-ip=209.85.210.202 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="xKmvhRRa" Received: by mail-pf1-f202.google.com with SMTP id d2e1a72fcca58-74928291bc3so439326b3a.0 for ; Tue, 24 Jun 2025 12:03:37 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1750791816; x=1751396616; darn=vger.kernel.org; h=to:from:subject:message-id:references:mime-version:in-reply-to:date :from:to:cc:subject:date:message-id:reply-to; bh=afW80ZcEEJPE1ltr0EmfHWGXUQF1oWc8ptVOF2S/Uts=; b=xKmvhRRa66vrJMPgD8+9E/ld2Ij4Inbzc4+fHidIpoHpwaJS7k7AgorQvIsBSC2X6x PVADmIyQpHz8oAaPPF1keB3fF38j1c6b0gsKaMm5DS35TTHr8syuhzISGZnSQk90r2Js esh6bPYZK1krSzNizhYi7Z+IN9KpX1DERReG1oiYEUq5pmo+fLW5BusFf76q8LhDqyZX WilrGyumzDLExLzUKXp2Zv2cZ8RvOQ3z58ad5V6miAbEsKMLKrWxqdzlTLS29KC0Iah8 lL5IHNnPBB9zgsz10zhf6Bwib47RfdhrEY2lJPurEp2+xT13wjuW5OMDlZN90XLV3qT5 Z7tQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1750791816; x=1751396616; h=to:from:subject:message-id:references:mime-version:in-reply-to:date :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=afW80ZcEEJPE1ltr0EmfHWGXUQF1oWc8ptVOF2S/Uts=; b=CtbgtlZZgV7dfHhMYJ4j273iIYNKL5Lm5Ljh4Kvcb5lbmXzLSdF771gsu+xRfqV7pW 38dG2SoW6DqdaOsvcttVxT3sawnJ/QnLnUou+Xv2tgVtOwypfTMXb/vVqyMywVQ5E8w4 PJMhPgtIaZbm/JwpLOYaY6PxYj6TXRoh6cSy8zc6jNYkQ9bXbtQ88+T38iFto1Rwx5JY cYlopCZKE1LbjERMdpD65laCIm2R1RsCEy2B9wyok7NNtYR04VTFNIsRgCUflgGV66k3 YseDR+nYwPBi1Va0v/JWxUst3Has738O+GBtSVNp/d1No9Blp3MjINq0Vn/6eLER+gG7 d1ZA== X-Forwarded-Encrypted: i=1; AJvYcCXuARAjaT4wN6FG5I/Ldzh6nIZYTaePiiNwVe5sORoCXVMM6luHDId4ew/UP8lj//GPIVEwmT06S/dRiFk=@vger.kernel.org X-Gm-Message-State: AOJu0Yzp0tfX2sVzibRSJqac9AjJ/JaZwTXgBvP7ajnZarDq+Y28sI4g 4eQ4TkXq8KjW1D2ewwSLZkltX+tiA1FUZbboCmn2RpgXhN3CzC5aak7fFOrOo2lMgBjBHVxbC/q NGAUZrltOvA== X-Google-Smtp-Source: AGHT+IGO41J2m+KRVvAkZqtaf/fDEU02WT3KsOZvnS1GIfQ/BtppdtfGOoQgWLRf6EMpLFJFzoEGCm7POlSL X-Received: from pfbha5.prod.google.com ([2002:a05:6a00:8505:b0:748:df52:fdc5]) (user=irogers job=prod-delivery.src-stubby-dispatcher) by 2002:a05:6a00:391a:b0:737:9b:582a with SMTP id d2e1a72fcca58-74ad4546a08mr383375b3a.24.1750791816273; Tue, 24 Jun 2025 12:03:36 -0700 (PDT) Date: Tue, 24 Jun 2025 12:03:23 -0700 In-Reply-To: <20250624190326.2038704-1-irogers@google.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 References: <20250624190326.2038704-1-irogers@google.com> X-Mailer: git-send-email 2.50.0.714.g196bf9f422-goog Message-ID: <20250624190326.2038704-4-irogers@google.com> Subject: [PATCH v1 3/5] perf hwmon_pmu: Hold path rather than fd 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 , Howard Chu , Charlie Jenkins , Thomas Richter , Athira Rajeev , Stephen Brennan , Jean-Philippe Romain , Junhao He , "Dr. David Alan Gilbert" , Dmitry Vyukov , linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" Hold the path to the hwmon_pmu rather than the file descriptor. The file descriptor is somewhat problematic in that it reflects the directory state when opened, something that may vary in testing. Using a path simplifies testing and to some extent cleanup as the hwmon_pmu is owned by the pmus list and intentionally global and leaked when perf terminates, the file descriptor being left open looks like a leak. Signed-off-by: Ian Rogers --- tools/perf/tests/hwmon_pmu.c | 11 ++++++----- tools/perf/util/hwmon_pmu.c | 38 ++++++++++++++++++++++++++---------- tools/perf/util/hwmon_pmu.h | 4 ++-- tools/perf/util/pmus.c | 2 +- tools/perf/util/pmus.h | 2 +- 5 files changed, 38 insertions(+), 19 deletions(-) diff --git a/tools/perf/tests/hwmon_pmu.c b/tools/perf/tests/hwmon_pmu.c index 0837aca1cdfa..151f02701c8c 100644 --- a/tools/perf/tests/hwmon_pmu.c +++ b/tools/perf/tests/hwmon_pmu.c @@ -93,9 +93,10 @@ static struct perf_pmu *test_pmu_get(char *dir, size_t s= z) pr_err("Failed to mkdir hwmon directory\n"); goto err_out; } - hwmon_dirfd =3D openat(test_dirfd, "hwmon1234", O_DIRECTORY); + strncat(dir, "/hwmon1234", sz - strlen(dir)); + hwmon_dirfd =3D open(dir, O_PATH|O_DIRECTORY); if (hwmon_dirfd < 0) { - pr_err("Failed to open test hwmon directory \"%s/hwmon1234\"\n", dir); + pr_err("Failed to open test hwmon directory \"%s\"\n", dir); goto err_out; } file =3D openat(hwmon_dirfd, "name", O_WRONLY | O_CREAT, 0600); @@ -130,18 +131,18 @@ static struct perf_pmu *test_pmu_get(char *dir, size_= t sz) } =20 /* Make the PMU reading the files created above. */ - hwm =3D perf_pmus__add_test_hwmon_pmu(hwmon_dirfd, "hwmon1234", test_hwmo= n_name); + hwm =3D perf_pmus__add_test_hwmon_pmu(dir, "hwmon1234", test_hwmon_name); if (!hwm) pr_err("Test hwmon creation failed\n"); =20 err_out: if (!hwm) { test_pmu_put(dir, hwm); - if (hwmon_dirfd >=3D 0) - close(hwmon_dirfd); } if (test_dirfd >=3D 0) close(test_dirfd); + if (hwmon_dirfd >=3D 0) + close(hwmon_dirfd); return hwm; } =20 diff --git a/tools/perf/util/hwmon_pmu.c b/tools/perf/util/hwmon_pmu.c index c25e7296f1c1..7edda010ba27 100644 --- a/tools/perf/util/hwmon_pmu.c +++ b/tools/perf/util/hwmon_pmu.c @@ -104,7 +104,7 @@ static const char *const hwmon_units[HWMON_TYPE_MAX] = =3D { struct hwmon_pmu { struct perf_pmu pmu; struct hashmap events; - int hwmon_dir_fd; + char *hwmon_dir; }; =20 /** @@ -245,7 +245,7 @@ static int hwmon_pmu__read_events(struct hwmon_pmu *pmu) return 0; =20 /* Use openat so that the directory contents are refreshed. */ - io_dir__init(&dir, openat(pmu->hwmon_dir_fd, ".", O_CLOEXEC | O_DIRECTORY= | O_RDONLY)); + io_dir__init(&dir, open(pmu->hwmon_dir, O_CLOEXEC | O_DIRECTORY | O_RDONL= Y)); =20 if (dir.dirfd < 0) return -ENOENT; @@ -283,7 +283,7 @@ static int hwmon_pmu__read_events(struct hwmon_pmu *pmu) __set_bit(item, alarm ? value->alarm_items : value->items); if (item =3D=3D HWMON_ITEM_LABEL) { char buf[128]; - int fd =3D openat(pmu->hwmon_dir_fd, ent->d_name, O_RDONLY); + int fd =3D openat(dir.dirfd, ent->d_name, O_RDONLY); ssize_t read_len; =20 if (fd < 0) @@ -342,7 +342,8 @@ static int hwmon_pmu__read_events(struct hwmon_pmu *pmu) return err; } =20 -struct perf_pmu *hwmon_pmu__new(struct list_head *pmus, int hwmon_dir, con= st char *sysfs_name, const char *name) +struct perf_pmu *hwmon_pmu__new(struct list_head *pmus, const char *hwmon_= dir, + const char *sysfs_name, const char *name) { char buf[32]; struct hwmon_pmu *hwm; @@ -365,7 +366,11 @@ struct perf_pmu *hwmon_pmu__new(struct list_head *pmus= , int hwmon_dir, const cha return NULL; } =20 - hwm->hwmon_dir_fd =3D hwmon_dir; + hwm->hwmon_dir =3D strdup(hwmon_dir); + if (!hwm->hwmon_dir) { + perf_pmu__delete(&hwm->pmu); + return NULL; + } hwm->pmu.alias_name =3D strdup(sysfs_name); if (!hwm->pmu.alias_name) { perf_pmu__delete(&hwm->pmu); @@ -399,7 +404,7 @@ void hwmon_pmu__exit(struct perf_pmu *pmu) free(value); } hashmap__clear(&hwm->events); - close(hwm->hwmon_dir_fd); + zfree(&hwm->hwmon_dir); } =20 static size_t hwmon_pmu__describe_items(struct hwmon_pmu *hwm, char *out_b= uf, size_t out_buf_len, @@ -409,6 +414,10 @@ static size_t hwmon_pmu__describe_items(struct hwmon_p= mu *hwm, char *out_buf, si size_t bit; char buf[64]; size_t len =3D 0; + int dir =3D open(hwm->hwmon_dir, O_CLOEXEC | O_DIRECTORY | O_RDONLY); + + if (dir < 0) + return 0; =20 for_each_set_bit(bit, items, HWMON_ITEM__MAX) { int fd; @@ -421,7 +430,7 @@ static size_t hwmon_pmu__describe_items(struct hwmon_pm= u *hwm, char *out_buf, si key.num, hwmon_item_strs[bit], is_alarm ? "_alarm" : ""); - fd =3D openat(hwm->hwmon_dir_fd, buf, O_RDONLY); + fd =3D openat(dir, buf, O_RDONLY); if (fd > 0) { ssize_t read_len =3D read(fd, buf, sizeof(buf)); =20 @@ -443,6 +452,7 @@ static size_t hwmon_pmu__describe_items(struct hwmon_pm= u *hwm, char *out_buf, si close(fd); } } + close(dir); return len; } =20 @@ -712,6 +722,7 @@ int perf_pmus__read_hwmon_pmus(struct list_head *pmus) size_t line_len; int hwmon_dir, name_fd; struct io io; + char buf2[128]; =20 if (class_hwmon_ent->d_type !=3D DT_LNK) continue; @@ -730,12 +741,13 @@ int perf_pmus__read_hwmon_pmus(struct list_head *pmus) close(hwmon_dir); continue; } - io__init(&io, name_fd, buf, sizeof(buf)); + io__init(&io, name_fd, buf2, sizeof(buf2)); io__getline(&io, &line, &line_len); if (line_len > 0 && line[line_len - 1] =3D=3D '\n') line[line_len - 1] =3D '\0'; - hwmon_pmu__new(pmus, hwmon_dir, class_hwmon_ent->d_name, line); + hwmon_pmu__new(pmus, buf, class_hwmon_ent->d_name, line); close(name_fd); + close(hwmon_dir); } free(line); close(class_hwmon_dir.dirfd); @@ -753,6 +765,10 @@ int evsel__hwmon_pmu_open(struct evsel *evsel, .type_and_num =3D evsel->core.attr.config, }; int idx =3D 0, thread =3D 0, nthreads, err =3D 0; + int dir =3D open(hwm->hwmon_dir, O_CLOEXEC | O_DIRECTORY | O_RDONLY); + + if (dir < 0) + return -errno; =20 nthreads =3D perf_thread_map__nr(threads); for (idx =3D start_cpu_map_idx; idx < end_cpu_map_idx; idx++) { @@ -763,7 +779,7 @@ int evsel__hwmon_pmu_open(struct evsel *evsel, snprintf(buf, sizeof(buf), "%s%d_input", hwmon_type_strs[key.type], key.num); =20 - fd =3D openat(hwm->hwmon_dir_fd, buf, O_RDONLY); + fd =3D openat(dir, buf, O_RDONLY); FD(evsel, idx, thread) =3D fd; if (fd < 0) { err =3D -errno; @@ -771,6 +787,7 @@ int evsel__hwmon_pmu_open(struct evsel *evsel, } } } + close(dir); return 0; out_close: if (err) @@ -784,6 +801,7 @@ int evsel__hwmon_pmu_open(struct evsel *evsel, } thread =3D nthreads; } while (--idx >=3D 0); + close(dir); return err; } =20 diff --git a/tools/perf/util/hwmon_pmu.h b/tools/perf/util/hwmon_pmu.h index b3329774d2b2..dc711b289ff5 100644 --- a/tools/perf/util/hwmon_pmu.h +++ b/tools/perf/util/hwmon_pmu.h @@ -135,14 +135,14 @@ bool parse_hwmon_filename(const char *filename, * hwmon_pmu__new() - Allocate and construct a hwmon PMU. * * @pmus: The list of PMUs to be added to. - * @hwmon_dir: An O_DIRECTORY file descriptor for a hwmon directory. + * @hwmon_dir: The path to a hwmon directory. * @sysfs_name: Name of the hwmon sysfs directory like hwmon0. * @name: The contents of the "name" file in the hwmon directory. * * Exposed for testing. Regular construction should happen via * perf_pmus__read_hwmon_pmus. */ -struct perf_pmu *hwmon_pmu__new(struct list_head *pmus, int hwmon_dir, +struct perf_pmu *hwmon_pmu__new(struct list_head *pmus, const char *hwmon_= dir, const char *sysfs_name, const char *name); void hwmon_pmu__exit(struct perf_pmu *pmu); =20 diff --git a/tools/perf/util/pmus.c b/tools/perf/util/pmus.c index 3bbd26fec78a..eed8d8005cff 100644 --- a/tools/perf/util/pmus.c +++ b/tools/perf/util/pmus.c @@ -762,7 +762,7 @@ struct perf_pmu *perf_pmus__add_test_pmu(int test_sysfs= _dirfd, const char *name) return perf_pmu__lookup(&other_pmus, test_sysfs_dirfd, name, /*eager_load= =3D*/true); } =20 -struct perf_pmu *perf_pmus__add_test_hwmon_pmu(int hwmon_dir, +struct perf_pmu *perf_pmus__add_test_hwmon_pmu(const char *hwmon_dir, const char *sysfs_name, const char *name) { diff --git a/tools/perf/util/pmus.h b/tools/perf/util/pmus.h index 8def20e615ad..d5140804a630 100644 --- a/tools/perf/util/pmus.h +++ b/tools/perf/util/pmus.h @@ -29,7 +29,7 @@ int perf_pmus__num_core_pmus(void); bool perf_pmus__supports_extended_type(void); =20 struct perf_pmu *perf_pmus__add_test_pmu(int test_sysfs_dirfd, const char = *name); -struct perf_pmu *perf_pmus__add_test_hwmon_pmu(int hwmon_dir, +struct perf_pmu *perf_pmus__add_test_hwmon_pmu(const char *hwmon_dir, const char *sysfs_name, const char *name); struct perf_pmu *perf_pmus__fake_pmu(void); --=20 2.50.0.714.g196bf9f422-goog