From ba3e978e46ac7727cae7ae58c669e3883f70495a Mon Sep 17 00:00:00 2001 From: Douglas Raillard Date: Thu, 26 Jun 2025 20:27:26 +0100 Subject: [PATCH 1/2] lisa._assets.kmodules.lisa: Use variably-sized strings for event fields FEATURE Use variably-sized strings (__data_loc char[]) instead of fixed-size arrays for string fields. This avoids wasting space when the string is smaller than the maximum size, at the cost of 4 bytes of overhead. --- lisa/_assets/kmodules/lisa/ftrace_events.h | 71 ++++++++++------------ lisa/_assets/kmodules/lisa/tp.c | 7 +++ 2 files changed, 40 insertions(+), 38 deletions(-) diff --git a/lisa/_assets/kmodules/lisa/ftrace_events.h b/lisa/_assets/kmodules/lisa/ftrace_events.h index 9b9aa1f61..a7a7b3053 100644 --- a/lisa/_assets/kmodules/lisa/ftrace_events.h +++ b/lisa/_assets/kmodules/lisa/ftrace_events.h @@ -2,15 +2,9 @@ #undef TRACE_SYSTEM #define TRACE_SYSTEM lisa -#define MAX_SPAN_SIZE 128 - #if !defined(_FTRACE_EVENTS_H) || defined(TRACE_HEADER_MULTI_READ) #define _FTRACE_EVENTS_H -#define PATH_SIZE 64 -#define __SPAN_SIZE (round_up(NR_CPUS, 4)/4) -#define SPAN_SIZE (__SPAN_SIZE > MAX_SPAN_SIZE ? MAX_SPAN_SIZE : __SPAN_SIZE) - #include #include #include @@ -45,12 +39,12 @@ TRACE_EVENT(lisa__sched_pelt_cfs, __field( unsigned long, util ) __field( unsigned long, load ) __field( int, cpu ) - __array( char, path, PATH_SIZE ) + __string( path, path ) ), TP_fast_assign( __entry->cpu = cpu; - strscpy(__entry->path, path, PATH_SIZE); + __lisa_assign_str(path, path); __entry->load = avg->load_avg; #if HAS_KERNEL_FEATURE(SCHED_AVG_RBL) __entry->RBL_LOAD_ENTRY = avg->RBL_LOAD_MEMBER; @@ -65,8 +59,8 @@ TRACE_EVENT(lisa__sched_pelt_cfs, " " RBL_LOAD_STR "=%lu" #endif , - __entry->cpu, __entry->path, __entry->load, - __entry->util, __entry->update_time + __entry->cpu, __get_str(path), __entry->load, __entry->util, + __entry->update_time #if HAS_KERNEL_FEATURE(SCHED_AVG_RBL) ,__entry->RBL_LOAD_ENTRY #endif @@ -150,14 +144,14 @@ TRACE_EVENT(lisa__sched_pelt_se, __field( unsigned long, util ) __field( int, cpu ) __field( int, pid ) - __array( char, path, PATH_SIZE ) - __array( char, comm, TASK_COMM_LEN ) + __string( path, path ) + __string( comm, comm ) ), TP_fast_assign( __entry->cpu = cpu; - strscpy(__entry->path, path, PATH_SIZE); - strscpy(__entry->comm, comm, TASK_COMM_LEN); + __lisa_assign_str(path, path); + __lisa_assign_str(comm, comm); __entry->pid = pid; __entry->load = avg->load_avg; #if HAS_KERNEL_FEATURE(SCHED_AVG_RBL) @@ -173,7 +167,7 @@ TRACE_EVENT(lisa__sched_pelt_se, " " RBL_LOAD_STR "=%lu" #endif , - __entry->cpu, __entry->path, __entry->comm, __entry->pid, + __entry->cpu, __get_str(path), __get_str(comm), __entry->pid, __entry->load, __entry->util, __entry->update_time #if HAS_KERNEL_FEATURE(SCHED_AVG_RBL) ,__entry->RBL_LOAD_ENTRY @@ -191,16 +185,16 @@ TRACE_EVENT(lisa__sched_overutilized, TP_STRUCT__entry( __field( bool, overutilized ) - __array( char, span, SPAN_SIZE ) + __string( span, span ) ), TP_fast_assign( __entry->overutilized = overutilized; - strscpy(__entry->span, span, SPAN_SIZE); + __lisa_assign_str(span, span); ), TP_printk("overutilized=%d span=0x%s", - __entry->overutilized, __entry->span) + __entry->overutilized, __get_str(span)) ); #endif @@ -241,14 +235,14 @@ TRACE_EVENT(lisa__sched_util_est_se, __field( unsigned int, ewma ) __field( int, cpu ) __field( int, pid ) - __array( char, path, PATH_SIZE ) - __array( char, comm, TASK_COMM_LEN ) + __string( path, path ) + __string( comm, comm ) ), TP_fast_assign( __entry->cpu = cpu; - strscpy(__entry->path, path, PATH_SIZE); - strscpy(__entry->comm, comm, TASK_COMM_LEN); + __lisa_assign_str(path, path); + __lisa_assign_str(comm, comm); __entry->pid = pid; __entry->enqueued = avg->util_est.enqueued & ~UTIL_AVG_UNCHANGED; __entry->ewma = avg->util_est.ewma; @@ -256,7 +250,7 @@ TRACE_EVENT(lisa__sched_util_est_se, ), TP_printk("cpu=%d path=%s comm=%s pid=%d enqueued=%u ewma=%u util=%lu", - __entry->cpu, __entry->path, __entry->comm, __entry->pid, + __entry->cpu, __get_str(path), __get_str(comm), __entry->pid, __entry->enqueued, __entry->ewma, __entry->util) ); #endif @@ -274,21 +268,21 @@ TRACE_EVENT(lisa__sched_util_est_se_unified, __field( unsigned int, util_est ) __field( int, cpu ) __field( int, pid ) - __array( char, path, PATH_SIZE ) - __array( char, comm, TASK_COMM_LEN ) + __string( path, path ) + __string( comm, comm ) ), TP_fast_assign( __entry->cpu = cpu; - strscpy(__entry->path, path, PATH_SIZE); - strscpy(__entry->comm, comm, TASK_COMM_LEN); + __lisa_assign_str(path, path); + __lisa_assign_str(comm, comm); __entry->pid = pid; __entry->util_est = avg->util_est & ~UTIL_AVG_UNCHANGED; __entry->util = avg->util_avg; ), TP_printk("cpu=%d path=%s comm=%s pid=%d util_est=%u util=%lu", - __entry->cpu, __entry->path, __entry->comm, __entry->pid, + __entry->cpu, __get_str(path), __get_str(comm), __entry->pid, __entry->util_est, __entry->util) ); #endif @@ -305,20 +299,20 @@ TRACE_EVENT(lisa__sched_util_est_cfs, __field( unsigned int, enqueued ) __field( unsigned int, ewma ) __field( int, cpu ) - __array( char, path, PATH_SIZE ) + __string( path, path ) ), TP_fast_assign( __entry->cpu = cpu; - strscpy(__entry->path, path, PATH_SIZE); + __lisa_assign_str(path, path); __entry->enqueued = avg->util_est.enqueued; __entry->ewma = avg->util_est.ewma; __entry->util = avg->util_avg; ), TP_printk("cpu=%d path=%s enqueued=%u ewma=%u util=%lu", - __entry->cpu, __entry->path, __entry->enqueued, - __entry->ewma, __entry->util) + __entry->cpu, __get_str(path), __entry->enqueued, + __entry->ewma, __entry->util) ); #endif @@ -333,18 +327,19 @@ TRACE_EVENT(lisa__sched_util_est_cfs_unified, __field( unsigned long, util ) __field( unsigned int, util_est ) __field( int, cpu ) - __array( char, path, PATH_SIZE ) + __string( path, path ) ), TP_fast_assign( __entry->cpu = cpu; - strscpy(__entry->path, path, PATH_SIZE); + __lisa_assign_str(path, path); __entry->util_est = avg->util_est; __entry->util = avg->util_avg; ), TP_printk("cpu=%d path=%s util_est=%u util=%lu", - __entry->cpu, __entry->path, __entry->util_est, __entry->util) + __entry->cpu, __get_str(path), __entry->util_est, + __entry->util) ); #endif @@ -364,12 +359,12 @@ TRACE_EVENT_CONDITION(lisa__uclamp_util_se, __field(unsigned long, uclamp_max ) __field( int, cpu ) __field( pid_t, pid ) - __array( char, comm, TASK_COMM_LEN ) + __string( comm, p->comm ) ), TP_fast_assign( __entry->pid = p->pid; - memcpy(__entry->comm, p->comm, TASK_COMM_LEN); + __lisa_assign_str(comm, p->comm); __entry->cpu = rq ? rq_cpu(rq) : -1; __entry->util_avg = p->se.avg.util_avg; __entry->uclamp_avg = uclamp_rq_util_with(rq, p->se.avg.util_avg); @@ -385,7 +380,7 @@ TRACE_EVENT_CONDITION(lisa__uclamp_util_se, " uclamp_min=%lu uclamp_max=%lu" # endif , - __entry->pid, __entry->comm, __entry->cpu, + __entry->pid, __get_str(comm), __entry->cpu, __entry->util_avg, __entry->uclamp_avg # if HAS_KERNEL_FEATURE(RQ_UCLAMP) ,__entry->uclamp_min, __entry->uclamp_max diff --git a/lisa/_assets/kmodules/lisa/tp.c b/lisa/_assets/kmodules/lisa/tp.c index 7fa112ab9..20a2b2c8b 100644 --- a/lisa/_assets/kmodules/lisa/tp.c +++ b/lisa/_assets/kmodules/lisa/tp.c @@ -11,6 +11,13 @@ #include "wq.h" #include "tp.h" +#define PATH_SIZE 64 + +#define MAX_SPAN_SIZE 128 +#define __SPAN_SIZE (round_up(NR_CPUS, 4)/4) +#define SPAN_SIZE (__SPAN_SIZE > MAX_SPAN_SIZE ? MAX_SPAN_SIZE : __SPAN_SIZE) + + #if HAS_KERNEL_FEATURE(CFS_PELT) static inline void _trace_cfs(struct cfs_rq *cfs_rq, void (*trace_event)(int, char*, -- GitLab From b0b5500a303074f1a381eee8b1b9d0462b0940cf Mon Sep 17 00:00:00 2001 From: Douglas Raillard Date: Thu, 26 Jun 2025 21:03:07 +0100 Subject: [PATCH 2/2] lisa._assets.kmodules.lisa: Avoid unnecessary string copies BREAKING CHANGE Avoid copying strings unnecessarily and use an empty string when there is no data to display, rather than the "(null)" string. This avoids wasting space on such fixed string, and avoids potential conflict with something that would actually be named "(null)". --- lisa/_assets/kmodules/lisa/ftrace_events.h | 6 ++-- lisa/_assets/kmodules/lisa/sched_helpers.h | 37 ++++++++-------------- lisa/_assets/kmodules/lisa/tp.c | 15 +++++---- lisa/analysis/load_tracking.py | 2 +- 4 files changed, 26 insertions(+), 34 deletions(-) diff --git a/lisa/_assets/kmodules/lisa/ftrace_events.h b/lisa/_assets/kmodules/lisa/ftrace_events.h index a7a7b3053..ea015138d 100644 --- a/lisa/_assets/kmodules/lisa/ftrace_events.h +++ b/lisa/_assets/kmodules/lisa/ftrace_events.h @@ -27,7 +27,7 @@ #if HAS_KERNEL_FEATURE(CFS_PELT) TRACE_EVENT(lisa__sched_pelt_cfs, - TP_PROTO(int cpu, char *path, const struct sched_avg *avg), + TP_PROTO(int cpu, const char *path, const struct sched_avg *avg), TP_ARGS(cpu, path, avg), @@ -290,7 +290,7 @@ TRACE_EVENT(lisa__sched_util_est_se_unified, #if HAS_KERNEL_FEATURE(CFS_UTIL_EST) TRACE_EVENT(lisa__sched_util_est_cfs, - TP_PROTO(int cpu, char *path, const struct sched_avg *avg), + TP_PROTO(int cpu, const char *path, const struct sched_avg *avg), TP_ARGS(cpu, path, avg), @@ -319,7 +319,7 @@ TRACE_EVENT(lisa__sched_util_est_cfs, #if HAS_KERNEL_FEATURE(CFS_UTIL_EST_UNIFIED) TRACE_EVENT(lisa__sched_util_est_cfs_unified, - TP_PROTO(int cpu, char *path, const struct sched_avg *avg), + TP_PROTO(int cpu, const char *path, const struct sched_avg *avg), TP_ARGS(cpu, path, avg), diff --git a/lisa/_assets/kmodules/lisa/sched_helpers.h b/lisa/_assets/kmodules/lisa/sched_helpers.h index 08bbcf408..e4c736a26 100644 --- a/lisa/_assets/kmodules/lisa/sched_helpers.h +++ b/lisa/_assets/kmodules/lisa/sched_helpers.h @@ -66,15 +66,16 @@ static inline bool task_group_is_autogroup(const struct task_group *tg) #endif #if HAS_TYPE(struct, task_group) -static int autogroup_path(const struct task_group *tg, char *buf, int buflen) +static const char *autogroup_path(const struct task_group *tg, char *buf, int buflen) { # if HAS_KERNEL_FEATURE(SCHED_AUTOGROUP) && HAS_MEMBER(struct, autogroup, id) if (!task_group_is_autogroup(tg)) - return 0; + return ""; - return snprintf(buf, buflen, "%s-%ld", "/autogroup", tg->autogroup->id); + snprintf(buf, buflen, "%s-%ld", "/autogroup", tg->autogroup->id); + return buf; # else - return 0; + return ""; # endif } #endif @@ -103,19 +104,17 @@ static inline unsigned long uclamp_rq_util_with(const struct rq *rq, unsigned lo #if HAS_TYPE(struct, cfs_rq) -static inline void cfs_rq_tg_path(const struct cfs_rq *cfs_rq, char *path, int len) +static inline const char *cfs_rq_tg_path(const struct cfs_rq *cfs_rq, char *path, int len) { - if (!path) - return; - # if defined(CONFIG_FAIR_GROUP_SCHED) && HAS_MEMBER(struct, cfs_rq, tg) && HAS_MEMBER(struct, task_group, css) && HAS_MEMBER(struct, cgroup_subsys_state, cgroup) - if (cfs_rq && task_group_is_autogroup(cfs_rq->tg)) - autogroup_path(cfs_rq->tg, path, len); - else if (cfs_rq && cfs_rq->tg->css.cgroup) + if (cfs_rq && task_group_is_autogroup(cfs_rq->tg)) { + return autogroup_path(cfs_rq->tg, path, len); + } else if (cfs_rq && cfs_rq->tg->css.cgroup) { cgroup_path((struct cgroup *)cfs_rq->tg->css.cgroup, path, len); - else + return path; + } # endif - strscpy(path, "(null)", len); + return ""; } #endif @@ -150,17 +149,9 @@ static inline const struct sched_avg *cfs_rq_avg(const struct cfs_rq *cfs_rq) # endif } -static inline char *cfs_rq_path(const struct cfs_rq *cfs_rq, char *str, int len) +static inline const char *cfs_rq_path(const struct cfs_rq *cfs_rq, char *str, int len) { - if (!cfs_rq) { - if (str) - strscpy(str, "(null)", len); - else - return NULL; - } - - cfs_rq_tg_path(cfs_rq, str, len); - return str; + return cfs_rq_tg_path(cfs_rq, str, len); } static inline int cfs_rq_cpu(const struct cfs_rq *cfs_rq) diff --git a/lisa/_assets/kmodules/lisa/tp.c b/lisa/_assets/kmodules/lisa/tp.c index 20a2b2c8b..d504909bb 100644 --- a/lisa/_assets/kmodules/lisa/tp.c +++ b/lisa/_assets/kmodules/lisa/tp.c @@ -20,15 +20,15 @@ #if HAS_KERNEL_FEATURE(CFS_PELT) static inline void _trace_cfs(struct cfs_rq *cfs_rq, - void (*trace_event)(int, char*, + void (*trace_event)(int, const char*, const struct sched_avg*)) { if (cfs_rq) { const struct sched_avg *avg = cfs_rq_avg(cfs_rq); - char path[PATH_SIZE]; + char tmp[PATH_SIZE]; int cpu = cfs_rq_cpu(cfs_rq); - cfs_rq_path(cfs_rq, path, PATH_SIZE); + const char *path = cfs_rq_path(cfs_rq, tmp, PATH_SIZE); trace_event(cpu, path, avg); } } @@ -42,13 +42,13 @@ static inline void _trace_se(struct sched_entity *se, { const struct cfs_rq *gcfs_rq = get_group_cfs_rq(se); const struct cfs_rq *cfs_rq = get_se_cfs_rq(se); - char path[PATH_SIZE]; + char tmp[PATH_SIZE]; - cfs_rq_path(gcfs_rq, path, PATH_SIZE); + const char *path = gcfs_rq ? cfs_rq_path(gcfs_rq, tmp, PATH_SIZE) : ""; int cpu = cfs_rq ? cfs_rq_cpu(cfs_rq) : -1; struct task_struct *p = gcfs_rq ? NULL : container_of(se, struct task_struct, se); - char *comm = p ? p->comm : "(null)"; + char *comm = p ? p->comm : ""; pid_t pid = p ? p->pid : -1; trace_event(cpu, path, comm, pid, &se->avg); @@ -146,7 +146,8 @@ static void sched_overutilized_probe(void *feature, struct root_domain *rd, bool if (trace_lisa__sched_overutilized_enabled()) { char span[SPAN_SIZE]; - cpumap_print_to_pagebuf(false, span, rd_span(rd)); + ssize_t count = cpumap_print_to_pagebuf(false, span, rd_span(rd)); + BUG_ON(count >= ARRAY_SIZE(span)); trace_lisa__sched_overutilized(overutilized, span); } diff --git a/lisa/analysis/load_tracking.py b/lisa/analysis/load_tracking.py index ede175b87..d1c75a170 100644 --- a/lisa/analysis/load_tracking.py +++ b/lisa/analysis/load_tracking.py @@ -106,7 +106,7 @@ class LoadTrackingAnalysis(TraceAnalysisBase): # Legacy sched_load_avg_* events don't have a `path` field. if not event.startswith('sched_load_avg_'): if event in self._SCHED_PELT_SE_NAMES: - df = df[df.path == "(null)"] + df = df[(df.path == "(null)") | (df.path == "")] if event in self._SCHED_PELT_CFS_NAMES: df = df[df.path == "/"] -- GitLab