From: Arnaldo Carvalho de Melo <acme@kernel.org> To: German Gomez <german.gomez@arm.com> Cc: linux-kernel@vger.kernel.org, linux-perf-users@vger.kernel.org, Mike Leach <mike.leach@linaro.org>, Leo Yan <leo.yan@linaro.org>, John Garry <john.garry@huawei.com>, Will Deacon <will@kernel.org>, Mathieu Poirier <mathieu.poirier@linaro.org>, Mark Rutland <mark.rutland@arm.com>, Alexander Shishkin <alexander.shishkin@linux.intel.com>, Jiri Olsa <jolsa@kernel.org>, Namhyung Kim <namhyung@kernel.org>, coresight@lists.linaro.org, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH 1/4] perf pmu: Add function to check if a pmu file exists Date: Tue, 10 May 2022 13:41:14 -0300 [thread overview] Message-ID: <YnqVqq5QW/b14oPZ@kernel.org> (raw) In-Reply-To: <20220504150216.581281-2-german.gomez@arm.com> Em Wed, May 04, 2022 at 04:02:12PM +0100, German Gomez escreveu: > Add a utility function perf_pmu__file_exists() to check if a given pmu > file exists in the sysfs filesystem. While reviewing this I noticed: int perf_pmu__caps_parse(struct perf_pmu *pmu) { struct stat st; char caps_path[PATH_MAX]; const char *sysfs = sysfs__mountpoint(); DIR *caps_dir; struct dirent *evt_ent; int nr_caps = 0; if (!sysfs) return -1; snprintf(caps_path, PATH_MAX, "%s" EVENT_SOURCE_DEVICE_PATH "%s/caps", sysfs, pmu->name); if (stat(caps_path, &st) < 0) return 0; /* no error if caps does not exist */ ------------------------ Shouldn't we introduce a: const int perf__pmu_pathname_scnprintf(struct perf_pmu *pmu, char *pathname, size_t size, char *filename) { return scnprintf(pathname, size, "%s" EVENT_SOURCE_DEVICE_PATH "%s/%s", sysfs, pmu->name, filename); } And use in your perf_pmu__file_exists() and in the other places where this open coded pattern appears? I'm waiting for reviews for the ARM specific bits. - Arnaldo > Signed-off-by: German Gomez <german.gomez@arm.com> > --- > tools/perf/util/pmu.c | 17 +++++++++++++++++ > tools/perf/util/pmu.h | 2 ++ > 2 files changed, 19 insertions(+) > > diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c > index 9a1c7e63e663..9479e9a4da54 100644 > --- a/tools/perf/util/pmu.c > +++ b/tools/perf/util/pmu.c > @@ -1854,6 +1854,23 @@ int perf_pmu__scan_file(struct perf_pmu *pmu, const char *name, const char *fmt, > return ret; > } > > +bool perf_pmu__file_exists(struct perf_pmu *pmu, const char *name) > +{ > + char path[PATH_MAX]; > + struct stat statbuf; > + const char *sysfs = sysfs__mountpoint(); > + > + if (!sysfs) > + return false; > + > + snprintf(path, PATH_MAX, > + "%s" EVENT_SOURCE_DEVICE_PATH "%s/%s", sysfs, pmu->name, name); > + if (!file_available(path)) > + return false; > + > + return stat(path, &statbuf) == 0; > +} > + > static int perf_pmu__new_caps(struct list_head *list, char *name, char *value) > { > struct perf_pmu_caps *caps = zalloc(sizeof(*caps)); > diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h > index 541889fa9f9c..ab728eaf13b6 100644 > --- a/tools/perf/util/pmu.h > +++ b/tools/perf/util/pmu.h > @@ -120,6 +120,8 @@ bool pmu_have_event(const char *pname, const char *name); > > int perf_pmu__scan_file(struct perf_pmu *pmu, const char *name, const char *fmt, ...) __scanf(3, 4); > > +bool perf_pmu__file_exists(struct perf_pmu *pmu, const char *name); > + > int perf_pmu__test(void); > > struct perf_event_attr *perf_pmu__get_default_config(struct perf_pmu *pmu); > -- > 2.25.1 -- - Arnaldo
WARNING: multiple messages have this Message-ID (diff)
From: Arnaldo Carvalho de Melo <acme@kernel.org> To: German Gomez <german.gomez@arm.com> Cc: linux-kernel@vger.kernel.org, linux-perf-users@vger.kernel.org, Mike Leach <mike.leach@linaro.org>, Leo Yan <leo.yan@linaro.org>, John Garry <john.garry@huawei.com>, Will Deacon <will@kernel.org>, Mathieu Poirier <mathieu.poirier@linaro.org>, Mark Rutland <mark.rutland@arm.com>, Alexander Shishkin <alexander.shishkin@linux.intel.com>, Jiri Olsa <jolsa@kernel.org>, Namhyung Kim <namhyung@kernel.org>, coresight@lists.linaro.org, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH 1/4] perf pmu: Add function to check if a pmu file exists Date: Tue, 10 May 2022 13:41:14 -0300 [thread overview] Message-ID: <YnqVqq5QW/b14oPZ@kernel.org> (raw) In-Reply-To: <20220504150216.581281-2-german.gomez@arm.com> Em Wed, May 04, 2022 at 04:02:12PM +0100, German Gomez escreveu: > Add a utility function perf_pmu__file_exists() to check if a given pmu > file exists in the sysfs filesystem. While reviewing this I noticed: int perf_pmu__caps_parse(struct perf_pmu *pmu) { struct stat st; char caps_path[PATH_MAX]; const char *sysfs = sysfs__mountpoint(); DIR *caps_dir; struct dirent *evt_ent; int nr_caps = 0; if (!sysfs) return -1; snprintf(caps_path, PATH_MAX, "%s" EVENT_SOURCE_DEVICE_PATH "%s/caps", sysfs, pmu->name); if (stat(caps_path, &st) < 0) return 0; /* no error if caps does not exist */ ------------------------ Shouldn't we introduce a: const int perf__pmu_pathname_scnprintf(struct perf_pmu *pmu, char *pathname, size_t size, char *filename) { return scnprintf(pathname, size, "%s" EVENT_SOURCE_DEVICE_PATH "%s/%s", sysfs, pmu->name, filename); } And use in your perf_pmu__file_exists() and in the other places where this open coded pattern appears? I'm waiting for reviews for the ARM specific bits. - Arnaldo > Signed-off-by: German Gomez <german.gomez@arm.com> > --- > tools/perf/util/pmu.c | 17 +++++++++++++++++ > tools/perf/util/pmu.h | 2 ++ > 2 files changed, 19 insertions(+) > > diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c > index 9a1c7e63e663..9479e9a4da54 100644 > --- a/tools/perf/util/pmu.c > +++ b/tools/perf/util/pmu.c > @@ -1854,6 +1854,23 @@ int perf_pmu__scan_file(struct perf_pmu *pmu, const char *name, const char *fmt, > return ret; > } > > +bool perf_pmu__file_exists(struct perf_pmu *pmu, const char *name) > +{ > + char path[PATH_MAX]; > + struct stat statbuf; > + const char *sysfs = sysfs__mountpoint(); > + > + if (!sysfs) > + return false; > + > + snprintf(path, PATH_MAX, > + "%s" EVENT_SOURCE_DEVICE_PATH "%s/%s", sysfs, pmu->name, name); > + if (!file_available(path)) > + return false; > + > + return stat(path, &statbuf) == 0; > +} > + > static int perf_pmu__new_caps(struct list_head *list, char *name, char *value) > { > struct perf_pmu_caps *caps = zalloc(sizeof(*caps)); > diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h > index 541889fa9f9c..ab728eaf13b6 100644 > --- a/tools/perf/util/pmu.h > +++ b/tools/perf/util/pmu.h > @@ -120,6 +120,8 @@ bool pmu_have_event(const char *pname, const char *name); > > int perf_pmu__scan_file(struct perf_pmu *pmu, const char *name, const char *fmt, ...) __scanf(3, 4); > > +bool perf_pmu__file_exists(struct perf_pmu *pmu, const char *name); > + > int perf_pmu__test(void); > > struct perf_event_attr *perf_pmu__get_default_config(struct perf_pmu *pmu); > -- > 2.25.1 -- - Arnaldo _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2022-05-10 16:41 UTC|newest] Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-05-04 15:02 [PATCH 0/4] perf cs_etm: Basic support for virtual/kernel timestamps German Gomez 2022-05-04 15:02 ` German Gomez 2022-05-04 15:02 ` [PATCH 1/4] perf pmu: Add function to check if a pmu file exists German Gomez 2022-05-04 15:02 ` German Gomez 2022-05-10 16:41 ` Arnaldo Carvalho de Melo [this message] 2022-05-10 16:41 ` Arnaldo Carvalho de Melo 2022-05-11 11:49 ` German Gomez 2022-05-11 11:49 ` German Gomez 2022-05-04 15:02 ` [PATCH 2/4] perf cs_etm: Keep separate symbols for ETMv4 and ETE parameters German Gomez 2022-05-04 15:02 ` German Gomez 2022-05-04 15:02 ` [PATCH 3/4] perf cs_etm: Record ts_source in AUXTRACE_INFO for ETMv4 and ETE German Gomez 2022-05-04 15:02 ` German Gomez 2022-05-04 15:02 ` [PATCH 4/4] perf cs_etm: Set the time field in the synthetic samples German Gomez 2022-05-04 15:02 ` German Gomez 2022-05-05 9:56 ` [PATCH 0/4] perf cs_etm: Basic support for virtual/kernel timestamps Mike Leach 2022-05-05 9:56 ` Mike Leach
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=YnqVqq5QW/b14oPZ@kernel.org \ --to=acme@kernel.org \ --cc=alexander.shishkin@linux.intel.com \ --cc=coresight@lists.linaro.org \ --cc=german.gomez@arm.com \ --cc=john.garry@huawei.com \ --cc=jolsa@kernel.org \ --cc=leo.yan@linaro.org \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-perf-users@vger.kernel.org \ --cc=mark.rutland@arm.com \ --cc=mathieu.poirier@linaro.org \ --cc=mike.leach@linaro.org \ --cc=namhyung@kernel.org \ --cc=will@kernel.org \ /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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.