From 3a546a67a4cc251b4ec2ba02d8f7aacf0bfc3148 Mon Sep 17 00:00:00 2001 From: Eder Zulian Date: Tue, 13 Aug 2024 16:23:38 +0200 Subject: [PATCH 01/20] rtla: use the definition for stdout fd when calling isatty() Use the STDOUT_FILENO definition when testing whether the standard output file descriptor refers to a terminal (for better redability). Link: https://lore.kernel.org/20240813142338.376039-1-ezulian@redhat.com Signed-off-by: Eder Zulian Signed-off-by: Steven Rostedt (Google) --- tools/tracing/rtla/src/osnoise_top.c | 2 +- tools/tracing/rtla/src/timerlat_top.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/tracing/rtla/src/osnoise_top.c b/tools/tracing/rtla/src/osnoise_top.c index 2f756628613d..66e3a4382bc2 100644 --- a/tools/tracing/rtla/src/osnoise_top.c +++ b/tools/tracing/rtla/src/osnoise_top.c @@ -627,7 +627,7 @@ osnoise_top_apply_config(struct osnoise_tool *tool, struct osnoise_top_params *p auto_house_keeping(¶ms->monitored_cpus); } - if (isatty(1) && !params->quiet) + if (isatty(STDOUT_FILENO) && !params->quiet) params->pretty_output = 1; return 0; diff --git a/tools/tracing/rtla/src/timerlat_top.c b/tools/tracing/rtla/src/timerlat_top.c index 8c16419fe22a..94a2f5bbaeb7 100644 --- a/tools/tracing/rtla/src/timerlat_top.c +++ b/tools/tracing/rtla/src/timerlat_top.c @@ -850,7 +850,7 @@ timerlat_top_apply_config(struct osnoise_tool *top, struct timerlat_top_params * } } - if (isatty(1) && !params->quiet) + if (isatty(STDOUT_FILENO) && !params->quiet) params->pretty_output = 1; return 0; From f88b8871c70f10d692cc2cfa1fb020c281dd7603 Mon Sep 17 00:00:00 2001 From: Ba Jing Date: Tue, 3 Sep 2024 07:34:08 +0800 Subject: [PATCH 02/20] tools/rv: Correct the grammatical errors in the comments The form of "print" should be consistent with "parses". Link: https://lore.kernel.org/20240902233408.8684-1-bajing@cmss.chinamobile.com Signed-off-by: Ba Jing Signed-off-by: Steven Rostedt (Google) --- tools/verification/rv/src/in_kernel.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/verification/rv/src/in_kernel.c b/tools/verification/rv/src/in_kernel.c index f04479ecc96c..f2bbc75a76f4 100644 --- a/tools/verification/rv/src/in_kernel.c +++ b/tools/verification/rv/src/in_kernel.c @@ -332,7 +332,7 @@ static void ikm_print_header(struct trace_seq *s) * ikm_event_handler - callback to handle event events * * Called any time a rv:"monitor"_event events is generated. - * It parses and print event. + * It parses and prints event. */ static int ikm_event_handler(struct trace_seq *s, struct tep_record *record, @@ -384,7 +384,7 @@ ikm_event_handler(struct trace_seq *s, struct tep_record *record, * ikm_error_handler - callback to handle error events * * Called any time a rv:"monitor"_errors events is generated. - * It parses and print event. + * It parses and prints event. */ static int ikm_error_handler(struct trace_seq *s, struct tep_record *record, From 1c5e11b3ee9c1a583a6630148ed50346e5bbb59b Mon Sep 17 00:00:00 2001 From: Ba Jing Date: Tue, 3 Sep 2024 08:30:19 +0800 Subject: [PATCH 03/20] tools/rv: Correct the grammatical errors in the comments The word "trace" begins with a consonant sound, so "a" should be used instead of "an". Link: https://lore.kernel.org/20240903003019.8969-1-bajing@cmss.chinamobile.com Signed-off-by: Ba Jing Signed-off-by: Steven Rostedt (Google) --- tools/verification/rv/src/trace.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/verification/rv/src/trace.c b/tools/verification/rv/src/trace.c index 2c7deed47f8d..1b9f9bfa1893 100644 --- a/tools/verification/rv/src/trace.c +++ b/tools/verification/rv/src/trace.c @@ -81,7 +81,7 @@ void trace_instance_destroy(struct trace_instance *trace) } /** - * trace_instance_init - create an trace instance + * trace_instance_init - create a trace instance * * It is more than the tracefs instance, as it contains other * things required for the tracing, such as the local events and From ac1987f8f525379a0677f7f23c7a7ef2596a338d Mon Sep 17 00:00:00 2001 From: Andrew Kreimer Date: Wed, 11 Sep 2024 14:43:38 +0300 Subject: [PATCH 04/20] rv: Fix a typo Fix a typo in comments. Cc: Masami Hiramatsu Cc: Mathieu Desnoyers Link: https://lore.kernel.org/20240911114349.20449-1-algonell@gmail.com Reported-by: Matthew Wilcox Signed-off-by: Andrew Kreimer Signed-off-by: Steven Rostedt (Google) --- kernel/trace/rv/rv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kernel/trace/rv/rv.c b/kernel/trace/rv/rv.c index dc819aec43e8..279c70e1bd74 100644 --- a/kernel/trace/rv/rv.c +++ b/kernel/trace/rv/rv.c @@ -41,7 +41,7 @@ * per-task monitor, and so on), and the helper functions that glue the * monitor to the system via trace. Generally, a monitor includes some form * of trace output as a reaction for event parsing and exceptions, - * as depicted bellow: + * as depicted below: * * Linux +----- RV Monitor ----------------------------------+ Formal * Realm | | Realm From cfb1ea216c1656a4112becbc4bf757891933b902 Mon Sep 17 00:00:00 2001 From: Gabriele Monaco Date: Thu, 26 Sep 2024 16:34:17 +0200 Subject: [PATCH 05/20] rtla: Fix consistency in getopt_long for timerlat_hist Commit e9a4062e1527 ("rtla: Add --trace-buffer-size option") adds a new long option to rtla utilities, but among all affected files, timerlat_hist misses a trailing `:` in the corresponding short option inside the getopt string (e.g. `\3:`). This patch propagates the `:`. Although this change is not functionally required, it improves consistency and slightly reduces the likelihood a future change would introduce a problem. Cc: John Kacur Cc: Luis Goncalves Cc: Tomas Glozar Link: https://lore.kernel.org/20240926143417.54039-1-gmonaco@redhat.com Signed-off-by: Gabriele Monaco Signed-off-by: Steven Rostedt (Google) --- tools/tracing/rtla/src/timerlat_hist.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/tracing/rtla/src/timerlat_hist.c b/tools/tracing/rtla/src/timerlat_hist.c index a3907c390d67..1f9137c592f4 100644 --- a/tools/tracing/rtla/src/timerlat_hist.c +++ b/tools/tracing/rtla/src/timerlat_hist.c @@ -778,7 +778,7 @@ static struct timerlat_hist_params /* getopt_long stores the option index here. */ int option_index = 0; - c = getopt_long(argc, argv, "a:c:C::b:d:e:E:DhH:i:knp:P:s:t::T:uU0123456:7:8:9\1\2:\3", + c = getopt_long(argc, argv, "a:c:C::b:d:e:E:DhH:i:knp:P:s:t::T:uU0123456:7:8:9\1\2:\3:", long_options, &option_index); /* detect the end of the options. */ From 099a84019b64406a83b8d4f9f63235dbc16ed5b8 Mon Sep 17 00:00:00 2001 From: Jan Stancek Date: Thu, 10 Oct 2024 11:32:58 +0200 Subject: [PATCH 06/20] tools/rtla: drop __NR_sched_getattr It's not used since commit 084ce16df0f0 ("tools/rtla: Remove unused sched_getattr() function"). Link: https://lore.kernel.org/c355dc9ad23470098d6a8d0f31fbd702551c9ea8.1728552769.git.jstancek@redhat.com Signed-off-by: Jan Stancek Reviewed-by: Tomas Glozar Signed-off-by: Steven Rostedt (Google) --- tools/tracing/rtla/src/utils.c | 32 ++++++++++++++------------------ 1 file changed, 14 insertions(+), 18 deletions(-) diff --git a/tools/tracing/rtla/src/utils.c b/tools/tracing/rtla/src/utils.c index 9ac71a66840c..05b2b3fc005e 100644 --- a/tools/tracing/rtla/src/utils.c +++ b/tools/tracing/rtla/src/utils.c @@ -211,24 +211,20 @@ long parse_ns_duration(char *val) /* * This is a set of helper functions to use SCHED_DEADLINE. */ -#ifdef __x86_64__ -# define __NR_sched_setattr 314 -# define __NR_sched_getattr 315 -#elif __i386__ -# define __NR_sched_setattr 351 -# define __NR_sched_getattr 352 -#elif __arm__ -# define __NR_sched_setattr 380 -# define __NR_sched_getattr 381 -#elif __aarch64__ || __riscv -# define __NR_sched_setattr 274 -# define __NR_sched_getattr 275 -#elif __powerpc__ -# define __NR_sched_setattr 355 -# define __NR_sched_getattr 356 -#elif __s390x__ -# define __NR_sched_setattr 345 -# define __NR_sched_getattr 346 +#ifndef __NR_sched_setattr +# ifdef __x86_64__ +# define __NR_sched_setattr 314 +# elif __i386__ +# define __NR_sched_setattr 351 +# elif __arm__ +# define __NR_sched_setattr 380 +# elif __aarch64__ || __riscv +# define __NR_sched_setattr 274 +# elif __powerpc__ +# define __NR_sched_setattr 355 +# elif __s390x__ +# define __NR_sched_setattr 345 +# endif #endif #define SCHED_DEADLINE 6 From 0eecee340672c4b512f6f4a8c6add26df05d130c Mon Sep 17 00:00:00 2001 From: Jan Stancek Date: Thu, 10 Oct 2024 17:09:48 +0200 Subject: [PATCH 07/20] tools/rtla: fix collision with glibc sched_attr/sched_set_attr MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit glibc commit 21571ca0d703 ("Linux: Add the sched_setattr and sched_getattr functions") now also provides 'struct sched_attr' and sched_setattr() which collide with the ones from rtla. In file included from src/trace.c:11: src/utils.h:49:8: error: redefinition of ‘struct sched_attr’ 49 | struct sched_attr { | ^~~~~~~~~~ In file included from /usr/include/bits/sched.h:60, from /usr/include/sched.h:43, from /usr/include/tracefs/tracefs.h:10, from src/trace.c:4: /usr/include/linux/sched/types.h:98:8: note: originally defined here 98 | struct sched_attr { | ^~~~~~~~~~ Define 'struct sched_attr' conditionally, similar to what strace did: https://lore.kernel.org/all/20240930222913.3981407-1-raj.khem@gmail.com/ and rename rtla's version of sched_setattr() to avoid collision. Link: https://lore.kernel.org/8088f66a7a57c1b209cd8ae0ae7c336a7f8c930d.1728572865.git.jstancek@redhat.com Signed-off-by: Jan Stancek Signed-off-by: Steven Rostedt (Google) --- tools/tracing/rtla/src/utils.c | 4 ++-- tools/tracing/rtla/src/utils.h | 2 ++ 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/tools/tracing/rtla/src/utils.c b/tools/tracing/rtla/src/utils.c index 05b2b3fc005e..6fae234aaf36 100644 --- a/tools/tracing/rtla/src/utils.c +++ b/tools/tracing/rtla/src/utils.c @@ -229,7 +229,7 @@ long parse_ns_duration(char *val) #define SCHED_DEADLINE 6 -static inline int sched_setattr(pid_t pid, const struct sched_attr *attr, +static inline int syscall_sched_setattr(pid_t pid, const struct sched_attr *attr, unsigned int flags) { return syscall(__NR_sched_setattr, pid, attr, flags); } @@ -239,7 +239,7 @@ int __set_sched_attr(int pid, struct sched_attr *attr) int flags = 0; int retval; - retval = sched_setattr(pid, attr, flags); + retval = syscall_sched_setattr(pid, attr, flags); if (retval < 0) { err_msg("Failed to set sched attributes to the pid %d: %s\n", pid, strerror(errno)); diff --git a/tools/tracing/rtla/src/utils.h b/tools/tracing/rtla/src/utils.h index d44513e6c66a..99c9cf81bcd0 100644 --- a/tools/tracing/rtla/src/utils.h +++ b/tools/tracing/rtla/src/utils.h @@ -46,6 +46,7 @@ update_sum(unsigned long long *a, unsigned long long *b) *a += *b; } +#ifndef SCHED_ATTR_SIZE_VER0 struct sched_attr { uint32_t size; uint32_t sched_policy; @@ -56,6 +57,7 @@ struct sched_attr { uint64_t sched_deadline; uint64_t sched_period; }; +#endif /* SCHED_ATTR_SIZE_VER0 */ int parse_prio(char *arg, struct sched_attr *sched_param); int parse_cpu_set(char *cpu_list, cpu_set_t *set); From 4eba4723c5254ba8251ecb7094a5078d5c300646 Mon Sep 17 00:00:00 2001 From: Tomas Glozar Date: Fri, 11 Oct 2024 14:10:14 +0200 Subject: [PATCH 08/20] rtla/timerlat: Make timerlat_top_cpu->*_count unsigned long long Most fields of struct timerlat_top_cpu are unsigned long long, but the fields {irq,thread,user}_count are int (32-bit signed). This leads to overflow when tracing on a large number of CPUs for a long enough time: $ rtla timerlat top -a20 -c 1-127 -d 12h ... 0 12:00:00 | IRQ Timer Latency (us) | Thread Timer Latency (us) CPU COUNT | cur min avg max | cur min avg max 1 #43200096 | 0 0 1 2 | 3 2 6 12 ... 127 #43200096 | 0 0 1 2 | 3 2 5 11 ALL #119144 e4 | 0 5 4 | 2 28 16 The average latency should be 0-1 for IRQ and 5-6 for thread, but is reported as 5 and 28, about 4 to 5 times more, due to the count overflowing when summed over all CPUs: 43200096 * 127 = 5486412192, however, 1191444898 (= 5486412192 mod MAX_INT) is reported instead, as seen on the last line of the output, and the averages are thus ~4.6 times higher than they should be (5486412192 / 1191444898 = ~4.6). Fix the issue by changing {irq,thread,user}_count fields to unsigned long long, similarly to other fields in struct timerlat_top_cpu and to the count variable in timerlat_top_print_sum. Link: https://lore.kernel.org/20241011121015.2868751-1-tglozar@redhat.com Reported-by: Attila Fazekas Signed-off-by: Tomas Glozar Signed-off-by: Steven Rostedt (Google) --- tools/tracing/rtla/src/timerlat_top.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tools/tracing/rtla/src/timerlat_top.c b/tools/tracing/rtla/src/timerlat_top.c index 94a2f5bbaeb7..7fb85c8ee3bc 100644 --- a/tools/tracing/rtla/src/timerlat_top.c +++ b/tools/tracing/rtla/src/timerlat_top.c @@ -54,9 +54,9 @@ struct timerlat_top_params { }; struct timerlat_top_cpu { - int irq_count; - int thread_count; - int user_count; + unsigned long long irq_count; + unsigned long long thread_count; + unsigned long long user_count; unsigned long long cur_irq; unsigned long long min_irq; @@ -280,7 +280,7 @@ static void timerlat_top_print(struct osnoise_tool *top, int cpu) /* * Unless trace is being lost, IRQ counter is always the max. */ - trace_seq_printf(s, "%3d #%-9d |", cpu, cpu_data->irq_count); + trace_seq_printf(s, "%3d #%-9llu |", cpu, cpu_data->irq_count); if (!cpu_data->irq_count) { trace_seq_printf(s, "%s %s %s %s |", no_value, no_value, no_value, no_value); From 76b3102148135945b013797fac9b206273f0f777 Mon Sep 17 00:00:00 2001 From: Tomas Glozar Date: Fri, 11 Oct 2024 14:10:15 +0200 Subject: [PATCH 09/20] rtla/timerlat: Make timerlat_hist_cpu->*_count unsigned long long Do the same fix as in previous commit also for timerlat-hist. Link: https://lore.kernel.org/20241011121015.2868751-2-tglozar@redhat.com Reported-by: Attila Fazekas Signed-off-by: Tomas Glozar Signed-off-by: Steven Rostedt (Google) --- tools/tracing/rtla/src/timerlat_hist.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/tools/tracing/rtla/src/timerlat_hist.c b/tools/tracing/rtla/src/timerlat_hist.c index 1f9137c592f4..d49c8f0855fe 100644 --- a/tools/tracing/rtla/src/timerlat_hist.c +++ b/tools/tracing/rtla/src/timerlat_hist.c @@ -62,9 +62,9 @@ struct timerlat_hist_cpu { int *thread; int *user; - int irq_count; - int thread_count; - int user_count; + unsigned long long irq_count; + unsigned long long thread_count; + unsigned long long user_count; unsigned long long min_irq; unsigned long long sum_irq; @@ -304,15 +304,15 @@ timerlat_print_summary(struct timerlat_hist_params *params, continue; if (!params->no_irq) - trace_seq_printf(trace->seq, "%9d ", + trace_seq_printf(trace->seq, "%9llu ", data->hist[cpu].irq_count); if (!params->no_thread) - trace_seq_printf(trace->seq, "%9d ", + trace_seq_printf(trace->seq, "%9llu ", data->hist[cpu].thread_count); if (params->user_hist) - trace_seq_printf(trace->seq, "%9d ", + trace_seq_printf(trace->seq, "%9llu ", data->hist[cpu].user_count); } trace_seq_printf(trace->seq, "\n"); @@ -488,15 +488,15 @@ timerlat_print_stats_all(struct timerlat_hist_params *params, trace_seq_printf(trace->seq, "count:"); if (!params->no_irq) - trace_seq_printf(trace->seq, "%9d ", + trace_seq_printf(trace->seq, "%9llu ", sum.irq_count); if (!params->no_thread) - trace_seq_printf(trace->seq, "%9d ", + trace_seq_printf(trace->seq, "%9llu ", sum.thread_count); if (params->user_hist) - trace_seq_printf(trace->seq, "%9d ", + trace_seq_printf(trace->seq, "%9llu ", sum.user_count); trace_seq_printf(trace->seq, "\n"); From 0f59a6c9c421a44e652353e3ec15cf2425b904fe Mon Sep 17 00:00:00 2001 From: Tomas Glozar Date: Thu, 17 Oct 2024 16:09:09 +0200 Subject: [PATCH 10/20] tools/build: Add libcpupower dependency detection Add the ability to detect the presence of libcpupower on a system to the Makefiles in tools/build. Link: https://lore.kernel.org/20241017140914.3200454-2-tglozar@redhat.com Signed-off-by: Tomas Glozar Signed-off-by: Steven Rostedt (Google) --- tools/build/Makefile.feature | 1 + tools/build/feature/Makefile | 4 ++++ tools/build/feature/test-libcpupower.c | 8 ++++++++ 3 files changed, 13 insertions(+) create mode 100644 tools/build/feature/test-libcpupower.c diff --git a/tools/build/Makefile.feature b/tools/build/Makefile.feature index ffd117135094..2ebfb826dcea 100644 --- a/tools/build/Makefile.feature +++ b/tools/build/Makefile.feature @@ -53,6 +53,7 @@ FEATURE_TESTS_BASIC := \ libslang-include-subdir \ libtraceevent \ libtracefs \ + libcpupower \ libcrypto \ libunwind \ pthread-attr-setaffinity-np \ diff --git a/tools/build/feature/Makefile b/tools/build/feature/Makefile index 5938cf799dc6..6ef3e1ca583e 100644 --- a/tools/build/feature/Makefile +++ b/tools/build/feature/Makefile @@ -38,6 +38,7 @@ FILES= \ test-libslang.bin \ test-libslang-include-subdir.bin \ test-libtraceevent.bin \ + test-libcpupower.bin \ test-libtracefs.bin \ test-libcrypto.bin \ test-libunwind.bin \ @@ -245,6 +246,9 @@ $(OUTPUT)test-libslang-include-subdir.bin: $(OUTPUT)test-libtraceevent.bin: $(BUILD) -ltraceevent +$(OUTPUT)test-libcpupower.bin: + $(BUILD) -lcpupower + $(OUTPUT)test-libtracefs.bin: $(BUILD) $(shell $(PKG_CONFIG) --cflags libtracefs 2>/dev/null) -ltracefs diff --git a/tools/build/feature/test-libcpupower.c b/tools/build/feature/test-libcpupower.c new file mode 100644 index 000000000000..a346aa332a71 --- /dev/null +++ b/tools/build/feature/test-libcpupower.c @@ -0,0 +1,8 @@ +// SPDX-License-Identifier: GPL-2.0 +#include + +int main(void) +{ + int rv = cpuidle_state_count(0); + return rv; +} From e2b48b226b84848972c4fde6066ed9ff1254463f Mon Sep 17 00:00:00 2001 From: Tomas Glozar Date: Thu, 17 Oct 2024 16:09:10 +0200 Subject: [PATCH 11/20] rtla: Add optional dependency on libcpupower If libcpupower is present, set HAVE_LIBCPUPOWER_SUPPORT macro to allow features depending on libcpupower in rtla. Link: https://lore.kernel.org/20241017140914.3200454-3-tglozar@redhat.com Signed-off-by: Tomas Glozar Signed-off-by: Steven Rostedt (Google) --- tools/tracing/rtla/Makefile | 2 ++ tools/tracing/rtla/Makefile.config | 10 ++++++++++ 2 files changed, 12 insertions(+) diff --git a/tools/tracing/rtla/Makefile b/tools/tracing/rtla/Makefile index b5878be36125..a6a7dee16622 100644 --- a/tools/tracing/rtla/Makefile +++ b/tools/tracing/rtla/Makefile @@ -32,8 +32,10 @@ DOCSRC := ../../../Documentation/tools/rtla/ FEATURE_TESTS := libtraceevent FEATURE_TESTS += libtracefs +FEATURE_TESTS += libcpupower FEATURE_DISPLAY := libtraceevent FEATURE_DISPLAY += libtracefs +FEATURE_DISPLAY += libcpupower ifeq ($(V),1) Q = diff --git a/tools/tracing/rtla/Makefile.config b/tools/tracing/rtla/Makefile.config index 5f8c286712d4..92a6e12e42d3 100644 --- a/tools/tracing/rtla/Makefile.config +++ b/tools/tracing/rtla/Makefile.config @@ -43,6 +43,16 @@ else $(info libtracefs is missing. Please install libtracefs-dev/libtracefs-devel) endif +$(call feature_check,libcpupower) +ifeq ($(feature-libcpupower), 1) + $(call detected,CONFIG_LIBCPUPOWER) + CFLAGS += -DHAVE_LIBCPUPOWER_SUPPORT + EXTLIBS += -lcpupower +else + $(info libcpupower is missing, building without --deepest-idle-state support.) + $(info Please install libcpupower-dev/kernel-tools-libs-devel) +endif + ifeq ($(STOP_ERROR),1) $(error Please, check the errors above.) endif From 083d29d3784319e9e9fab3ac02683a7b26ae3480 Mon Sep 17 00:00:00 2001 From: Tomas Glozar Date: Thu, 17 Oct 2024 16:09:11 +0200 Subject: [PATCH 12/20] rtla/utils: Add idle state disabling via libcpupower Add functions to utils.c to disable idle states through functions of libcpupower. This will serve as the basis for disabling idle states per cpu when running timerlat. Link: https://lore.kernel.org/20241017140914.3200454-4-tglozar@redhat.com Signed-off-by: Tomas Glozar Signed-off-by: Steven Rostedt (Google) --- tools/tracing/rtla/src/utils.c | 150 +++++++++++++++++++++++++++++++++ tools/tracing/rtla/src/utils.h | 13 +++ 2 files changed, 163 insertions(+) diff --git a/tools/tracing/rtla/src/utils.c b/tools/tracing/rtla/src/utils.c index 6fae234aaf36..4995d35cf3ec 100644 --- a/tools/tracing/rtla/src/utils.c +++ b/tools/tracing/rtla/src/utils.c @@ -4,6 +4,9 @@ */ #define _GNU_SOURCE +#ifdef HAVE_LIBCPUPOWER_SUPPORT +#include +#endif /* HAVE_LIBCPUPOWER_SUPPORT */ #include #include #include @@ -515,6 +518,153 @@ int set_cpu_dma_latency(int32_t latency) return fd; } +#ifdef HAVE_LIBCPUPOWER_SUPPORT +static unsigned int **saved_cpu_idle_disable_state; +static size_t saved_cpu_idle_disable_state_alloc_ctr; + +/* + * save_cpu_idle_state_disable - save disable for all idle states of a cpu + * + * Saves the current disable of all idle states of a cpu, to be subsequently + * restored via restore_cpu_idle_disable_state. + * + * Return: idle state count on success, negative on error + */ +int save_cpu_idle_disable_state(unsigned int cpu) +{ + unsigned int nr_states; + unsigned int state; + int disabled; + int nr_cpus; + + nr_states = cpuidle_state_count(cpu); + + if (nr_states == 0) + return 0; + + if (saved_cpu_idle_disable_state == NULL) { + nr_cpus = sysconf(_SC_NPROCESSORS_CONF); + saved_cpu_idle_disable_state = calloc(nr_cpus, sizeof(unsigned int *)); + if (!saved_cpu_idle_disable_state) + return -1; + } + + saved_cpu_idle_disable_state[cpu] = calloc(nr_states, sizeof(unsigned int)); + if (!saved_cpu_idle_disable_state[cpu]) + return -1; + saved_cpu_idle_disable_state_alloc_ctr++; + + for (state = 0; state < nr_states; state++) { + disabled = cpuidle_is_state_disabled(cpu, state); + if (disabled < 0) + return disabled; + saved_cpu_idle_disable_state[cpu][state] = disabled; + } + + return nr_states; +} + +/* + * restore_cpu_idle_disable_state - restore disable for all idle states of a cpu + * + * Restores the current disable state of all idle states of a cpu that was + * previously saved by save_cpu_idle_disable_state. + * + * Return: idle state count on success, negative on error + */ +int restore_cpu_idle_disable_state(unsigned int cpu) +{ + unsigned int nr_states; + unsigned int state; + int disabled; + int result; + + nr_states = cpuidle_state_count(cpu); + + if (nr_states == 0) + return 0; + + if (!saved_cpu_idle_disable_state) + return -1; + + for (state = 0; state < nr_states; state++) { + if (!saved_cpu_idle_disable_state[cpu]) + return -1; + disabled = saved_cpu_idle_disable_state[cpu][state]; + result = cpuidle_state_disable(cpu, state, disabled); + if (result < 0) + return result; + } + + free(saved_cpu_idle_disable_state[cpu]); + saved_cpu_idle_disable_state[cpu] = NULL; + saved_cpu_idle_disable_state_alloc_ctr--; + if (saved_cpu_idle_disable_state_alloc_ctr == 0) { + free(saved_cpu_idle_disable_state); + saved_cpu_idle_disable_state = NULL; + } + + return nr_states; +} + +/* + * free_cpu_idle_disable_states - free saved idle state disable for all cpus + * + * Frees the memory used for storing cpu idle state disable for all cpus + * and states. + * + * Normally, the memory is freed automatically in + * restore_cpu_idle_disable_state; this is mostly for cleaning up after an + * error. + */ +void free_cpu_idle_disable_states(void) +{ + int cpu; + int nr_cpus; + + if (!saved_cpu_idle_disable_state) + return; + + nr_cpus = sysconf(_SC_NPROCESSORS_CONF); + + for (cpu = 0; cpu < nr_cpus; cpu++) { + free(saved_cpu_idle_disable_state[cpu]); + saved_cpu_idle_disable_state[cpu] = NULL; + } + + free(saved_cpu_idle_disable_state); + saved_cpu_idle_disable_state = NULL; +} + +/* + * set_deepest_cpu_idle_state - limit idle state of cpu + * + * Disables all idle states deeper than the one given in + * deepest_state (assuming states with higher number are deeper). + * + * This is used to reduce the exit from idle latency. Unlike + * set_cpu_dma_latency, it can disable idle states per cpu. + * + * Return: idle state count on success, negative on error + */ +int set_deepest_cpu_idle_state(unsigned int cpu, unsigned int deepest_state) +{ + unsigned int nr_states; + unsigned int state; + int result; + + nr_states = cpuidle_state_count(cpu); + + for (state = deepest_state + 1; state < nr_states; state++) { + result = cpuidle_state_disable(cpu, state, 1); + if (result < 0) + return result; + } + + return nr_states; +} +#endif /* HAVE_LIBCPUPOWER_SUPPORT */ + #define _STR(x) #x #define STR(x) _STR(x) diff --git a/tools/tracing/rtla/src/utils.h b/tools/tracing/rtla/src/utils.h index 99c9cf81bcd0..101d4799a009 100644 --- a/tools/tracing/rtla/src/utils.h +++ b/tools/tracing/rtla/src/utils.h @@ -66,6 +66,19 @@ int set_comm_sched_attr(const char *comm_prefix, struct sched_attr *attr); int set_comm_cgroup(const char *comm_prefix, const char *cgroup); int set_pid_cgroup(pid_t pid, const char *cgroup); int set_cpu_dma_latency(int32_t latency); +#ifdef HAVE_LIBCPUPOWER_SUPPORT +int save_cpu_idle_disable_state(unsigned int cpu); +int restore_cpu_idle_disable_state(unsigned int cpu); +void free_cpu_idle_disable_states(void); +int set_deepest_cpu_idle_state(unsigned int cpu, unsigned int state); +static inline int have_libcpupower_support(void) { return 1; } +#else +static inline int save_cpu_idle_disable_state(unsigned int cpu) { return -1; } +static inline int restore_cpu_idle_disable_state(unsigned int cpu) { return -1; } +static inline void free_cpu_idle_disable_states(void) { } +static inline int set_deepest_cpu_idle_state(unsigned int cpu, unsigned int state) { return -1; } +static inline int have_libcpupower_support(void) { return 0; } +#endif /* HAVE_LIBCPUPOWER_SUPPORT */ int auto_house_keeping(cpu_set_t *monitored_cpus); #define ns_to_usf(x) (((double)x/1000)) From 549b92c94c7e6db12842c5d64c036e6613dbd902 Mon Sep 17 00:00:00 2001 From: Tomas Glozar Date: Thu, 17 Oct 2024 16:09:12 +0200 Subject: [PATCH 13/20] rtla/timerlat: Add --deepest-idle-state for top Add option to limit deepest idle state on CPUs where timerlat is running for the duration of the workload. Link: https://lore.kernel.org/20241017140914.3200454-5-tglozar@redhat.com Signed-off-by: Tomas Glozar Signed-off-by: Steven Rostedt (Google) --- tools/tracing/rtla/src/timerlat_top.c | 42 ++++++++++++++++++++++++++- 1 file changed, 41 insertions(+), 1 deletion(-) diff --git a/tools/tracing/rtla/src/timerlat_top.c b/tools/tracing/rtla/src/timerlat_top.c index 7fb85c8ee3bc..fca0da3caa87 100644 --- a/tools/tracing/rtla/src/timerlat_top.c +++ b/tools/tracing/rtla/src/timerlat_top.c @@ -48,6 +48,7 @@ struct timerlat_top_params { int pretty_output; int warmup; int buffer_size; + int deepest_idle_state; cpu_set_t hk_cpu_set; struct sched_attr sched_param; struct trace_events *events; @@ -447,7 +448,7 @@ static void timerlat_top_usage(char *usage) "", " usage: rtla timerlat [top] [-h] [-q] [-a us] [-d s] [-D] [-n] [-p us] [-i us] [-T us] [-s us] \\", " [[-t[file]] [-e sys[:event]] [--filter ] [--trigger ] [-c cpu-list] [-H cpu-list]\\", - " [-P priority] [--dma-latency us] [--aa-only us] [-C[=cgroup_name]] [-u|-k] [--warm-up s]", + " [-P priority] [--dma-latency us] [--aa-only us] [-C[=cgroup_name]] [-u|-k] [--warm-up s] [--deepest-idle-state n]", "", " -h/--help: print this menu", " -a/--auto: set automatic trace mode, stopping the session if argument in us latency is hit", @@ -481,6 +482,7 @@ static void timerlat_top_usage(char *usage) " -U/--user-load: enable timerlat for user-defined user-space workload", " --warm-up s: let the workload run for s seconds before collecting data", " --trace-buffer-size kB: set the per-cpu trace buffer size in kB", + " --deepest-idle-state n: only go down to idle state n on cpus used by timerlat to reduce exit from idle latency", NULL, }; @@ -518,6 +520,9 @@ static struct timerlat_top_params /* disabled by default */ params->dma_latency = -1; + /* disabled by default */ + params->deepest_idle_state = -2; + /* display data in microseconds */ params->output_divisor = 1000; @@ -550,6 +555,7 @@ static struct timerlat_top_params {"aa-only", required_argument, 0, '5'}, {"warm-up", required_argument, 0, '6'}, {"trace-buffer-size", required_argument, 0, '7'}, + {"deepest-idle-state", required_argument, 0, '8'}, {0, 0, 0, 0} }; @@ -726,6 +732,9 @@ static struct timerlat_top_params case '7': params->buffer_size = get_llong_from_str(optarg); break; + case '8': + params->deepest_idle_state = get_llong_from_str(optarg); + break; default: timerlat_top_usage("Invalid option"); } @@ -922,6 +931,7 @@ int timerlat_top_main(int argc, char *argv[]) int return_value = 1; char *max_lat; int retval; + int nr_cpus, i; params = timerlat_top_parse_args(argc, argv); if (!params) @@ -971,6 +981,28 @@ int timerlat_top_main(int argc, char *argv[]) } } + if (params->deepest_idle_state >= -1) { + if (!have_libcpupower_support()) { + err_msg("rtla built without libcpupower, --deepest-idle-state is not supported\n"); + goto out_free; + } + + nr_cpus = sysconf(_SC_NPROCESSORS_CONF); + + for (i = 0; i < nr_cpus; i++) { + if (params->cpus && !CPU_ISSET(i, ¶ms->monitored_cpus)) + continue; + if (save_cpu_idle_disable_state(i) < 0) { + err_msg("Could not save cpu idle state.\n"); + goto out_free; + } + if (set_deepest_cpu_idle_state(i, params->deepest_idle_state) < 0) { + err_msg("Could not set deepest cpu idle state.\n"); + goto out_free; + } + } + } + if (params->trace_output) { record = osnoise_init_trace_tool("timerlat"); if (!record) { @@ -1125,6 +1157,13 @@ int timerlat_top_main(int argc, char *argv[]) timerlat_aa_destroy(); if (dma_latency_fd >= 0) close(dma_latency_fd); + if (params->deepest_idle_state >= -1) { + for (i = 0; i < nr_cpus; i++) { + if (params->cpus && !CPU_ISSET(i, ¶ms->monitored_cpus)) + continue; + restore_cpu_idle_disable_state(i); + } + } trace_events_destroy(&record->trace, params->events); params->events = NULL; out_free: @@ -1134,6 +1173,7 @@ int timerlat_top_main(int argc, char *argv[]) osnoise_destroy_tool(record); osnoise_destroy_tool(top); free(params); + free_cpu_idle_disable_states(); out_exit: exit(return_value); } From cfbfbfc96f6d6947605ed905d73b05feaac78181 Mon Sep 17 00:00:00 2001 From: Tomas Glozar Date: Thu, 17 Oct 2024 16:09:13 +0200 Subject: [PATCH 14/20] rtla/timerlat: Add --deepest-idle-state for hist Support limiting deepest idle state also for timerlat-hist. Link: https://lore.kernel.org/20241017140914.3200454-6-tglozar@redhat.com Signed-off-by: Tomas Glozar Signed-off-by: Steven Rostedt (Google) --- tools/tracing/rtla/src/timerlat_hist.c | 42 +++++++++++++++++++++++++- 1 file changed, 41 insertions(+), 1 deletion(-) diff --git a/tools/tracing/rtla/src/timerlat_hist.c b/tools/tracing/rtla/src/timerlat_hist.c index d49c8f0855fe..01dd337da13a 100644 --- a/tools/tracing/rtla/src/timerlat_hist.c +++ b/tools/tracing/rtla/src/timerlat_hist.c @@ -55,6 +55,7 @@ struct timerlat_hist_params { int entries; int warmup; int buffer_size; + int deepest_idle_state; }; struct timerlat_hist_cpu { @@ -655,7 +656,7 @@ static void timerlat_hist_usage(char *usage) " [-t[file]] [-e sys[:event]] [--filter ] [--trigger ] [-c cpu-list] [-H cpu-list]\\", " [-P priority] [-E N] [-b N] [--no-irq] [--no-thread] [--no-header] [--no-summary] \\", " [--no-index] [--with-zeros] [--dma-latency us] [-C[=cgroup_name]] [--no-aa] [--dump-task] [-u|-k]", - " [--warm-up s]", + " [--warm-up s] [--deepest-idle-state n]", "", " -h/--help: print this menu", " -a/--auto: set automatic trace mode, stopping the session if argument in us latency is hit", @@ -695,6 +696,7 @@ static void timerlat_hist_usage(char *usage) " -U/--user-load: enable timerlat for user-defined user-space workload", " --warm-up s: let the workload run for s seconds before collecting data", " --trace-buffer-size kB: set the per-cpu trace buffer size in kB", + " --deepest-idle-state n: only go down to idle state n on cpus used by timerlat to reduce exit from idle latency", NULL, }; @@ -732,6 +734,9 @@ static struct timerlat_hist_params /* disabled by default */ params->dma_latency = -1; + /* disabled by default */ + params->deepest_idle_state = -2; + /* display data in microseconds */ params->output_divisor = 1000; params->bucket_size = 1; @@ -772,6 +777,7 @@ static struct timerlat_hist_params {"dump-task", no_argument, 0, '\1'}, {"warm-up", required_argument, 0, '\2'}, {"trace-buffer-size", required_argument, 0, '\3'}, + {"deepest-idle-state", required_argument, 0, '\4'}, {0, 0, 0, 0} }; @@ -960,6 +966,9 @@ static struct timerlat_hist_params case '\3': params->buffer_size = get_llong_from_str(optarg); break; + case '\4': + params->deepest_idle_state = get_llong_from_str(optarg); + break; default: timerlat_hist_usage("Invalid option"); } @@ -1152,6 +1161,7 @@ int timerlat_hist_main(int argc, char *argv[]) int return_value = 1; pthread_t timerlat_u; int retval; + int nr_cpus, i; params = timerlat_hist_parse_args(argc, argv); if (!params) @@ -1201,6 +1211,28 @@ int timerlat_hist_main(int argc, char *argv[]) } } + if (params->deepest_idle_state >= -1) { + if (!have_libcpupower_support()) { + err_msg("rtla built without libcpupower, --deepest-idle-state is not supported\n"); + goto out_free; + } + + nr_cpus = sysconf(_SC_NPROCESSORS_CONF); + + for (i = 0; i < nr_cpus; i++) { + if (params->cpus && !CPU_ISSET(i, ¶ms->monitored_cpus)) + continue; + if (save_cpu_idle_disable_state(i) < 0) { + err_msg("Could not save cpu idle state.\n"); + goto out_free; + } + if (set_deepest_cpu_idle_state(i, params->deepest_idle_state) < 0) { + err_msg("Could not set deepest cpu idle state.\n"); + goto out_free; + } + } + } + if (params->trace_output) { record = osnoise_init_trace_tool("timerlat"); if (!record) { @@ -1332,6 +1364,13 @@ int timerlat_hist_main(int argc, char *argv[]) timerlat_aa_destroy(); if (dma_latency_fd >= 0) close(dma_latency_fd); + if (params->deepest_idle_state >= -1) { + for (i = 0; i < nr_cpus; i++) { + if (params->cpus && !CPU_ISSET(i, ¶ms->monitored_cpus)) + continue; + restore_cpu_idle_disable_state(i); + } + } trace_events_destroy(&record->trace, params->events); params->events = NULL; out_free: @@ -1340,6 +1379,7 @@ int timerlat_hist_main(int argc, char *argv[]) osnoise_destroy_tool(record); osnoise_destroy_tool(tool); free(params); + free_cpu_idle_disable_states(); out_exit: exit(return_value); } From 13216486e3ede30d6910a22e0e15988b7016366b Mon Sep 17 00:00:00 2001 From: Tomas Glozar Date: Thu, 17 Oct 2024 16:09:14 +0200 Subject: [PATCH 15/20] rtla: Documentation: Mention --deepest-idle-state Add --deepest-idle-state to manpage and mention libcpupower dependency in README.txt. Link: https://lore.kernel.org/20241017140914.3200454-7-tglozar@redhat.com Signed-off-by: Tomas Glozar Signed-off-by: Steven Rostedt (Google) --- Documentation/tools/rtla/common_timerlat_options.rst | 8 ++++++++ tools/tracing/rtla/README.txt | 4 ++++ 2 files changed, 12 insertions(+) diff --git a/Documentation/tools/rtla/common_timerlat_options.rst b/Documentation/tools/rtla/common_timerlat_options.rst index cef6651f1435..10dc802f8d65 100644 --- a/Documentation/tools/rtla/common_timerlat_options.rst +++ b/Documentation/tools/rtla/common_timerlat_options.rst @@ -31,6 +31,14 @@ *cyclictest* sets this value to *0* by default, use **--dma-latency** *0* to have similar results. +**--deepest-idle-state** *n* + Disable idle states higher than *n* for cpus that are running timerlat threads to + reduce exit from idle latencies. If *n* is -1, all idle states are disabled. + On exit from timerlat, the idle state setting is restored to its original state + before running timerlat. + + Requires rtla to be built with libcpupower. + **-k**, **--kernel-threads** Use timerlat kernel-space threads, in contrast of **-u**. diff --git a/tools/tracing/rtla/README.txt b/tools/tracing/rtla/README.txt index 4af3fd40f171..dd5621038c55 100644 --- a/tools/tracing/rtla/README.txt +++ b/tools/tracing/rtla/README.txt @@ -11,6 +11,7 @@ RTLA depends on the following libraries and tools: - libtracefs - libtraceevent + - libcpupower (optional, for --deepest-idle-state) It also depends on python3-docutils to compile man pages. @@ -26,6 +27,9 @@ For development, we suggest the following steps for compiling rtla: $ make $ sudo make install $ cd .. + $ cd $libcpupower_src + $ make + $ sudo make install $ cd $rtla_src $ make $ sudo make install From fcbc60d7dc4b125c8de130aa1512e5d20726c06e Mon Sep 17 00:00:00 2001 From: Tomas Glozar Date: Mon, 21 Oct 2024 14:31:40 +0200 Subject: [PATCH 16/20] rtla/timerlat: Do not set params->user_workload with -U Since commit fb9e90a67ee9 ("rtla/timerlat: Make user-space threads the default"), rtla-timerlat has been defaulting to params->user_workload if neither that or params->kernel_workload is set. This has unintentionally made -U, which sets only params->user_hist/top but not params->user_workload, to behave like -u unless -k is set, preventing the user from running a custom workload. Example: $ rtla timerlat hist -U -c 0 & [1] 7413 $ python sample/timerlat_load.py 0 Error opening timerlat fd, did you run timerlat -U? $ ps | grep timerlatu 7415 pts/4 00:00:00 timerlatu/0 Fix the issue by checking for params->user_top/hist instead of params->user_workload when setting default thread mode. Link: https://lore.kernel.org/20241021123140.14652-1-tglozar@redhat.com Fixes: fb9e90a67ee9 ("rtla/timerlat: Make user-space threads the default") Signed-off-by: Tomas Glozar Signed-off-by: Steven Rostedt (Google) --- tools/tracing/rtla/src/timerlat_hist.c | 2 +- tools/tracing/rtla/src/timerlat_top.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/tracing/rtla/src/timerlat_hist.c b/tools/tracing/rtla/src/timerlat_hist.c index 01dd337da13a..8b66387e5f35 100644 --- a/tools/tracing/rtla/src/timerlat_hist.c +++ b/tools/tracing/rtla/src/timerlat_hist.c @@ -1073,7 +1073,7 @@ timerlat_hist_apply_config(struct osnoise_tool *tool, struct timerlat_hist_param * If the user did not specify a type of thread, try user-threads first. * Fall back to kernel threads otherwise. */ - if (!params->kernel_workload && !params->user_workload) { + if (!params->kernel_workload && !params->user_hist) { retval = tracefs_file_exists(NULL, "osnoise/per_cpu/cpu0/timerlat_fd"); if (retval) { debug_msg("User-space interface detected, setting user-threads\n"); diff --git a/tools/tracing/rtla/src/timerlat_top.c b/tools/tracing/rtla/src/timerlat_top.c index fca0da3caa87..f7422948205d 100644 --- a/tools/tracing/rtla/src/timerlat_top.c +++ b/tools/tracing/rtla/src/timerlat_top.c @@ -839,7 +839,7 @@ timerlat_top_apply_config(struct osnoise_tool *top, struct timerlat_top_params * * If the user did not specify a type of thread, try user-threads first. * Fall back to kernel threads otherwise. */ - if (!params->kernel_workload && !params->user_workload) { + if (!params->kernel_workload && !params->user_top) { retval = tracefs_file_exists(NULL, "osnoise/per_cpu/cpu0/timerlat_fd"); if (retval) { debug_msg("User-space interface detected, setting user-threads\n"); From fc5f5aef9f97788f085651923789e3c53c065f58 Mon Sep 17 00:00:00 2001 From: furkanonder Date: Mon, 21 Oct 2024 15:09:49 +0000 Subject: [PATCH 17/20] tools/rtla: Improve code readability in timerlat_load.py The enhancements made to timerlat_load.py are intended to improve the script's robustness and readability. Summary of the changes: - Unnecessary semicolons at the end of lines have been removed. - Parentheses surrounding the if statement checking args.prio have been eliminated. - String concatenation for constructing timerlat_path has been replaced with an f-string. - Spacing in a multiplication expression has been adjusted for improved clarity. Cc: "jkacur@redhat.com" Cc: "lgoncalv@redhat.com" Link: https://lore.kernel.org/j2B-ted7pv3TaldTyqfIHrMmjq2fVyBFgnu3TskiQJsyRzy9loPTVVJoqHnrCWu5T88MDIFc612jUglH6Sxkdg9LN-I1XuITmoL70uECmus=@protonmail.com Signed-off-by: Furkan Onder Reviewed-by: Tomas Glozar Signed-off-by: Steven Rostedt (Google) --- tools/tracing/rtla/sample/timerlat_load.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tools/tracing/rtla/sample/timerlat_load.py b/tools/tracing/rtla/sample/timerlat_load.py index 8cc5eb2d2e69..785e9a83539a 100644 --- a/tools/tracing/rtla/sample/timerlat_load.py +++ b/tools/tracing/rtla/sample/timerlat_load.py @@ -37,12 +37,12 @@ except: exit(1) try: - os.sched_setaffinity(0, affinity_mask); + os.sched_setaffinity(0, affinity_mask) except: print("Error setting affinity") exit(1) -if (args.prio): +if args.prio: try: param = os.sched_param(int(args.prio)) os.sched_setscheduler(0, os.SCHED_FIFO, param) @@ -51,21 +51,21 @@ if (args.prio): exit(1) try: - timerlat_path = "/sys/kernel/tracing/osnoise/per_cpu/cpu" + args.cpu + "/timerlat_fd" + timerlat_path = f"/sys/kernel/tracing/osnoise/per_cpu/cpu{args.cpu}/timerlat_fd" timerlat_fd = open(timerlat_path, 'r') except: print("Error opening timerlat fd, did you run timerlat -U?") exit(1) try: - data_fd = open("/dev/full", 'r'); + data_fd = open("/dev/full", 'r') except: print("Error opening data fd") while True: try: timerlat_fd.read(1) - data_fd.read(20*1024*1024) + data_fd.read(20 * 1024 * 1024) except: print("Leaving") break From bd26818343dc02936a4f2f7b63368d5e1e1773c8 Mon Sep 17 00:00:00 2001 From: furkanonder Date: Mon, 21 Oct 2024 15:12:30 +0000 Subject: [PATCH 18/20] tools/rtla: Enhance argument parsing in timerlat_load.py The enhancements made to timerlat_load.py are aimed at improving the clarity of argument parsing. Summary of Changes: - The cpu argument is now specified as an integer type in the argument parser to enforce input validation, and the construction of affinity_mask has been simplified to directly use the integer value of args.cpu. - The prio argument is similarly updated to be of integer type for consistency and validation, eliminating the need for the conversion of args.prio to an integer, as this is now handled by the argument parser. Cc: "jkacur@redhat.com" Cc: "lgoncalv@redhat.com" Link: https://lore.kernel.org/QfgO7ayKD9dsLk8_ZDebkAV0OF7wla7UmasbP9CBmui_sChOeizy512t3RqCHTjvQoUBUDP8dwEOVCdHQ5KvVNEiP69CynMY94SFDERWl94=@protonmail.com Signed-off-by: Furkan Onder Reviewed-by: Tomas Glozar Signed-off-by: Steven Rostedt (Google) --- tools/tracing/rtla/sample/timerlat_load.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/tools/tracing/rtla/sample/timerlat_load.py b/tools/tracing/rtla/sample/timerlat_load.py index 785e9a83539a..d7341ed5127a 100644 --- a/tools/tracing/rtla/sample/timerlat_load.py +++ b/tools/tracing/rtla/sample/timerlat_load.py @@ -25,13 +25,12 @@ import sys import os parser = argparse.ArgumentParser(description='user-space timerlat thread in Python') -parser.add_argument("cpu", help='CPU to run timerlat thread') -parser.add_argument("-p", "--prio", help='FIFO priority') - +parser.add_argument("cpu", type=int, help='CPU to run timerlat thread') +parser.add_argument("-p", "--prio", type=int, help='FIFO priority') args = parser.parse_args() try: - affinity_mask = { int(args.cpu) } + affinity_mask = {args.cpu} except: print("Invalid cpu: " + args.cpu) exit(1) @@ -44,7 +43,7 @@ except: if args.prio: try: - param = os.sched_param(int(args.prio)) + param = os.sched_param(args.prio) os.sched_setscheduler(0, os.SCHED_FIFO, param) except: print("Error setting priority") From 4d8c1ba0790b4341a58e2042810a952c62df06e9 Mon Sep 17 00:00:00 2001 From: furkanonder Date: Mon, 21 Oct 2024 15:13:57 +0000 Subject: [PATCH 19/20] tools/rtla: Improve exception handling in timerlat_load.py The enhancements made to timerlat_load.py are intended to improve the script's exception handling. Summary of the changes: - Specific exceptions are now caught for CPU affinity and priority settings, with clearer error messages provided. - The timerlat file descriptor opening now includes handling for PermissionError and OSError, with informative messages. - In the infinite loop, generic exceptions have been replaced with specific types like KeyboardInterrupt and IOError, improving feedback. Before: $ sudo python timerlat_load.py 122 Error setting affinity After: $ sudo python timerlat_load.py 122 Error setting affinity: [Errno 22] Invalid argument Before: $ sudo python timerlat_load.py 1 -p 950 Error setting priority After: $ sudo python timerlat_load.py 1 -p 950 Error setting priority: [Errno 22] Invalid argument Before: $ python timerlat_load.py 1 Error opening timerlat fd, did you run timerlat -U? After: $ python timerlat_load.py 1 Permission denied. Please check your access rights. Cc: "lgoncalv@redhat.com" Cc: "jkacur@redhat.com" Link: https://lore.kernel.org/Q_k1s4hBtUy2px8ou0QKenjEK2_T_LoV8IxAE79aBakBogb-7uHp2fpET3oWtI1t3dy8uKjWeRzQOdKNzIzOOpyM4OjutJOriZ9TrGY6b-g=@protonmail.com Signed-off-by: Furkan Onder Reviewed-by: Tomas Glozar Signed-off-by: Steven Rostedt (Google) --- tools/tracing/rtla/sample/timerlat_load.py | 37 ++++++++++++---------- 1 file changed, 21 insertions(+), 16 deletions(-) diff --git a/tools/tracing/rtla/sample/timerlat_load.py b/tools/tracing/rtla/sample/timerlat_load.py index d7341ed5127a..a819c3588073 100644 --- a/tools/tracing/rtla/sample/timerlat_load.py +++ b/tools/tracing/rtla/sample/timerlat_load.py @@ -31,43 +31,48 @@ args = parser.parse_args() try: affinity_mask = {args.cpu} -except: - print("Invalid cpu: " + args.cpu) - exit(1) - -try: os.sched_setaffinity(0, affinity_mask) -except: - print("Error setting affinity") - exit(1) +except Exception as e: + print(f"Error setting affinity: {e}") + sys.exit(1) if args.prio: try: param = os.sched_param(args.prio) os.sched_setscheduler(0, os.SCHED_FIFO, param) - except: - print("Error setting priority") - exit(1) + except Exception as e: + print(f"Error setting priority: {e}") + sys.exit(1) try: timerlat_path = f"/sys/kernel/tracing/osnoise/per_cpu/cpu{args.cpu}/timerlat_fd" timerlat_fd = open(timerlat_path, 'r') -except: +except PermissionError: + print("Permission denied. Please check your access rights.") + sys.exit(1) +except OSError: print("Error opening timerlat fd, did you run timerlat -U?") - exit(1) + sys.exit(1) try: data_fd = open("/dev/full", 'r') -except: - print("Error opening data fd") +except Exception as e: + print(f"Error opening data fd: {e}") + sys.exit(1) while True: try: timerlat_fd.read(1) data_fd.read(20 * 1024 * 1024) - except: + except KeyboardInterrupt: print("Leaving") break + except IOError as e: + print(f"I/O error occurred: {e}") + break + except Exception as e: + print(f"Unexpected error: {e}") + break timerlat_fd.close() data_fd.close() From 571f8b3f866a6d990a50fe5c89fe0ea78784d70b Mon Sep 17 00:00:00 2001 From: Gabriele Monaco Date: Thu, 17 Oct 2024 08:42:39 +0200 Subject: [PATCH 20/20] verification/dot2: Improve dot parser robustness This patch makes the dot parser used by dot2c and dot2k slightly more robust, namely: * allows parsing files with the gv extension (GraphViz) * correctly parses edges with any indentation * used to work only with a single character (e.g. '\t') Additionally it fixes a couple of warnings reported by pylint such as wrong indentation and comparison to False instead of `not ...` Link: https://lore.kernel.org/20241017064238.41394-2-gmonaco@redhat.com Signed-off-by: Gabriele Monaco Signed-off-by: Steven Rostedt (Google) --- tools/verification/dot2/automata.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/tools/verification/dot2/automata.py b/tools/verification/dot2/automata.py index baffeb960ff0..bdeb98baa8b0 100644 --- a/tools/verification/dot2/automata.py +++ b/tools/verification/dot2/automata.py @@ -29,11 +29,11 @@ class Automata: def __get_model_name(self): basename = ntpath.basename(self.__dot_path) - if basename.endswith(".dot") == False: + if not basename.endswith(".dot") and not basename.endswith(".gv"): print("not a dot file") raise Exception("not a dot file: %s" % self.__dot_path) - model_name = basename[0:-4] + model_name = ntpath.splitext(basename)[0] if model_name.__len__() == 0: raise Exception("not a dot file: %s" % self.__dot_path) @@ -68,9 +68,9 @@ class Automata: def __get_cursor_begin_events(self): cursor = 0 while self.__dot_lines[cursor].split()[0] != "{node": - cursor += 1 + cursor += 1 while self.__dot_lines[cursor].split()[0] == "{node": - cursor += 1 + cursor += 1 # skip initial state transition cursor += 1 return cursor @@ -94,11 +94,11 @@ class Automata: initial_state = state[7:] else: states.append(state) - if self.__dot_lines[cursor].__contains__("doublecircle") == True: + if "doublecircle" in self.__dot_lines[cursor]: final_states.append(state) has_final_states = True - if self.__dot_lines[cursor].__contains__("ellipse") == True: + if "ellipse" in self.__dot_lines[cursor]: final_states.append(state) has_final_states = True @@ -110,7 +110,7 @@ class Automata: # Insert the initial state at the bein og the states states.insert(0, initial_state) - if has_final_states == False: + if not has_final_states: final_states.append(initial_state) return states, initial_state, final_states @@ -120,7 +120,7 @@ class Automata: cursor = self.__get_cursor_begin_events() events = [] - while self.__dot_lines[cursor][1] == '"': + while self.__dot_lines[cursor].lstrip()[0] == '"': # transitions have the format: # "all_fired" -> "both_fired" [ label = "disable_irq" ]; # ------------ event is here ------------^^^^^ @@ -161,7 +161,7 @@ class Automata: # and we are back! Let's fill the matrix cursor = self.__get_cursor_begin_events() - while self.__dot_lines[cursor][1] == '"': + while self.__dot_lines[cursor].lstrip()[0] == '"': if self.__dot_lines[cursor].split()[1] == "->": line = self.__dot_lines[cursor].split() origin_state = line[0].replace('"','').replace(',','_')