From: "Sapkal, Swapnil" <swapnil.sapkal@amd.com>
To: Namhyung Kim <namhyung@kernel.org>,
Ravi Bangoria <ravi.bangoria@amd.com>
Cc: <peterz@infradead.org>, <mingo@redhat.com>, <acme@kernel.org>,
<irogers@google.com>, <yu.c.chen@intel.com>,
<mark.rutland@arm.com>, <alexander.shishkin@linux.intel.com>,
<jolsa@kernel.org>, <rostedt@goodmis.org>,
<vincent.guittot@linaro.org>, <bristot@redhat.com>,
<adrian.hunter@intel.com>, <james.clark@arm.com>,
<kan.liang@linux.intel.com>, <gautham.shenoy@amd.com>,
<kprateek.nayak@amd.com>, <juri.lelli@redhat.com>,
<yangjihong@bytedance.com>, <void@manifault.com>, <tj@kernel.org>,
<linux-kernel@vger.kernel.org>,
<linux-perf-users@vger.kernel.org>, <santosh.shukla@amd.com>,
<ananth.narayan@amd.com>, <sandipan.das@amd.com>
Subject: Re: [PATCH 3/5] perf sched stats: Add schedstat v16 support
Date: Fri, 27 Sep 2024 16:38:04 +0530 [thread overview]
Message-ID: <438e185b-b1dc-4a34-ae09-667a706aa1a3@amd.com> (raw)
In-Reply-To: <ZvT74_Sp1bQZvqbO@google.com>
Hello Namhyung,
On 9/26/2024 11:44 AM, Namhyung Kim wrote:
> On Mon, Sep 16, 2024 at 04:47:20PM +0000, Ravi Bangoria wrote:
>> From: Swapnil Sapkal <swapnil.sapkal@amd.com>
>>
>> /proc/schedstat file output is standardized with version number.
>> Add support to record and raw dump v16 version layout.
>
> How many difference between v15 and v16? Can we have it in the same
> file with a different version number?
There is difference in ordering in domain fields between v15 and v16,
busy and idle load balancing fields are interchanged. Furthermore if new
fields are added, the parser breaks and maintaning a separate header
file seemed cleaner.
It will be difficult to define the perf structs, if the fields are
present in same header file.
If you any suggestion to handle this, I am all ears.
--
Thanks and Regards,
Swapnil
>
> Thanks,
> Namhyung
>
>>
>> Signed-off-by: Swapnil Sapkal <swapnil.sapkal@amd.com>
>> Co-developed-by: Ravi Bangoria <ravi.bangoria@amd.com>
>> Signed-off-by: Ravi Bangoria <ravi.bangoria@amd.com>
>> ---
>> tools/lib/perf/Makefile | 2 +-
>> tools/lib/perf/include/perf/event.h | 14 +++++++
>> .../lib/perf/include/perf/schedstat-cpu-v16.h | 13 ++++++
>> .../perf/include/perf/schedstat-domain-v16.h | 40 +++++++++++++++++++
>> tools/perf/util/event.c | 6 +++
>> tools/perf/util/synthetic-events.c | 6 +++
>> 6 files changed, 80 insertions(+), 1 deletion(-)
>> create mode 100644 tools/lib/perf/include/perf/schedstat-cpu-v16.h
>> create mode 100644 tools/lib/perf/include/perf/schedstat-domain-v16.h
>>
>> diff --git a/tools/lib/perf/Makefile b/tools/lib/perf/Makefile
>> index ebbfea891a6a..de0f4ffd9e16 100644
>> --- a/tools/lib/perf/Makefile
>> +++ b/tools/lib/perf/Makefile
>> @@ -187,7 +187,7 @@ install_lib: libs
>> $(call do_install_mkdir,$(libdir_SQ)); \
>> cp -fpR $(LIBPERF_ALL) $(DESTDIR)$(libdir_SQ)
>>
>> -HDRS := bpf_perf.h core.h cpumap.h threadmap.h evlist.h evsel.h event.h mmap.h schedstat-cpu-v15.h schedstat-domain-v15.h
>> +HDRS := bpf_perf.h core.h cpumap.h threadmap.h evlist.h evsel.h event.h mmap.h schedstat-cpu-v15.h schedstat-domain-v15.h schedstat-cpu-v16.h schedstat-domain-v16.h
>> INTERNAL_HDRS := cpumap.h evlist.h evsel.h lib.h mmap.h rc_check.h threadmap.h xyarray.h
>>
>> INSTALL_HDRS_PFX := $(DESTDIR)$(prefix)/include/perf
>> diff --git a/tools/lib/perf/include/perf/event.h b/tools/lib/perf/include/perf/event.h
>> index 35be296d68d5..c332d467c9c9 100644
>> --- a/tools/lib/perf/include/perf/event.h
>> +++ b/tools/lib/perf/include/perf/event.h
>> @@ -463,6 +463,12 @@ struct perf_record_schedstat_cpu_v15 {
>> #undef CPU_FIELD
>> };
>>
>> +struct perf_record_schedstat_cpu_v16 {
>> +#define CPU_FIELD(_type, _name, _ver) _type _name;
>> +#include "schedstat-cpu-v16.h"
>> +#undef CPU_FIELD
>> +};
>> +
>> struct perf_record_schedstat_cpu {
>> struct perf_event_header header;
>> __u16 version;
>> @@ -470,6 +476,7 @@ struct perf_record_schedstat_cpu {
>> __u32 cpu;
>> union {
>> struct perf_record_schedstat_cpu_v15 v15;
>> + struct perf_record_schedstat_cpu_v16 v16;
>> };
>> };
>>
>> @@ -479,6 +486,12 @@ struct perf_record_schedstat_domain_v15 {
>> #undef DOMAIN_FIELD
>> };
>>
>> +struct perf_record_schedstat_domain_v16 {
>> +#define DOMAIN_FIELD(_type, _name, _ver) _type _name;
>> +#include "schedstat-domain-v16.h"
>> +#undef DOMAIN_FIELD
>> +};
>> +
>> #define DOMAIN_NAME_LEN 16
>>
>> struct perf_record_schedstat_domain {
>> @@ -490,6 +503,7 @@ struct perf_record_schedstat_domain {
>> char name[DOMAIN_NAME_LEN];
>> union {
>> struct perf_record_schedstat_domain_v15 v15;
>> + struct perf_record_schedstat_domain_v16 v16;
>> };
>> __u16 nr_cpus;
>> __u8 cpu_mask[];
>> diff --git a/tools/lib/perf/include/perf/schedstat-cpu-v16.h b/tools/lib/perf/include/perf/schedstat-cpu-v16.h
>> new file mode 100644
>> index 000000000000..f3a55131a05a
>> --- /dev/null
>> +++ b/tools/lib/perf/include/perf/schedstat-cpu-v16.h
>> @@ -0,0 +1,13 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +
>> +#ifdef CPU_FIELD
>> +CPU_FIELD(__u32, yld_count, v16)
>> +CPU_FIELD(__u32, array_exp, v16)
>> +CPU_FIELD(__u32, sched_count, v16)
>> +CPU_FIELD(__u32, sched_goidle, v16)
>> +CPU_FIELD(__u32, ttwu_count, v16)
>> +CPU_FIELD(__u32, ttwu_local, v16)
>> +CPU_FIELD(__u64, rq_cpu_time, v16)
>> +CPU_FIELD(__u64, run_delay, v16)
>> +CPU_FIELD(__u64, pcount, v16)
>> +#endif /* CPU_FIELD */
>> diff --git a/tools/lib/perf/include/perf/schedstat-domain-v16.h b/tools/lib/perf/include/perf/schedstat-domain-v16.h
>> new file mode 100644
>> index 000000000000..d6ef895c9d32
>> --- /dev/null
>> +++ b/tools/lib/perf/include/perf/schedstat-domain-v16.h
>> @@ -0,0 +1,40 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +
>> +#ifdef DOMAIN_FIELD
>> +DOMAIN_FIELD(__u32, busy_lb_count, v16)
>> +DOMAIN_FIELD(__u32, busy_lb_balanced, v16)
>> +DOMAIN_FIELD(__u32, busy_lb_failed, v16)
>> +DOMAIN_FIELD(__u32, busy_lb_imbalance, v16)
>> +DOMAIN_FIELD(__u32, busy_lb_gained, v16)
>> +DOMAIN_FIELD(__u32, busy_lb_hot_gained, v16)
>> +DOMAIN_FIELD(__u32, busy_lb_nobusyq, v16)
>> +DOMAIN_FIELD(__u32, busy_lb_nobusyg, v16)
>> +DOMAIN_FIELD(__u32, idle_lb_count, v16)
>> +DOMAIN_FIELD(__u32, idle_lb_balanced, v16)
>> +DOMAIN_FIELD(__u32, idle_lb_failed, v16)
>> +DOMAIN_FIELD(__u32, idle_lb_imbalance, v16)
>> +DOMAIN_FIELD(__u32, idle_lb_gained, v16)
>> +DOMAIN_FIELD(__u32, idle_lb_hot_gained, v16)
>> +DOMAIN_FIELD(__u32, idle_lb_nobusyq, v16)
>> +DOMAIN_FIELD(__u32, idle_lb_nobusyg, v16)
>> +DOMAIN_FIELD(__u32, newidle_lb_count, v16)
>> +DOMAIN_FIELD(__u32, newidle_lb_balanced, v16)
>> +DOMAIN_FIELD(__u32, newidle_lb_failed, v16)
>> +DOMAIN_FIELD(__u32, newidle_lb_imbalance, v16)
>> +DOMAIN_FIELD(__u32, newidle_lb_gained, v16)
>> +DOMAIN_FIELD(__u32, newidle_lb_hot_gained, v16)
>> +DOMAIN_FIELD(__u32, newidle_lb_nobusyq, v16)
>> +DOMAIN_FIELD(__u32, newidle_lb_nobusyg, v16)
>> +DOMAIN_FIELD(__u32, alb_count, v16)
>> +DOMAIN_FIELD(__u32, alb_failed, v16)
>> +DOMAIN_FIELD(__u32, alb_pushed, v16)
>> +DOMAIN_FIELD(__u32, sbe_count, v16)
>> +DOMAIN_FIELD(__u32, sbe_balanced, v16)
>> +DOMAIN_FIELD(__u32, sbe_pushed, v16)
>> +DOMAIN_FIELD(__u32, sbf_count, v16)
>> +DOMAIN_FIELD(__u32, sbf_balanced, v16)
>> +DOMAIN_FIELD(__u32, sbf_pushed, v16)
>> +DOMAIN_FIELD(__u32, ttwu_wake_remote, v16)
>> +DOMAIN_FIELD(__u32, ttwu_move_affine, v16)
>> +DOMAIN_FIELD(__u32, ttwu_move_balance, v16)
>> +#endif /* DOMAIN_FIELD */
>> diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
>> index c9bc8237e3fa..d138e4a5787c 100644
>> --- a/tools/perf/util/event.c
>> +++ b/tools/perf/util/event.c
>> @@ -566,6 +566,9 @@ size_t perf_event__fprintf_schedstat_cpu(union perf_event *event, FILE *fp)
>> if (version == 15) {
>> #include <perf/schedstat-cpu-v15.h>
>> return size;
>> + } else if (version == 16) {
>> +#include <perf/schedstat-cpu-v16.h>
>> + return size;
>> }
>> #undef CPU_FIELD
>>
>> @@ -641,6 +644,9 @@ size_t perf_event__fprintf_schedstat_domain(union perf_event *event, FILE *fp)
>> if (version == 15) {
>> #include <perf/schedstat-domain-v15.h>
>> return size;
>> + } else if (version == 16) {
>> +#include <perf/schedstat-domain-v16.h>
>> + return size;
>> }
>> #undef DOMAIN_FIELD
>>
>> diff --git a/tools/perf/util/synthetic-events.c b/tools/perf/util/synthetic-events.c
>> index 9d8450b6eda9..73b2492a4cde 100644
>> --- a/tools/perf/util/synthetic-events.c
>> +++ b/tools/perf/util/synthetic-events.c
>> @@ -2546,6 +2546,8 @@ static union perf_event *__synthesize_schedstat_cpu(struct io *io, __u16 version
>>
>> if (version == 15) {
>> #include <perf/schedstat-cpu-v15.h>
>> + } else if (version == 16) {
>> +#include <perf/schedstat-cpu-v16.h>
>> }
>> #undef CPU_FIELD
>>
>> @@ -2667,6 +2669,8 @@ static union perf_event *__synthesize_schedstat_domain(struct io *io, __u16 vers
>>
>> if (version == 15) {
>> #include <perf/schedstat-domain-v15.h>
>> + } else if (version == 16) {
>> +#include <perf/schedstat-domain-v16.h>
>> }
>> #undef DOMAIN_FIELD
>>
>> @@ -2709,6 +2713,8 @@ int perf_event__synthesize_schedstat(const struct perf_tool *tool,
>>
>> if (!strcmp(line, "version 15\n")) {
>> version = 15;
>> + } else if (!strcmp(line, "version 16\n")) {
>> + version = 16;
>> } else {
>> pr_err("Unsupported /proc/schedstat version: %s", line + 8);
>> goto out_free_line;
>> --
>> 2.46.0
>>
next prev parent reply other threads:[~2024-09-27 11:08 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-09-16 16:47 [PATCH 0/5] perf sched: Introduce stats tool Ravi Bangoria
2024-09-16 16:47 ` [PATCH 1/5] sched/stats: Print domain name in /proc/schedstat Ravi Bangoria
2024-09-16 16:47 ` [PATCH 2/5] perf sched stats: Add record and rawdump support Ravi Bangoria
2024-09-17 10:35 ` James Clark
2024-09-18 8:52 ` Sapkal, Swapnil
2024-09-26 6:12 ` Namhyung Kim
2024-09-27 11:04 ` Sapkal, Swapnil
2024-09-16 16:47 ` [PATCH 3/5] perf sched stats: Add schedstat v16 support Ravi Bangoria
2024-09-26 6:14 ` Namhyung Kim
2024-09-27 11:08 ` Sapkal, Swapnil [this message]
2024-09-16 16:47 ` [PATCH 4/5] perf sched stats: Add support for report subcommand Ravi Bangoria
2024-09-16 16:47 ` [PATCH 5/5] perf sched stats: Add support for live mode Ravi Bangoria
2024-09-17 10:35 ` [PATCH 0/5] perf sched: Introduce stats tool James Clark
2024-09-18 13:19 ` Sapkal, Swapnil
2024-09-17 10:57 ` Madadi Vineeth Reddy
2024-09-18 8:43 ` Sapkal, Swapnil
2024-09-18 8:45 ` Sapkal, Swapnil
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=438e185b-b1dc-4a34-ae09-667a706aa1a3@amd.com \
--to=swapnil.sapkal@amd.com \
--cc=acme@kernel.org \
--cc=adrian.hunter@intel.com \
--cc=alexander.shishkin@linux.intel.com \
--cc=ananth.narayan@amd.com \
--cc=bristot@redhat.com \
--cc=gautham.shenoy@amd.com \
--cc=irogers@google.com \
--cc=james.clark@arm.com \
--cc=jolsa@kernel.org \
--cc=juri.lelli@redhat.com \
--cc=kan.liang@linux.intel.com \
--cc=kprateek.nayak@amd.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-perf-users@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=mingo@redhat.com \
--cc=namhyung@kernel.org \
--cc=peterz@infradead.org \
--cc=ravi.bangoria@amd.com \
--cc=rostedt@goodmis.org \
--cc=sandipan.das@amd.com \
--cc=santosh.shukla@amd.com \
--cc=tj@kernel.org \
--cc=vincent.guittot@linaro.org \
--cc=void@manifault.com \
--cc=yangjihong@bytedance.com \
--cc=yu.c.chen@intel.com \
/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: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox