[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <438e185b-b1dc-4a34-ae09-667a706aa1a3@amd.com>
Date: Fri, 27 Sep 2024 16:38:04 +0530
From: "Sapkal, Swapnil" <swapnil.sapkal@....com>
To: Namhyung Kim <namhyung@...nel.org>, Ravi Bangoria <ravi.bangoria@....com>
CC: <peterz@...radead.org>, <mingo@...hat.com>, <acme@...nel.org>,
<irogers@...gle.com>, <yu.c.chen@...el.com>, <mark.rutland@....com>,
<alexander.shishkin@...ux.intel.com>, <jolsa@...nel.org>,
<rostedt@...dmis.org>, <vincent.guittot@...aro.org>, <bristot@...hat.com>,
<adrian.hunter@...el.com>, <james.clark@....com>,
<kan.liang@...ux.intel.com>, <gautham.shenoy@....com>,
<kprateek.nayak@....com>, <juri.lelli@...hat.com>,
<yangjihong@...edance.com>, <void@...ifault.com>, <tj@...nel.org>,
<linux-kernel@...r.kernel.org>, <linux-perf-users@...r.kernel.org>,
<santosh.shukla@....com>, <ananth.narayan@....com>, <sandipan.das@....com>
Subject: Re: [PATCH 3/5] perf sched stats: Add schedstat v16 support
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@....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@....com>
>> Co-developed-by: Ravi Bangoria <ravi.bangoria@....com>
>> Signed-off-by: Ravi Bangoria <ravi.bangoria@....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
>>
Powered by blists - more mailing lists