lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<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

Powered by Openwall GNU/*/Linux Powered by OpenVZ