[PATCH 0/3] stalld: Improve legacy kernel support and unify sched_debug parsing

[PATCH 0/3] stalld: Improve legacy kernel support and unify sched_debug parsing

[Derek Barbosa]

This patch series enhances stalld's compatibility with legacy 3.X kernels
and improves the robustness of the sched_debug backend parsing logic.

The main improvements include:

1. Consolidates the OLD and NEW task format parsing logic into a single, more
maintainable implementation. The new parser uses field offset detection to
handle different kernel versions' varying sched_debug output formats,
eliminating code duplication and improving reliability across kernel versions.

2. Legacy kernel build support: Adds automatic detection of 3.X kernels in the
Makefile and configures appropriate build flags. This includes disabling eBPF
support (unavailable on legacy kernels), removing Intel CET protection flags,
and defaulting to the sched_debug backend instead of the queue_track backend.

3. Shell compatibility fix: Corrects a bash syntax issue in the run-local script
that prevented execution on older bash versions (4.2.X) commonly found on legacy
systems.

Clark Williams (1):
  sched_debug: Unify parsing methods for task_info

Derek Barbosa (2):
  Makefile: Add support for legacy kernels
  scripts: fix run-local if bashism

 Makefile             |  40 ++++-
 scripts/run-local.sh |   2 +-
 src/sched_debug.c    | 405 +++++++++++++++++++------------------------
 src/sched_debug.h    |  65 ++++++-
 src/stalld.c         |   6 +-
 5 files changed, 288 insertions(+), 230 deletions(-)

--
2.50.0

[PATCH 1/3] sched_debug: Unify parsing methods for task_info

[Derek Barbosa]

From: Clark Williams <williams@redhat.com>

In the sched_debug backend code, there are two logical paths for parsing
the sched_debug file's task_info "running tasks". These are currently
divided into "OLD" and "NEW" parsing functions, each with their own
logic.

Unify these branching code paths by creating a line-based "word" parser
that stores the word-offset of the neccessary fields in a struct.
Accomodate "legacy" behavior where needed using an enumerated type.

parse_task_lines() now can parse multiple formats of output from the
debugfs file which on modern systems is located at
/sys/kernel/debug/sched/debug, and on 3.X "legacy" systems, is located
at /proc/sched_debug.

The detect_task_format() function records field offsets which are used
by parse_task_lines to pull fields out of the "running tasks:" section.

Signed-off-by: Clark Williams <williams@redhat.com>
Signed-off-by: Derek Barbosa <debarbos@redhat.com>
---
 src/sched_debug.c | 405 +++++++++++++++++++++-------------------------
 src/sched_debug.h |  65 +++++++-
 2 files changed, 244 insertions(+), 226 deletions(-)

diff --git a/src/sched_debug.c b/src/sched_debug.c
index fa2f74b..180932c 100644
--- a/src/sched_debug.c
+++ b/src/sched_debug.c
@@ -24,6 +24,9 @@
  */
 static int config_task_format;
 
+static struct task_format_offsets
+    config_task_format_offsets  = { 0, 0, 0, 0 };
+
 /*
  * Read the contents of sched_debug into the input buffer.
  */
@@ -88,8 +91,12 @@ static char *get_next_cpu_info_start(char *start)
 {
 	const char *next_cpu = "cpu#";
 
-	/* Skip the current CPU definition. */
-	start += 10;
+        /*
+         * Skip the current CPU definition.
+         * We want to move our "cursor" past the current "cpu#" definition.
+         * This number is arbitrary. It is purely to assist strstr().
+         */
+        start += 10;
 
 	return strstr(start, next_cpu);
 }
@@ -143,9 +150,13 @@ static inline char *skipchars(char *str)
 	return str;
 }
 
+/*
+ * Note, for our purposes newline is *not* a space
+ * and we want to stop when we hit it
+ */
 static inline char *skipspaces(char *str)
 {
-	while (*str && isspace(*str))
+	while (*str && isspace(*str) && (*str != '\n'))
 		str++;
 	return str;
 }
@@ -156,6 +167,20 @@ static inline char *nextline(char *str)
 	return ptr ? ptr+1 : NULL;
 }
 
+/*
+ * skip a specified number of words on a task line
+ */
+
+static inline char *skipwords(char *ptr, int nwords)
+{
+	int i;
+	for (i=0; i < nwords; i++) {
+		ptr = skipspaces(ptr);
+		ptr = skipchars(ptr);
+	}
+	return ptr;
+}
+
 /*
  * Read sched_debug and figure out if it's old or new format
  * done once so if we fail just exit the program.
@@ -173,6 +198,7 @@ static int detect_task_format(void)
 	char *ptr;
 	int status;
 	int fd;
+	int i, count=0;
 
 	bufsiz = bufincrement = BUFFER_PAGES * page_size;
 
@@ -204,122 +230,63 @@ static int detect_task_format(void)
 
 	ptr = strstr(buffer, TASK_MARKER);
 	if (ptr == NULL) {
-		fprintf(stderr, "unable to find 'runnable tasks' in buffer, invalid input\n");
+		die("unable to find 'runnable tasks' in buffer, invalid input\n");
 		exit(-1);
 	}
 
-	ptr += strlen(TASK_MARKER) + 1;
-	ptr = skipspaces(ptr);
+	ptr = nextline(ptr);
+	i = 0;
 
-	if (strncmp(ptr, "task", 4) == 0) {
-		retval = OLD_TASK_FORMAT;
-		log_msg("detected old task format\n");
-	} else if (strncmp(ptr, "S", 1) == 0) {
+	/*
+	 * Determine the TASK_FORMAT from the first "word" in the header
+	 * line.
+	 */
+	ptr = skipspaces(ptr);
+	if (strncmp(ptr, "S", strlen("S")) == 0) {
+		log_msg("detect_task_format: NEW_TASK_FORMAT detected\n");
 		retval = NEW_TASK_FORMAT;
-		log_msg("detected new task format\n");
 	}
-
-	free(buffer);
-	return retval;
-}
-
-/*
- * Parse the new sched_debug format.
- *
- * Example:
- * ' S           task   PID         tree-key  switches  prio     wait-time             sum-exec        sum-sleep'
- * '-----------------------------------------------------------------------------------------------------------'
- * ' I         rcu_gp     3        13.973264         2   100         0.000000         0.004469         0.000000 0 0 /
- */
-static int parse_new_task_format(char *buffer, struct task_info *task_info, int nr_entries)
-{
-	char *R, *X, *start = buffer;
-	struct task_info *task;
-	int tasks = 0;
-	int comm_size;
-	char *end;
+	else {
+		log_msg("detect_task_format: OLD_TASK_FORMAT detected\n");
+		retval = OLD_TASK_FORMAT;
+	}
 
 	/*
-	 * If we have less than two tasks on the CPU there is no
-	 * possibility of a stall.
+	 * Look for our header keywords and store their offset
+	 * we'll use the offsets when we actually parse the task
+	 * line data
 	 */
-	if (nr_entries < 2)
-		return 0;
-
-	while (tasks < nr_entries) {
-		task = &task_info[tasks];
-
-		/*
-		 * Runnable tasks.
-		 */
-		R = strstr(start, "\n R");
-
-		/*
-		 * Dying tasks.
-		 */
-		X = strstr(start, "\n X");
-
-		/*
-		 * Get the first one, the only one, or break.
-		 */
-		if (X && R) {
-			start = R < X ? R : X;
-		} else if (X || R) {
-			start = R ? R : X;
-		} else {
-			break;
+	while (*ptr != '\n') {
+		ptr = skipspaces(ptr);
+		if (strncmp(ptr, "task", strlen("task")) == 0) {
+			config_task_format_offsets.task = i;
+			count++;
+			log_msg("detect_task_format: found 'task' at word %d\n", i);
 		}
-
-		/* Skip '\n R' || '\n X'. */
-		start = &start[3];
-
-		/* Skip the spaces. */
-		start = skipspaces(start);
-
-		/* Find the end of the string. */
-		end = skipchars(start);
-
-		comm_size = end - start;
-
-		if (comm_size >= COMM_SIZE) {
-			warn("comm_size is too large: %d\n", comm_size);
-			comm_size = COMM_SIZE - 1;
+		else if (strncmp(ptr, "PID", strlen("PID")) == 0) {
+			config_task_format_offsets.pid = i;
+			count++;
+			log_msg("detect_task_format: found 'PID' at word %d\n", i);
 		}
-
-		strncpy(task->comm, start, comm_size);
-
-		task->comm[comm_size] = '\0';
-
-		/* Go to the end of the task comm. */
-		start=end;
-
-		task->pid = strtol(start, &end, 10);
-
-		/* Get the id of the thread group leader. */
-		task->tgid = get_tgid(task->pid);
-
-		/* Go to the end of the pid. */
-		start=end;
-
-		/* Skip the tree-key. */
-		start = skipspaces(start);
-		start = skipchars(start);
-
-		task->ctxsw = strtol(start, &end, 10);
-
-		start = end;
-
-		task->prio = strtol(start, &end, 10);
-
-		task->since = time(NULL);
-
-		/* Go to the end and try to find the next occurrence. */
-		start = end;
-
-		tasks++;
+		else if (strncmp(ptr, "switches", strlen("switches")) == 0) {
+			config_task_format_offsets.switches = i;
+			count++;
+			log_msg("detect_task_format: found 'switches' at word %d\n", i);
+		}
+		else if (strncmp(ptr, "prio", strlen("prio")) == 0) {
+			config_task_format_offsets.prio = i;
+			count++;
+			log_msg("detect_task_format: found 'prio' at word %d\n", i);
+		}
+		ptr = skipchars(ptr);
+		i++;
 	}
 
-	return tasks;
+	if (count != 4)
+		die("detect_task_format: did not detect all task line fields we need\n");
+
+	free(buffer);
+	return retval;
 }
 
 /*
@@ -387,104 +354,80 @@ out_error:
 	return runnable;
 }
 
-static int count_task_lines(char *buffer)
-{
-	int lines = 0;
-	char *ptr;
-	int len;
-
-	len = strlen(buffer);
-
-	/* Find the runnable tasks: header. */
-	ptr = strstr(buffer, TASK_MARKER);
-	if (ptr == NULL)
-		return 0;
-
-	/* Skip to the end of the dashed line separator. */
-	ptr = strstr(ptr, "-\n");
-	if (ptr == NULL)
-		return 0;
-
-	ptr += 2;
-	while(*ptr && ptr < (buffer+len)) {
-		lines++;
-		ptr = strchr(ptr, '\n');
-		if (ptr == NULL)
-			break;
-		ptr++;
-	}
-	return lines;
-}
-
-/*
- * Parse the old sched debug format:
- *
- * Example:
- * '            task   PID         tree-key  switches  prio     wait-time             sum-exec        sum-sleep
- * ' ----------------------------------------------------------------------------------------------------------
- * '     watchdog/35   296       -11.731402      4081     0         0.000000        44.052473         0.000000 /
- */
-static int parse_old_task_format(char *buffer, struct task_info *task_info, int nr_entries)
+static int parse_task_lines(char *buffer, struct task_info *task_info, int nr_entries)
 {
 	int pid, ctxsw, prio, comm_size;
-	char *start, *end, *buffer_end;
+	char *ptr, *line, *end;
 	struct task_info *task;
 	char comm[COMM_SIZE];
-	int waiting_tasks = 0;
-
-	start = buffer;
-	start = strstr(start, TASK_MARKER);
-	start = strstr(start, "-\n");
-	start++;
+	int tasks = 0;
 
-	buffer_end = buffer + strlen(buffer);
+	if ((ptr = strstr(buffer, TASK_MARKER)) == NULL)
+		die ("no runnable task section found!\n");
 
 	/*
-	 * We can't short-circuit using nr_entries, we have to scan the
-	 * entire list of processes that is on this CPU.
+	 * If we have less than two tasks on the CPU there is no
+	 * possibility of a stall.
 	 */
-	while (*start && start < buffer_end) {
-		task = &task_info[waiting_tasks];
+	if (nr_entries < 2)
+		return 0;
+	line = ptr;
+
+	/* skip header and divider */
+	line = nextline(line);
+	line = nextline(line);
+	
+	/* now loop over the task info */
+	while (tasks < nr_entries) {
+		task = &task_info[tasks];
 
-		/* Only care about tasks that are not R (running on a CPU). */
-		if (start[0] == 'R') {
+		/*
+		 * In 3.X kernels, only the singular RUNNING task receives
+		 * a "running state" label. Therefore, only care about
+		 * tasks that are not R (running on a CPU).
+		 */
+		if ((config_task_format == OLD_TASK_FORMAT) &&
+			(*ptr == 'R')) {
 			/* Go to the end of the line and ignore this task. */
-			start = strchr(start, '\n');
-			start++;
+			ptr = strchr(ptr, '\n');
+			ptr++;
 			continue;
 		}
 
-		/* Pick up the comm field. */
-		start = skipspaces(start);
-		end = skipchars(start);
-		comm_size = end - start;
+		/* get the task field */
+		ptr = skipwords(line, config_task_format_offsets.task);
+
+		/* Find the end of the task field */
+		end = skipchars(ptr);
+		comm_size = end - ptr;
+
+		/* make sure we don't overflow the comm array */
 		if (comm_size >= COMM_SIZE) {
 			warn("comm_size is too large: %d\n", comm_size);
 			comm_size = COMM_SIZE - 1;
 		}
-		strncpy(comm, start, comm_size);
-		comm[comm_size] = 0;
-
-		/* Go to the end of the task comm. */
-		start=end;
-
-		/* Now pick up the pid. */
-		pid = strtol(start, &end, 10);
-
-		/* Go to the end of the pid. */
-		start=end;
-
-		/* Skip the tree-key. */
-		start = skipspaces(start);
-		start = skipchars(start);
-
-		/* Pick up the context switch count. */
-		ctxsw = strtol(start, &end, 10);
-		start = end;
-
-		/* Get the priority. */
-		prio = strtol(start, &end, 10);
-		if (is_runnable(pid)) {
+		strncpy(comm, ptr, comm_size);
+		comm[comm_size] = '\0';
+		ptr = end;
+
+		/* get the PID field */
+		ptr = skipwords(line, config_task_format_offsets.pid);
+		pid = strtol(ptr, NULL, 10);
+
+		/* get the context switches field */
+		ptr = skipwords(line, config_task_format_offsets.switches);
+		ctxsw = strtol(ptr, NULL, 10);
+
+		/* get the prio field */
+		ptr = skipwords(line, config_task_format_offsets.prio);
+		prio = strtol(ptr, NULL, 10);
+
+                /*
+                 * In older formats, we must check to
+                 * see if the process is runnable prior to storing header
+                 * fields and incrementing task processing
+                 */
+                if ((config_task_format == NEW_TASK_FORMAT) || (is_runnable(pid))) {
 			strncpy(task->comm, comm, comm_size);
 			task->comm[comm_size] = 0;
 			task->pid = pid;
@@ -492,18 +435,44 @@ static int parse_old_task_format(char *buffer, struct task_info *task_info, int
 			task->ctxsw = ctxsw;
 			task->prio = prio;
 			task->since = time(NULL);
-			waiting_tasks++;
+			/* increment the count of tasks processed */
+			tasks++;
+		} else {
+			continue;
 		}
 
-		if ((start = nextline(start)) == NULL)
-			break;
+	}
+	return tasks;
+}
 
-		if (waiting_tasks >= nr_entries) {
+
+static int count_task_lines(char *buffer)
+{
+	int lines = 0;
+	char *ptr;
+	int len;
+
+	len = strlen(buffer);
+
+	/* Find the runnable tasks: header. */
+	ptr = strstr(buffer, TASK_MARKER);
+	if (ptr == NULL)
+		return 0;
+
+	/* Skip to the end of the dashed line separator. */
+	ptr = strstr(ptr, "-\n");
+	if (ptr == NULL)
+		return 0;
+
+	ptr += 2;
+	while(*ptr && ptr < (buffer+len)) {
+		lines++;
+		ptr = strchr(ptr, '\n');
+		if (ptr == NULL)
 			break;
-		}
+		ptr++;
 	}
-
-	return waiting_tasks;
+	return lines;
 }
 
 static int fill_waiting_task(char *buffer, struct cpu_info *cpu_info)
@@ -515,36 +484,23 @@ static int fill_waiting_task(char *buffer, struct cpu_info *cpu_info)
 		warn("NULL cpu_info pointer!\n");
 		return 0;
 	}
-	nr_entries = cpu_info->nr_running;
-
-	switch (config_task_format) {
-	case NEW_TASK_FORMAT:
-		cpu_info->starving = malloc(sizeof(struct task_info) * nr_entries);
-		if (cpu_info->starving == NULL) {
-			warn("failed to malloc %d task_info structs", nr_entries);
-			return 0;
-		}
-		nr_waiting = parse_new_task_format(buffer, cpu_info->starving, nr_entries);
-		break;
-	case OLD_TASK_FORMAT:
-		/*
-		 * The old task format does not output a correct value for
-		 * nr_running (the initializer for nr_entries) so count the
-		 * task lines for this CPU data and use that instead.
-		 */
+
+	if (config_task_format == OLD_TASK_FORMAT)
 		nr_entries = count_task_lines(buffer);
-		if (nr_entries <= 0)
-			return 0;
-		cpu_info->starving = malloc(sizeof(struct task_info) * nr_entries);
-		if (cpu_info->starving == NULL) {
-			warn("failed to malloc %d task_info structs", nr_entries);
-			return 0;
-		}
-		nr_waiting = parse_old_task_format(buffer, cpu_info->starving, nr_entries);
-		break;
-	default:
-		die("invalid value for config_task_format: %d\n", config_task_format);
+	else
+		nr_entries = cpu_info->nr_running;
+
+	if (nr_entries <= 0)
+		return 0;
+
+	cpu_info->starving = malloc(sizeof(struct task_info) * nr_entries);
+	if (cpu_info->starving == NULL) {
+		warn("failed to malloc %d task_info structs", nr_entries);
+		return 0;
 	}
+
+	nr_waiting = parse_task_lines(buffer, cpu_info->starving, nr_entries);
+
 	return nr_waiting;
 }
 
@@ -574,7 +530,7 @@ static int sched_debug_parse(struct cpu_info *cpu_info, char *buffer, size_t buf
 	}
 
 	/*
-	 * The NEW_TASK_FORMAT produces useful output values for nr_running and
+	 * NEW_TASK_FORMAT and produces useful output values for nr_running and
 	 * rt_nr_running, so in this case use them. For the old format just leave
 	 * them initialized to zero.
 	 */
@@ -613,7 +569,8 @@ static int sched_debug_has_starving_task(struct cpu_info *cpu)
 static int sched_debug_init(void)
 {
 	find_sched_debug_path();
-	config_task_format = detect_task_format();
+	if ((config_task_format = detect_task_format()) == TASK_FORMAT_UNKNOWN)
+		die("Can't handle task format!\n");
 	return 0;
 }
 
diff --git a/src/sched_debug.h b/src/sched_debug.h
index 21f9da2..4b12c39 100644
--- a/src/sched_debug.h
+++ b/src/sched_debug.h
@@ -1,6 +1,67 @@
 /* SPDX-License-Identifier: GPL-2.0-or-later */
-#define OLD_TASK_FORMAT  1
-#define NEW_TASK_FORMAT  2
 #define TASK_MARKER	"runnable tasks:"
+#define TASK_DIVIDER	"-\n"
+
+/*
+ * Over time, the various 'runnable task' output in SCHED_DEBUG has
+ * changed significantly.
+ *
+ * Depending on the version of the running kernel, the task formats can
+ * differ greatly.
+ *
+ * For example, in 3.X kernels, the sched_debug running tasks format denotes the current
+ * running task on the current CPU with a singular state label, 'R'. Other tasks do not
+ * receive a state label.
+ *
+ * example:
+ * '          task   PID         tree-key  switches  prio     wait-time             sum-exec        sum-sleep'
+ * ' ----------------------------------------------------------------------------------------------------------'
+ * '      watchdog/5    33        -8.984472       151     0         0.000000         0.535614         0.000000 0 /'
+ * ' R          less  9542      2382.087644        56   120         0.000000        16.444493         0.000000 0 /'
+ *
+ * In 4.18+ kernels, the sched_debug format running tasks format included an additional 'S'
+ * state field to denote the state of the running tasks on said CPU.
+ *
+ * example:
+ * ' S           task   PID         tree-key  switches  prio     wait-time             sum-exec        sum-sleep'
+ * '-----------------------------------------------------------------------------------------------------------'
+ * ' I         rcu_gp     3        13.973264         2   100         0.000000         0.004469         0.000000 0 0 /'
+ *
+ * Introduced in 6.12+, 2cab4bd024d2 sched/debug: Fix the runnable tasks
+ * output, the sched_debug running tasks format was changed to include
+ * four new EEVDF fields.
+ *
+ * Example:
+ *  'S            task   PID       vruntime   eligible    deadline             slice          sum-exec      switches  prio         wait-time        sum-sleep       sum-block  node   group-id  group-path'
+ *  '-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------'
+ *  ' I kworker/R-rcu_g     4        -1.048576   E          -1.040501           0.700000         0.000000         2     100         0.000000         0.000000         0.000000   0      0        /'
+ *
+ * As there are considerable differences in the location of the fields
+ * needed to boost task prioriy, handle the logical code differences with
+ * an enumerated type.
+ */
+enum task_format {
+	TASK_FORMAT_UNKNOWN =0,
+	OLD_TASK_FORMAT,	// 3.10 kernel
+	NEW_TASK_FORMAT,	// 4.18+ kernel
+	TASK_FORMAT_LIMIT
+};
+
+
+/*
+ * set of offsets in a task format line based on offsets
+ * discovered by discover_task_format
+ *
+ * Note: These are *NOT* character offsets, these are "word" offsets.
+ * Requiring consumers of this struct to parse through the individual
+ * lines.
+ */
+struct task_format_offsets {
+	int task;
+	int pid;
+	int switches;
+	int prio;
+	int wait_time;
+};
 
 extern struct stalld_backend sched_debug_backend;
-- 
2.50.0

[PATCH 2/3] Makefile: Add support for legacy kernels

[Derek Barbosa]

In the sched_debug backend code, we conditionally parse the running
tasks block of the sched_debug file to support the older task format (as
denoted by OLD_TASK_FORMAT).

This task format is only present on legacy 3.X kernels. However, on said
legacy kernels, the current Makefile directives and #if macros in the
stalld.c code do not fully support compilation of stalld.

Add a LEGACY check to the makefile, which parses `uname -r` output for
the major kernel version, and adds a LEGACY compilation flag to the
CFLAGS. Use the compilation flag to default to sched_debug_backend in
the stalld.c file to avoid missing references to the queue_track code.

Also, disable Intel CET -fcf-protection and BPF support, remove
annocheck targets, and default to the c99 compiler (if present).

Signed-off-by: Derek Barbosa <debarbos@redhat.com>
---
 Makefile     | 40 +++++++++++++++++++++++++++++++++++++++-
 src/stalld.c |  6 ++++--
 2 files changed, 43 insertions(+), 3 deletions(-)

diff --git a/Makefile b/Makefile
index 7234a46..c09d574 100644
--- a/Makefile
+++ b/Makefile
@@ -9,6 +9,14 @@ ARCH=$(shell uname -m)
 endif
 $(info ARCH=$(ARCH))
 
+LEGACY_VER := 3
+ifeq ($(strip $(KERNEL_VER)),)
+KERNEL_VER=$(shell uname -r | cut -f 1 -d .)
+endif
+$(info Kernel Major Version is $(KERNEL_VER))
+
+IS_LEGACY:= $(shell test $(KERNEL_VER) -le $(LEGACY_VER) && echo "true" || echo "false")
+
 USE_BPF := 1
 FCF_PROTECTION := -fcf-protection
 MTUNE	:= -mtune=generic
@@ -33,6 +41,10 @@ ifeq ($(ARCH),powerpc)
 USE_BPF := 0
 MTUNE := -mtune=powerpc
 endif
+ifeq ($(IS_LEGACY),true)
+USE_BPF := 0
+FCF_PROTECTION := 
+endif
 
 $(info USE_BPF=$(USE_BPF))
 $(info FCF_PROTECTION=$(FCF_PROTECTION))
@@ -57,6 +69,7 @@ DEFS	:=	-DUSE_BPF=$(USE_BPF) -D_FORTIFY_SOURCE=3 -D_GLIBCXX_ASSERTIONS -DDEBUG_S
 
 # note that RPMCFLAGS and RPMLDFLAGS are variables that come from the specfile when
 # building for Fedora/CentOS/RHEL/et al
+#
 
 ifeq ($(RPMCFLAGS),)
 CFLAGS	:=	-DVERSION=\"$(VERSION)\" $(FOPTS) $(MOPTS) $(WOPTS) $(SOPTS) $(DEFS)
@@ -64,6 +77,12 @@ else
 CFLAGS	:=	 $(RPMCFLAGS) $(DEFS)
 endif
 
+ifeq ($(IS_LEGACY),true)
+$(info Compiling with Legacy Mode enabled)
+$(info Overwriting RPMCFLAGS...)
+CFLAGS	:=	-DVERSION=\"$(VERSION)\" $(FOPTS) $(MOPTS) $(WOPTS) $(DEFS) -std=c99 -DLEGACY
+endif
+
 ifeq ($(DEBUG),0)
 CFLAGS	+=	-g -O2
 else
@@ -200,7 +219,7 @@ uninstall:
 	rm -f $(DESTDIR)$(BINDIR)/throttlectl
 	make -C systemd DESTDIR=$(INSPATH) uninstall
 
-.PHONY: clean tarball systemd push annocheck
+.PHONY: clean tarball systemd push annocheck help
 clean:
 	@test ! -f stalld || rm stalld
 	@test ! -f stalld-static || rm stalld-static
@@ -221,3 +240,22 @@ tarball:  clean
 
 annocheck: stalld
 	annocheck --ignore-unknown --verbose --profile=el10 --debug-dir=/usr/lib/debug/ ./stalld
+
+help:
+	@echo  'Cleaning targets:'
+	@echo  '  clean            - Clean the main executable, tests, objects, and tarballs.'
+	@echo  ''
+	@echo 'Building targets:'
+	@echo '  stalld            - Build the main executable (stalld).'
+	@echo '  static            - Build a statically linked executable (stalld-static).'
+	@echo '  tests             - Run tests in the "tests" subdirectory.'
+	@echo '  tarball           - Create a source tarball: $(NAME)-$(VERSION).tar.$(CEXT)'
+	@echo ''
+	@echo 'Installation targets:'
+	@echo '  install           - Install the executable, man page, docs, and licenses.'
+	@echo '  uninstall         - Remove all installed files.'
+	@echo ''
+	@echo 'Utility targets:'
+	@echo '  annocheck         - Run the annocheck security analysis tool on the stalld executable.'
+	@echo '  help              - Display this help message.'
+
diff --git a/src/stalld.c b/src/stalld.c
index 4c00158..b47d21c 100644
--- a/src/stalld.c
+++ b/src/stalld.c
@@ -153,14 +153,16 @@ int config_reservation = 0;
 
 /*
  * Select a backend.
- * Note that eBPF not supported on i686 and powerpc
+ * Note that eBPF not supported on i686, powerpc, and on x86 3.X kernels.
  */
-#if !__powerpc__ && !__i386__
+#if !__powerpc__ && !__i386__ && !LEGACY
 struct stalld_backend *backend = &queue_track_backend;
 #else
 struct stalld_backend *backend = &sched_debug_backend;
 #endif
 
+
+
 /*
  * Set of CPUs in which stalld should run.
  */
-- 
2.50.0

Re: [PATCH 2/3] Makefile: Add support for legacy kernels

[Clark Williams]

On Thu, Oct 02, 2025 at 04:55:14PM -0400, Derek Barbosa wrote:
> In the sched_debug backend code, we conditionally parse the running
> tasks block of the sched_debug file to support the older task format (as
> denoted by OLD_TASK_FORMAT).
> 
> This task format is only present on legacy 3.X kernels. However, on said
> legacy kernels, the current Makefile directives and #if macros in the
> stalld.c code do not fully support compilation of stalld.
> 
> Add a LEGACY check to the makefile, which parses `uname -r` output for
> the major kernel version, and adds a LEGACY compilation flag to the
> CFLAGS. Use the compilation flag to default to sched_debug_backend in
> the stalld.c file to avoid missing references to the queue_track code.
> 
> Also, disable Intel CET -fcf-protection and BPF support, remove
> annocheck targets, and default to the c99 compiler (if present).
> 
> Signed-off-by: Derek Barbosa <debarbos@redhat.com>
Signed-off-by: Clark Williams <williams@redhat.com>

Thanks for the fixes!
Clark

-- 
The United States Coast Guard
Ruining Natural Selection since 1790

Re: [PATCH 2/3] Makefile: Add support for legacy kernels

[Crystal Wood]

On Thu, 2025-10-02 at 16:55 -0400, Derek Barbosa wrote:
> In the sched_debug backend code, we conditionally parse the running
> tasks block of the sched_debug file to support the older task format (as
> denoted by OLD_TASK_FORMAT).
> 
> This task format is only present on legacy 3.X kernels. However, on said
> legacy kernels, the current Makefile directives and #if macros in the
> stalld.c code do not fully support compilation of stalld.

What are the build issues?

BTW, I was pretty confused by this at first since I didn't see any
such things in the current stalld code, because when I cloned the repo
it gave me the old master branch by default (last update 5 years ago)
instead of main. :-P

> Add a LEGACY check to the makefile, which parses `uname -r` output for
> the major kernel version, and adds a LEGACY compilation flag to the
> CFLAGS. Use the compilation flag to default to sched_debug_backend in
> the stalld.c file to avoid missing references to the queue_track code.

As patch 1/3 says in a couple of places, this format wasn't introduced
in 4.0 exactly (it says 4.18, AFAICT it was actually 4.14).  So why are
we focusing on the major release (which basically signifies nothing
other than "we don't like minor numbers greater than 20")?

It's generally better to test features than version numbers anyway --
especially for a component that is prone to feature backports. :-)

The queue_track issue seems to be related to BPF, not the task format.

> Also, disable Intel CET -fcf-protection and BPF support, remove
> annocheck targets, and default to the c99 compiler (if present).

How are these related to the kernel version?  And I don't see any
changes to annocheck.

> Signed-off-by: Derek Barbosa <debarbos@redhat.com>
> ---
>  Makefile     | 40 +++++++++++++++++++++++++++++++++++++++-
>  src/stalld.c |  6 ++++--
>  2 files changed, 43 insertions(+), 3 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 7234a46..c09d574 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -9,6 +9,14 @@ ARCH=$(shell uname -m)
>  endif
>  $(info ARCH=$(ARCH))
>  
> +LEGACY_VER := 3
> +ifeq ($(strip $(KERNEL_VER)),)
> +KERNEL_VER=$(shell uname -r | cut -f 1 -d .)
> +endif
> +$(info Kernel Major Version is $(KERNEL_VER))
> +
> +IS_LEGACY:= $(shell test $(KERNEL_VER) -le $(LEGACY_VER) && echo "true" || echo "false")

What happens when we need to test for a feature that isn't even loosely
associated with 3.x versus 4.x+?  Why are all these things being lumped
under one vague "legacy" label?

>  USE_BPF := 1
>  FCF_PROTECTION := -fcf-protection
>  MTUNE	:= -mtune=generic
> @@ -33,6 +41,10 @@ ifeq ($(ARCH),powerpc)
>  USE_BPF := 0
>  MTUNE := -mtune=powerpc
>  endif
> +ifeq ($(IS_LEGACY),true)
> +USE_BPF := 0
> +FCF_PROTECTION := 
> +endif
>  
>  $(info USE_BPF=$(USE_BPF))
>  $(info FCF_PROTECTION=$(FCF_PROTECTION))
> @@ -57,6 +69,7 @@ DEFS	:=	-DUSE_BPF=$(USE_BPF) -D_FORTIFY_SOURCE=3 -D_GLIBCXX_ASSERTIONS -DDEBUG_S
>  
>  # note that RPMCFLAGS and RPMLDFLAGS are variables that come from the specfile when
>  # building for Fedora/CentOS/RHEL/et al
> +#
>  
>  ifeq ($(RPMCFLAGS),)
>  CFLAGS	:=	-DVERSION=\"$(VERSION)\" $(FOPTS) $(MOPTS) $(WOPTS) $(SOPTS) $(DEFS)
> @@ -64,6 +77,12 @@ else
>  CFLAGS	:=	 $(RPMCFLAGS) $(DEFS)
>  endif
>  
> +ifeq ($(IS_LEGACY),true)
> +$(info Compiling with Legacy Mode enabled)
> +$(info Overwriting RPMCFLAGS...)
> +CFLAGS	:=	-DVERSION=\"$(VERSION)\" $(FOPTS) $(MOPTS) $(WOPTS) $(DEFS) -std=c99 -DLEGACY
> +endif

It's overwriting CFLAGS, not RPMCFLAGS.  And if both RPMCFLAGS and
LEGACY are set, it looks like this ignores RPMCFLAGS.

Why is SOPTS dropped?  The closest thing I can see in the changelog is
-fcp-protection, but that's part of FOPTS (along with many other
things).

Speaking of SOPTS, is this tool intended to be Red Hat specific
(and require redhat-rpm-config even if no RPM building is intended)?

> @@ -200,7 +219,7 @@ uninstall:
>  	rm -f $(DESTDIR)$(BINDIR)/throttlectl
>  	make -C systemd DESTDIR=$(INSPATH) uninstall
>  
> -.PHONY: clean tarball systemd push annocheck
> +.PHONY: clean tarball systemd push annocheck help
>  clean:
>  	@test ! -f stalld || rm stalld
>  	@test ! -f stalld-static || rm stalld-static
> @@ -221,3 +240,22 @@ tarball:  clean
>  
>  annocheck: stalld
>  	annocheck --ignore-unknown --verbose --profile=el10 --debug-dir=/usr/lib/debug/ ./stalld
> +
> +help:
> +	@echo  'Cleaning targets:'
> +	@echo  '  clean            - Clean the main executable, tests, objects, and tarballs.'
> +	@echo  ''
> +	@echo 'Building targets:'
> +	@echo '  stalld            - Build the main executable (stalld).'
> +	@echo '  static            - Build a statically linked executable (stalld-static).'
> +	@echo '  tests             - Run tests in the "tests" subdirectory.'
> +	@echo '  tarball           - Create a source tarball: $(NAME)-$(VERSION).tar.$(CEXT)'
> +	@echo ''
> +	@echo 'Installation targets:'
> +	@echo '  install           - Install the executable, man page, docs, and licenses.'
> +	@echo '  uninstall         - Remove all installed files.'
> +	@echo ''
> +	@echo 'Utility targets:'
> +	@echo '  annocheck         - Run the annocheck security analysis tool on the stalld executable.'
> +	@echo '  help              - Display this help message.'
> +

This seems unrelated?

> diff --git a/src/stalld.c b/src/stalld.c
> index 4c00158..b47d21c 100644
> --- a/src/stalld.c
> +++ b/src/stalld.c
> @@ -153,14 +153,16 @@ int config_reservation = 0;
>  
>  /*
>   * Select a backend.
> - * Note that eBPF not supported on i686 and powerpc
> + * Note that eBPF not supported on i686, powerpc, and on x86 3.X kernels.
>   */
> -#if !__powerpc__ && !__i386__
> +#if !__powerpc__ && !__i386__ && !LEGACY
>  struct stalld_backend *backend = &queue_track_backend;
>  #else
>  struct stalld_backend *backend = &sched_debug_backend;
>  #endif

The file is guarded by USE_BPF, so why not use that here too?

-Crystal

Re: [PATCH 2/3] Makefile: Add support for legacy kernels

[Derek Barbosa]

On Tue, Oct 07, 2025 at 01:50:57PM -0500, Crystal Wood wrote:
> On Thu, 2025-10-02 at 16:55 -0400, Derek Barbosa wrote:
> > In the sched_debug backend code, we conditionally parse the running
> > tasks block of the sched_debug file to support the older task format (as
> > denoted by OLD_TASK_FORMAT).
> > 
> > This task format is only present on legacy 3.X kernels. However, on said
> > legacy kernels, the current Makefile directives and #if macros in the
> > stalld.c code do not fully support compilation of stalld.
> 
> What are the build issues?
> 
> BTW, I was pretty confused by this at first since I didn't see any
> such things in the current stalld code, because when I cloned the repo
> it gave me the old master branch by default (last update 5 years ago)
> instead of main. :-P
> 
> > Add a LEGACY check to the makefile, which parses `uname -r` output for
> > the major kernel version, and adds a LEGACY compilation flag to the
> > CFLAGS. Use the compilation flag to default to sched_debug_backend in
> > the stalld.c file to avoid missing references to the queue_track code.
> 
> As patch 1/3 says in a couple of places, this format wasn't introduced
> in 4.0 exactly (it says 4.18, AFAICT it was actually 4.14).  So why are
> we focusing on the major release (which basically signifies nothing
> other than "we don't like minor numbers greater than 20")?
> 
Thanks for the clarification!

Truth be told this was a hack that I had tested to ensure that I could build
this on some (very) old kernels.

As for the error messages, I need to re-install a 3.10 kernel on compatible box
again to pull out exactly what I was seeing.

In general, everything that was hamfistedly cut-out --as you may have
noticed-- was preventing the compilation of stalld.

> It's generally better to test features than version numbers anyway --
> especially for a component that is prone to feature backports. :-)

Touche. I need to go back and clean it up :)

> 
> The queue_track issue seems to be related to BPF, not the task format.
> 

Right, I'm pretty sure I mentioned that in my commit message. The old task
format is pretty much only present on _really_ old kernels. So, in this patch
series we attempt to unify the different parsing techniques.

After doing so, I decided to test my changes to ensure they actually worked on
the aforementioned kernels.

It was here that I noticed that the queue_track backend was still being
referenced, even with USE_BPF being set to 0 when running the `make` commands,
which would filter out the files themselves.

```
ifeq ($(USE_BPF),0)
SRC	:=	$(filter-out src/queue_track.c, $(SRC))
HDR	:=	$(filter-out src/queue_track.h, $(SRC))
endif
```
There were some undefined reference errors.

```
#if !__powerpc__ && !__i386__
struct stalld_backend *backend = &queue_track_backend;
#else
struct stalld_backend *backend = &sched_debug_backend;
#endif
```

We don't actually check if USE_BPF is enabled here.


> > Also, disable Intel CET -fcf-protection and BPF support, remove
> > annocheck targets, and default to the c99 compiler (if present).
> 
> How are these related to the kernel version?  And I don't see any
> changes to annocheck.
> 

If I am not mistaken, CET [0][1] is fairly recent. Attempting to build with this
flag on a RHEL-7 box (I need to check the gcc version again) was met with an
"unknown option".


> > Signed-off-by: Derek Barbosa <debarbos@redhat.com>
> > ---
> >  Makefile     | 40 +++++++++++++++++++++++++++++++++++++++-
> >  src/stalld.c |  6 ++++--
> >  2 files changed, 43 insertions(+), 3 deletions(-)
> > 
> > diff --git a/Makefile b/Makefile
> > index 7234a46..c09d574 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -9,6 +9,14 @@ ARCH=$(shell uname -m)
> >  endif
> >  $(info ARCH=$(ARCH))
> >  
> > +LEGACY_VER := 3
> > +ifeq ($(strip $(KERNEL_VER)),)
> > +KERNEL_VER=$(shell uname -r | cut -f 1 -d .)
> > +endif
> > +$(info Kernel Major Version is $(KERNEL_VER))
> > +
> > +IS_LEGACY:= $(shell test $(KERNEL_VER) -le $(LEGACY_VER) && echo "true" || echo "false")
> 
> What happens when we need to test for a feature that isn't even loosely
> associated with 3.x versus 4.x+?  Why are all these things being lumped
> under one vague "legacy" label?
> 

What other features would we be testing for, outside of the ones disabled for
compatability purposes?

> >  USE_BPF := 1
> >  FCF_PROTECTION := -fcf-protection
> >  MTUNE	:= -mtune=generic
> > @@ -33,6 +41,10 @@ ifeq ($(ARCH),powerpc)
> >  USE_BPF := 0
> >  MTUNE := -mtune=powerpc
> >  endif
> > +ifeq ($(IS_LEGACY),true)
> > +USE_BPF := 0
> > +FCF_PROTECTION := 
> > +endif

I realize that I'm going about this the wrong way. This will be fixed in the
next version.

> >  
> >  $(info USE_BPF=$(USE_BPF))
> >  $(info FCF_PROTECTION=$(FCF_PROTECTION))
> > @@ -57,6 +69,7 @@ DEFS	:=	-DUSE_BPF=$(USE_BPF) -D_FORTIFY_SOURCE=3 -D_GLIBCXX_ASSERTIONS -DDEBUG_S
> >  
> >  # note that RPMCFLAGS and RPMLDFLAGS are variables that come from the specfile when
> >  # building for Fedora/CentOS/RHEL/et al
> > +#
> >  
> >  ifeq ($(RPMCFLAGS),)
> >  CFLAGS	:=	-DVERSION=\"$(VERSION)\" $(FOPTS) $(MOPTS) $(WOPTS) $(SOPTS) $(DEFS)
> > @@ -64,6 +77,12 @@ else
> >  CFLAGS	:=	 $(RPMCFLAGS) $(DEFS)
> >  endif
> >  
> > +ifeq ($(IS_LEGACY),true)
> > +$(info Compiling with Legacy Mode enabled)
> > +$(info Overwriting RPMCFLAGS...)
> > +CFLAGS	:=	-DVERSION=\"$(VERSION)\" $(FOPTS) $(MOPTS) $(WOPTS) $(DEFS) -std=c99 -DLEGACY
> > +endif
> 
> It's overwriting CFLAGS, not RPMCFLAGS.  And if both RPMCFLAGS and
> LEGACY are set, it looks like this ignores RPMCFLAGS.
> 
> Why is SOPTS dropped?  The closest thing I can see in the changelog is
> -fcp-protection, but that's part of FOPTS (along with many other
> things).

```
SOPTS	:= 	-specs=/usr/lib/rpm/redhat/redhat-hardened-cc1
-specs=/usr/lib/rpm/redhat/redhat-annobin-cc1
```
When building on RHEL-7, it was though SOPTS still make their way through, even
though the comment specifies that these are variables that only come from the
specfile for DNF-family distributions.I was getting errors due to annobin not
being present, so I stripped it out.

> 
> Speaking of SOPTS, is this tool intended to be Red Hat specific
> (and require redhat-rpm-config even if no RPM building is intended)?
> 

That I am somewhat fuzzy on. Clark, could you help me out here? :)

> > @@ -200,7 +219,7 @@ uninstall:
> >  	rm -f $(DESTDIR)$(BINDIR)/throttlectl
> >  	make -C systemd DESTDIR=$(INSPATH) uninstall
> >  
> > -.PHONY: clean tarball systemd push annocheck
> > +.PHONY: clean tarball systemd push annocheck help
> >  clean:
> >  	@test ! -f stalld || rm stalld
> >  	@test ! -f stalld-static || rm stalld-static
> > @@ -221,3 +240,22 @@ tarball:  clean
> >  
> >  annocheck: stalld
> >  	annocheck --ignore-unknown --verbose --profile=el10 --debug-dir=/usr/lib/debug/ ./stalld
> > +
> > +help:
> > +	@echo  'Cleaning targets:'
> > +	@echo  '  clean            - Clean the main executable, tests, objects, and tarballs.'
> > +	@echo  ''
> > +	@echo 'Building targets:'
> > +	@echo '  stalld            - Build the main executable (stalld).'
> > +	@echo '  static            - Build a statically linked executable (stalld-static).'
> > +	@echo '  tests             - Run tests in the "tests" subdirectory.'
> > +	@echo '  tarball           - Create a source tarball: $(NAME)-$(VERSION).tar.$(CEXT)'
> > +	@echo ''
> > +	@echo 'Installation targets:'
> > +	@echo '  install           - Install the executable, man page, docs, and licenses.'
> > +	@echo '  uninstall         - Remove all installed files.'
> > +	@echo ''
> > +	@echo 'Utility targets:'
> > +	@echo '  annocheck         - Run the annocheck security analysis tool on the stalld executable.'
> > +	@echo '  help              - Display this help message.'
> > +
> 
> This seems unrelated?
> 

Sorry, I am unsure what you're referring to. Are you saying the addition of a
`make help` target is unrelated?

> > diff --git a/src/stalld.c b/src/stalld.c
> > index 4c00158..b47d21c 100644
> > --- a/src/stalld.c
> > +++ b/src/stalld.c
> > @@ -153,14 +153,16 @@ int config_reservation = 0;
> >  
> >  /*
> >   * Select a backend.
> > - * Note that eBPF not supported on i686 and powerpc
> > + * Note that eBPF not supported on i686, powerpc, and on x86 3.X kernels.
> >   */
> > -#if !__powerpc__ && !__i386__
> > +#if !__powerpc__ && !__i386__ && !LEGACY
> >  struct stalld_backend *backend = &queue_track_backend;
> >  #else
> >  struct stalld_backend *backend = &sched_debug_backend;
> >  #endif
> 
> The file is guarded by USE_BPF, so why not use that here too?

Thanks for pointing that out.

> 
> -Crystal
> 

[0] https://docs.kernel.org/next/x86/shstk.html
[1] https://www.phoronix.com/news/Linux-6.4-Shadow-Stack-Coming

-- 
Derek <debarbos@redhat.com>

Re: [PATCH 2/3] Makefile: Add support for legacy kernels

[Crystal Wood]

On Wed, 2025-10-08 at 09:07 -0400, Derek Barbosa wrote:
> On Tue, Oct 07, 2025 at 01:50:57PM -0500, Crystal Wood wrote:
> > On Thu, 2025-10-02 at 16:55 -0400, Derek Barbosa wrote:
> > > Also, disable Intel CET -fcf-protection and BPF support, remove
> > > annocheck targets, and default to the c99 compiler (if present).
> > 
> > How are these related to the kernel version?  And I don't see any
> > changes to annocheck.
> > 
> 
> If I am not mistaken, CET [0][1] is fairly recent. Attempting to build with this
> flag on a RHEL-7 box (I need to check the gcc version again) was met with an
> "unknown option".

OK but that's a GCC version issue, not a kernel version issue.

I still don't get why we only use c99 for legacy builds... does
something in the non-legacy build need a newer standard that is now
default?

> > > Signed-off-by: Derek Barbosa <debarbos@redhat.com>
> > > ---
> > >  Makefile     | 40 +++++++++++++++++++++++++++++++++++++++-
> > >  src/stalld.c |  6 ++++--
> > >  2 files changed, 43 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/Makefile b/Makefile
> > > index 7234a46..c09d574 100644
> > > --- a/Makefile
> > > +++ b/Makefile
> > > @@ -9,6 +9,14 @@ ARCH=$(shell uname -m)
> > >  endif
> > >  $(info ARCH=$(ARCH))
> > >  
> > > +LEGACY_VER := 3
> > > +ifeq ($(strip $(KERNEL_VER)),)
> > > +KERNEL_VER=$(shell uname -r | cut -f 1 -d .)
> > > +endif
> > > +$(info Kernel Major Version is $(KERNEL_VER))
> > > +
> > > +IS_LEGACY:= $(shell test $(KERNEL_VER) -le $(LEGACY_VER) && echo "true" || echo "false")
> > 
> > What happens when we need to test for a feature that isn't even loosely
> > associated with 3.x versus 4.x+?  Why are all these things being lumped
> > under one vague "legacy" label?
> > 
> 
> What other features would we be testing for, outside of the ones disabled for
> compatability purposes?

I just meant that lumping all compatibility flags into one big "LEGACY"
made me twitch. :-)

If we're going to do that anyway (e.g. to simplify testing) I'd at
least have a comment indicating that this is being used as a heuristic
for older systems in general to avoid the need for a more complicated
build system (at least for now), to save readers some head scratching.

> > > +ifeq ($(IS_LEGACY),true)
> > > +$(info Compiling with Legacy Mode enabled)
> > > +$(info Overwriting RPMCFLAGS...)
> > > +CFLAGS	:=	-DVERSION=\"$(VERSION)\" $(FOPTS) $(MOPTS) $(WOPTS) $(DEFS) -std=c99 -DLEGACY
> > > +endif
> > 
> > It's overwriting CFLAGS, not RPMCFLAGS.  And if both RPMCFLAGS and
> > LEGACY are set, it looks like this ignores RPMCFLAGS.

I think my confusion here was that it's overriding rather than
overwriting.  If we do end up doing this, a comment explaining why
would be helpful.

> > 
> > Why is SOPTS dropped?  The closest thing I can see in the changelog is
> > -fcp-protection, but that's part of FOPTS (along with many other
> > things).
> 
> ```
> SOPTS	:= 	-specs=/usr/lib/rpm/redhat/redhat-hardened-cc1
> -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1
> ```
> When building on RHEL-7, it was though SOPTS still make their way through, even
> though the comment specifies that these are variables that only come from the
> specfile for DNF-family distributions.I was getting errors due to annobin not
> being present, so I stripped it out.

Oh, I was looking for "annocheck" which explains why I didn't notice
that.

So the issue is that when you *are* building an RPM, the specfile isn't
passing the right things for that OS version?  Wouldn't that be an
issue with the specfile?

> > > @@ -221,3 +240,22 @@ tarball:  clean
> > >  
> > >  annocheck: stalld
> > >  	annocheck --ignore-unknown --verbose --profile=el10 --debug-dir=/usr/lib/debug/ ./stalld
> > > +
> > > +help:
> > > +	@echo  'Cleaning targets:'
> > > +	@echo  '  clean            - Clean the main executable, tests, objects, and tarballs.'
> > > +	@echo  ''
> > > +	@echo 'Building targets:'
> > > +	@echo '  stalld            - Build the main executable (stalld).'
> > > +	@echo '  static            - Build a statically linked executable (stalld-static).'
> > > +	@echo '  tests             - Run tests in the "tests" subdirectory.'
> > > +	@echo '  tarball           - Create a source tarball: $(NAME)-$(VERSION).tar.$(CEXT)'
> > > +	@echo ''
> > > +	@echo 'Installation targets:'
> > > +	@echo '  install           - Install the executable, man page, docs, and licenses.'
> > > +	@echo '  uninstall         - Remove all installed files.'
> > > +	@echo ''
> > > +	@echo 'Utility targets:'
> > > +	@echo '  annocheck         - Run the annocheck security analysis tool on the stalld executable.'
> > > +	@echo '  help              - Display this help message.'
> > > +
> > 
> > This seems unrelated?
> > 
> 
> Sorry, I am unsure what you're referring to. Are you saying the addition of a
> `make help` target is unrelated?

Yes, it seemed like something that would normally be a separate
patch, though that may be me getting stuck in kernel development mode
:-)

I wouldn't have mentioned it if the patch didn't feel off in other
ways.

-Crystal

Re: [PATCH 2/3] Makefile: Add support for legacy kernels

[Derek Barbosa]

On Wed, Oct 08, 2025 at 11:50:11AM -0500, Crystal Wood wrote:
> On Wed, 2025-10-08 at 09:07 -0400, Derek Barbosa wrote:
> > On Tue, Oct 07, 2025 at 01:50:57PM -0500, Crystal Wood wrote:
> > > On Thu, 2025-10-02 at 16:55 -0400, Derek Barbosa wrote:
> > > > Also, disable Intel CET -fcf-protection and BPF support, remove
> > > > annocheck targets, and default to the c99 compiler (if present).
> > > 
> > > How are these related to the kernel version?  And I don't see any
> > > changes to annocheck.
> > > 
> > 
> > If I am not mistaken, CET [0][1] is fairly recent. Attempting to build with this
> > flag on a RHEL-7 box (I need to check the gcc version again) was met with an
> > "unknown option".
> 
> OK but that's a GCC version issue, not a kernel version issue.

Sorry for conflating the two.

> I still don't get why we only use c99 for legacy builds... does
> something in the non-legacy build need a newer standard that is now
> default?

Let me spin up an old kernel.

```
[root@rt-qe-06 stalld]# gcc --version
gcc (GCC) 4.8.5 20150623 (Red Hat 4.8.5-44)
Copyright (C) 2015 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
```

IIRC, GCC 4.8.5 has incomplete support for c99, and the default is gnu90 [0].
> The default, if no C language dialect options are given, is -std=gnu90; this
> will change to -std=gnu99 or -std=gnu11 in some future release when the C99 or
> C11 support is complete.


> > > > Signed-off-by: Derek Barbosa <debarbos@redhat.com>
> > > > ---
> > > >  Makefile     | 40 +++++++++++++++++++++++++++++++++++++++-
> > > >  src/stalld.c |  6 ++++--
> > > >  2 files changed, 43 insertions(+), 3 deletions(-)
> > > > 
> > > > diff --git a/Makefile b/Makefile
> > > > index 7234a46..c09d574 100644
> > > > --- a/Makefile
> > > > +++ b/Makefile
> > > > @@ -9,6 +9,14 @@ ARCH=$(shell uname -m)
> > > >  endif
> > > >  $(info ARCH=$(ARCH))
> > > >  
> > > > +LEGACY_VER := 3
> > > > +ifeq ($(strip $(KERNEL_VER)),)
> > > > +KERNEL_VER=$(shell uname -r | cut -f 1 -d .)
> > > > +endif
> > > > +$(info Kernel Major Version is $(KERNEL_VER))
> > > > +
> > > > +IS_LEGACY:= $(shell test $(KERNEL_VER) -le $(LEGACY_VER) && echo "true" || echo "false")
> > > 
> > > What happens when we need to test for a feature that isn't even loosely
> > > associated with 3.x versus 4.x+?  Why are all these things being lumped
> > > under one vague "legacy" label?
> > > 
> > 
> > What other features would we be testing for, outside of the ones disabled for
> > compatability purposes?
> 
> I just meant that lumping all compatibility flags into one big "LEGACY"
> made me twitch. :-)

Fair. I will openly admit that it isn't "clean". It is a hack at best.
"LEGACY" is also a misnomer in the sense that users could interpret it as any
kernel version that isn't actively supported...

The reasoning came about that StallD, as is, doesn't compile on pretty old
environments such as RHEL-7, which runs a 3.10 franken-kernel. Considering the
code has config flags to determine whether or not the sched_debug file is of
"old" format or "new" format, I figured we want to support such environments.

I made the incorrect assumption that systems running such "old" kernels, would
by extension have equally-as-old compilers.

Of course, we could always go the direction of just dropping the functionality
of parsing said "old" sched_debug files. Clark said that this may make some
people upset though :P

> 
> If we're going to do that anyway (e.g. to simplify testing) I'd at
> least have a comment indicating that this is being used as a heuristic
> for older systems in general to avoid the need for a more complicated
> build system (at least for now), to save readers some head scratching.
> 
Agreed. I'll clean this up and add some more comments soon.

> > > > +ifeq ($(IS_LEGACY),true)
> > > > +$(info Compiling with Legacy Mode enabled)
> > > > +$(info Overwriting RPMCFLAGS...)
> > > > +CFLAGS	:=	-DVERSION=\"$(VERSION)\" $(FOPTS) $(MOPTS) $(WOPTS) $(DEFS) -std=c99 -DLEGACY
> > > > +endif
> > > 
> > > It's overwriting CFLAGS, not RPMCFLAGS.  And if both RPMCFLAGS and
> > > LEGACY are set, it looks like this ignores RPMCFLAGS.
> 
> I think my confusion here was that it's overriding rather than
> overwriting.  If we do end up doing this, a comment explaining why
> would be helpful.
> 
> > > 
> > > Why is SOPTS dropped?  The closest thing I can see in the changelog is
> > > -fcp-protection, but that's part of FOPTS (along with many other
> > > things).
> > 
> > ```
> > SOPTS	:= 	-specs=/usr/lib/rpm/redhat/redhat-hardened-cc1
> > -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1
> > ```
> > When building on RHEL-7, it was though SOPTS still make their way through, even
> > though the comment specifies that these are variables that only come from the
> > specfile for DNF-family distributions.I was getting errors due to annobin not
> > being present, so I stripped it out.
> 
> Oh, I was looking for "annocheck" which explains why I didn't notice
> that.
> 
> So the issue is that when you *are* building an RPM, the specfile isn't
> passing the right things for that OS version?  Wouldn't that be an
> issue with the specfile?
> 

It would be. However, annocheck/annobin gets invoked by default `make`.

```
ifeq ($(RPMCFLAGS),)
CFLAGS  :=      -DVERSION=\"$(VERSION)\" $(FOPTS) $(MOPTS) $(WOPTS) $(SOPTS) $(DEFS)
else
CFLAGS  :=       $(RPMCFLAGS) $(DEFS)
endif
```

On `main`, with just disabling USE_BPF:

```
[root@rt-qe-06 stalld]# make USE_BPF=0
ARCH=x86_64
USE_BPF=0
FCF_PROTECTION=-fcf-protection
MTUNE=-mtune=generic
CFLAGS=-DVERSION=\"1.23.1\" -flto=auto -ffat-lto-objects -fexceptions -fstack-protector-strong -fasynchronous-unwind-tables -fstack-clash-protection -fno-omit-frame-pointer -fcf-protection -fpie -mtune=generic -m64 -mno-omit-leaf-frame-pointer -Wall -Werror=format-security -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -DUSE_BPF=0 -D_FORTIFY_SOURCE=3 -D_GLIBCXX_ASSERTIONS -DDEBUG_STALLD=0 -g -O2
LDFLAGS=-ggdb -znow -pie
gcc -DVERSION=\"1.23.1\" -flto=auto -ffat-lto-objects -fexceptions -fstack-protector-strong -fasynchronous-unwind-tables -fstack-clash-protection -fno-omit-frame-pointer -fcf-protection -fpie -mtune=generic -m64 -mno-omit-leaf-frame-pointer -Wall -Werror=format-security -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -DUSE_BPF=0 -D_FORTIFY_SOURCE=3 -D_GLIBCXX_ASSERTIONS -DDEBUG_STALLD=0 -g -O2   -c -o src/stalld.o src/stalld.c
gcc: error: /usr/lib/rpm/redhat/redhat-annobin-cc1: No such file or directory
make: *** [src/stalld.o] Error 1
```

Ok, ignoring SOPTS

```
[root@rt-qe-06 stalld]# make USE_BPF=0 SOPTS=
ARCH=x86_64
USE_BPF=0
FCF_PROTECTION=-fcf-protection
MTUNE=-mtune=generic
CFLAGS=-DVERSION=\"1.23.1\" -flto=auto -ffat-lto-objects -fexceptions -fstack-protector-strong -fasynchronous-unwind-tables -fstack-clash-protection -fno-omit-frame-pointer -fcf-protection -fpie -mtune=generic -m64 -mno-omit-leaf-frame-pointer -Wall -Werror=format-security  -DUSE_BPF=0 -D_FORTIFY_SOURCE=3 -D_GLIBCXX_ASSERTIONS -DDEBUG_STALLD=0 -g -O2
LDFLAGS=-ggdb -znow -pie
gcc -DVERSION=\"1.23.1\" -flto=auto -ffat-lto-objects -fexceptions -fstack-protector-strong -fasynchronous-unwind-tables -fstack-clash-protection -fno-omit-frame-pointer -fcf-protection -fpie -mtune=generic -m64 -mno-omit-leaf-frame-pointer -Wall -Werror=format-security  -DUSE_BPF=0 -D_FORTIFY_SOURCE=3 -D_GLIBCXX_ASSERTIONS -DDEBUG_STALLD=0 -g -O2   -c -o src/stalld.o src/stalld.c
gcc: error: unrecognized command line option ‘-fcf-protection’
make: *** [src/stalld.o] Error 1
```

Ignoring FCF_PROTECTION...

```
[root@rt-qe-06 stalld]# make USE_BPF=0 SOPTS= FCF_PROTECTION=
ARCH=x86_64
USE_BPF=0
FCF_PROTECTION=
MTUNE=-mtune=generic
CFLAGS=-DVERSION=\"1.23.1\" -flto=auto -ffat-lto-objects -fexceptions -fstack-protector-strong -fasynchronous-unwind-tables -fstack-clash-protection -fno-omit-frame-pointer  -fpie -mtune=generic -m64 -mno-omit-leaf-frame-pointer -Wall -Werror=format-security  -DUSE_BPF=0 -D_FORTIFY_SOURCE=3 -D_GLIBCXX_ASSERTIONS -DDEBUG_STALLD=0 -g -O2
LDFLAGS=-ggdb -znow -pie
gcc -DVERSION=\"1.23.1\" -flto=auto -ffat-lto-objects -fexceptions -fstack-protector-strong -fasynchronous-unwind-tables -fstack-clash-protection -fno-omit-frame-pointer  -fpie -mtune=generic -m64 -mno-omit-leaf-frame-pointer -Wall -Werror=format-security  -DUSE_BPF=0 -D_FORTIFY_SOURCE=3 -D_GLIBCXX_ASSERTIONS -DDEBUG_STALLD=0 -g -O2   -c -o src/stalld.o src/stalld.c
In file included from src/stalld.c:39:0:
src/queue_track.h: In function ‘find_queued_task’:
src/queue_track.h:61:2: error: ‘for’ loop initial declarations are only allowed in C99 mode
  for (unsigned int i = 0;  \
  ^
src/queue_track.h:118:2: note: in expansion of macro ‘for_each_task_entry’
  for_each_task_entry(cpu_data, task) {
  ^
src/queue_track.h:61:2: note: use option -std=c99 or -std=gnu99 to compile your code
  for (unsigned int i = 0;  \
  ^
src/queue_track.h:118:2: note: in expansion of macro ‘for_each_task_entry’
  for_each_task_entry(cpu_data, task) {
  ^
make: *** [src/stalld.o] Error 1
```

Appending c99 to the CFLAGS on the command-line a-la `CFLAGS+=-std=c99`
overrides the cflags entirely... so editing the CFLAGS to append -std=c99..

```
[root@rt-qe-06 stalld]# make USE_BPF=0 SOPTS= FCF_PROTECTION=""
ARCH=x86_64
USE_BPF=0
FCF_PROTECTION=
MTUNE=-mtune=generic
CFLAGS=-DVERSION=\"1.23.1\" -flto=auto -ffat-lto-objects -fexceptions -fstack-protector-strong -fasynchronous-unwind-tables -fstack-clash-protection -fno-omit-frame-pointer  -fpie -mtune=generic -m64 -mno-omit-leaf-frame-pointer -Wall -Werror=format-security  -DUSE_BPF=0 -D_FORTIFY_SOURCE=3 -D_GLIBCXX_ASSERTIONS -DDEBUG_STALLD=0 -std=c99 -g -O2
LDFLAGS=-ggdb -znow -pie
gcc -DVERSION=\"1.23.1\" -flto=auto -ffat-lto-objects -fexceptions -fstack-protector-strong -fasynchronous-unwind-tables -fstack-clash-protection -fno-omit-frame-pointer  -fpie -mtune=generic -m64 -mno-omit-leaf-frame-pointer -Wall -Werror=format-security  -DUSE_BPF=0 -D_FORTIFY_SOURCE=3 -D_GLIBCXX_ASSERTIONS -DDEBUG_STALLD=0 -std=c99 -g -O2   -c -o src/stalld.o src/stalld.c
gcc -DVERSION=\"1.23.1\" -flto=auto -ffat-lto-objects -fexceptions -fstack-protector-strong -fasynchronous-unwind-tables -fstack-clash-protection -fno-omit-frame-pointer  -fpie -mtune=generic -m64 -mno-omit-leaf-frame-pointer -Wall -Werror=format-security  -DUSE_BPF=0 -D_FORTIFY_SOURCE=3 -D_GLIBCXX_ASSERTIONS -DDEBUG_STALLD=0 -std=c99 -g -O2   -c -o src/sched_debug.o src/sched_debug.c
gcc -o stalld	-ggdb -znow -pie src/stalld.o src/throttling.o src/sched_debug.o src/utils.o -lpthread
src/stalld.o:(.data.rel+0x0): undefined reference to `queue_track_backend'
collect2: error: ld returned 1 exit status
make: *** [stalld] Error 1
```

And there's where some of the hamfisted-hackery with the LEGACY flag came into
the picture.


> > > > @@ -221,3 +240,22 @@ tarball:  clean
> > > >  
> > > >  annocheck: stalld
> > > >  	annocheck --ignore-unknown --verbose --profile=el10 --debug-dir=/usr/lib/debug/ ./stalld
> > > > +
> > > > +help:
> > > > +	@echo  'Cleaning targets:'
> > > > +	@echo  '  clean            - Clean the main executable, tests, objects, and tarballs.'
> > > > +	@echo  ''
> > > > +	@echo 'Building targets:'
> > > > +	@echo '  stalld            - Build the main executable (stalld).'
> > > > +	@echo '  static            - Build a statically linked executable (stalld-static).'
> > > > +	@echo '  tests             - Run tests in the "tests" subdirectory.'
> > > > +	@echo '  tarball           - Create a source tarball: $(NAME)-$(VERSION).tar.$(CEXT)'
> > > > +	@echo ''
> > > > +	@echo 'Installation targets:'
> > > > +	@echo '  install           - Install the executable, man page, docs, and licenses.'
> > > > +	@echo '  uninstall         - Remove all installed files.'
> > > > +	@echo ''
> > > > +	@echo 'Utility targets:'
> > > > +	@echo '  annocheck         - Run the annocheck security analysis tool on the stalld executable.'
> > > > +	@echo '  help              - Display this help message.'
> > > > +
> > > 
> > > This seems unrelated?
> > > 
> > 
> > Sorry, I am unsure what you're referring to. Are you saying the addition of a
> > `make help` target is unrelated?
> 
> Yes, it seemed like something that would normally be a separate
> patch, though that may be me getting stuck in kernel development mode
> :-)
> 
> I wouldn't have mentioned it if the patch didn't feel off in other
> ways.

Fair! It is trivial for me to split this up. I'll include it in the next series.
Currently debugging a weird corner case in the new "unified" parser, so it may
take some time.

> 
> -Crystal
> 

[0] https://gcc.gnu.org/onlinedocs/gcc-4.8.5/gcc/Standards.html#Standards

-- 
Derek <debarbos@redhat.com>

Re: [PATCH 2/3] Makefile: Add support for legacy kernels

[Crystal Wood]

On Wed, 2025-10-08 at 14:41 -0400, Derek Barbosa wrote:
> On Wed, Oct 08, 2025 at 11:50:11AM -0500, Crystal Wood wrote:
> > The default, if no C language dialect options are given, is -std=gnu90; this
> > will change to -std=gnu99 or -std=gnu11 in some future release when the C99 or
> > C11 support is complete.

I meant, why can't we specify c99 regardless of legacy (which would also
help avoid introducing new stuff that breaks on legacy)?  Unless
something in queue_track.c already does need newer than c99.

> > > When building on RHEL-7, it was though SOPTS still make their way through, even
> > > though the comment specifies that these are variables that only come from the
> > > specfile for DNF-family distributions.I was getting errors due to annobin not
> > > being present, so I stripped it out.
> > 
> > Oh, I was looking for "annocheck" which explains why I didn't notice
> > that.
> > 
> > So the issue is that when you *are* building an RPM, the specfile isn't
> > passing the right things for that OS version?  Wouldn't that be an
> > issue with the specfile?
> > 
> 
> It would be. However, annocheck/annobin gets invoked by default `make`.
> 
> ```
> ifeq ($(RPMCFLAGS),)
> CFLAGS  :=      -DVERSION=\"$(VERSION)\" $(FOPTS) $(MOPTS) $(WOPTS) $(SOPTS) $(DEFS)
> else
> CFLAGS  :=       $(RPMCFLAGS) $(DEFS)
> endif
> ```
> 
> On `main`, with just disabling USE_BPF:
> 
> ```
> [root@rt-qe-06 stalld]# make USE_BPF=0
> ARCH=x86_64
> USE_BPF=0
> FCF_PROTECTION=-fcf-protection
> MTUNE=-mtune=generic
> CFLAGS=-DVERSION=\"1.23.1\" -flto=auto -ffat-lto-objects -fexceptions -fstack-protector-strong -fasynchronous-unwind-tables -fstack-clash-protection -fno-omit-frame-pointer -fcf-protection -fpie -mtune=generic -m64 -mno-omit-leaf-frame-pointer -Wall -Werror=format-security -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -DUSE_BPF=0 -D_FORTIFY_SOURCE=3 -D_GLIBCXX_ASSERTIONS -DDEBUG_STALLD=0 -g -O2
> LDFLAGS=-ggdb -znow -pie
> gcc -DVERSION=\"1.23.1\" -flto=auto -ffat-lto-objects -fexceptions -fstack-protector-strong -fasynchronous-unwind-tables -fstack-clash-protection -fno-omit-frame-pointer -fcf-protection -fpie -mtune=generic -m64 -mno-omit-leaf-frame-pointer -Wall -Werror=format-security -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -DUSE_BPF=0 -D_FORTIFY_SOURCE=3 -D_GLIBCXX_ASSERTIONS -DDEBUG_STALLD=0 -g -O2   -c -o src/stalld.o src/stalld.c
> gcc: error: /usr/lib/rpm/redhat/redhat-annobin-cc1: No such file or directory
> make: *** [src/stalld.o] Error 1

Yeah, that's the "why is a basic 'make' pulling in Red Hat stuff?"
question.

> [root@rt-qe-06 stalld]# make USE_BPF=0 SOPTS= FCF_PROTECTION=
> ARCH=x86_64
> USE_BPF=0
> FCF_PROTECTION=
> MTUNE=-mtune=generic
> CFLAGS=-DVERSION=\"1.23.1\" -flto=auto -ffat-lto-objects -fexceptions -fstack-protector-strong -fasynchronous-unwind-tables -fstack-clash-protection -fno-omit-frame-pointer  -fpie -mtune=generic -m64 -mno-omit-leaf-frame-pointer -Wall -Werror=format-security  -DUSE_BPF=0 -D_FORTIFY_SOURCE=3 -D_GLIBCXX_ASSERTIONS -DDEBUG_STALLD=0 -g -O2
> LDFLAGS=-ggdb -znow -pie
> gcc -DVERSION=\"1.23.1\" -flto=auto -ffat-lto-objects -fexceptions -fstack-protector-strong -fasynchronous-unwind-tables -fstack-clash-protection -fno-omit-frame-pointer  -fpie -mtune=generic -m64 -mno-omit-leaf-frame-pointer -Wall -Werror=format-security  -DUSE_BPF=0 -D_FORTIFY_SOURCE=3 -D_GLIBCXX_ASSERTIONS -DDEBUG_STALLD=0 -g -O2   -c -o src/stalld.o src/stalld.c
> In file included from src/stalld.c:39:0:
> src/queue_track.h: In function ‘find_queued_task’:
> src/queue_track.h:61:2: error: ‘for’ loop initial declarations are only allowed in C99 mode
>   for (unsigned int i = 0;  \
>   ^
> src/queue_track.h:118:2: note: in expansion of macro ‘for_each_task_entry’
>   for_each_task_entry(cpu_data, task) {
>   ^
> src/queue_track.h:61:2: note: use option -std=c99 or -std=gnu99 to compile your code
>   for (unsigned int i = 0;  \
>   ^
> src/queue_track.h:118:2: note: in expansion of macro ‘for_each_task_entry’
>   for_each_task_entry(cpu_data, task) {
>   ^
> make: *** [src/stalld.o] Error 1
> ```

So I guess the makefile's definition of HDR is wrong, and it turns out
HDR isn't even used anywhere :-P

> 
> Appending c99 to the CFLAGS on the command-line a-la `CFLAGS+=-std=c99`
> overrides the cflags entirely... so editing the CFLAGS to append -std=c99..

You're saying that if you do CFLAGS+=-std=c99 the only thing in CFLAGS
is -std=c99?  That shouldn't happen...

-Crystal

Re: [PATCH 2/3] Makefile: Add support for legacy kernels

[Derek Barbosa]

On Wed, Oct 08, 2025 at 04:38:49PM -0500, Crystal Wood wrote:
> On Wed, 2025-10-08 at 14:41 -0400, Derek Barbosa wrote:
> > On Wed, Oct 08, 2025 at 11:50:11AM -0500, Crystal Wood wrote:
> > > The default, if no C language dialect options are given, is -std=gnu90; this
> > > will change to -std=gnu99 or -std=gnu11 in some future release when the C99 or
> > > C11 support is complete.
> 
> I meant, why can't we specify c99 regardless of legacy (which would also
> help avoid introducing new stuff that breaks on legacy)?  Unless
> something in queue_track.c already does need newer than c99.
> 

Hm. I need to check if c99 breaks something in queue_track.

> > > > When building on RHEL-7, it was though SOPTS still make their way through, even
> > > > though the comment specifies that these are variables that only come from the
> > > > specfile for DNF-family distributions.I was getting errors due to annobin not
> > > > being present, so I stripped it out.
> > > 
> > > Oh, I was looking for "annocheck" which explains why I didn't notice
> > > that.
> > > 
> > > So the issue is that when you *are* building an RPM, the specfile isn't
> > > passing the right things for that OS version?  Wouldn't that be an
> > > issue with the specfile?
> > > 
> > 
> > It would be. However, annocheck/annobin gets invoked by default `make`.
> > 
> > ```
> > ifeq ($(RPMCFLAGS),)
> > CFLAGS  :=      -DVERSION=\"$(VERSION)\" $(FOPTS) $(MOPTS) $(WOPTS) $(SOPTS) $(DEFS)
> > else
> > CFLAGS  :=       $(RPMCFLAGS) $(DEFS)
> > endif
> > ```
> > 
> > On `main`, with just disabling USE_BPF:
> > 
> > ```
> > [root@rt-qe-06 stalld]# make USE_BPF=0
> > ARCH=x86_64
> > USE_BPF=0
> > FCF_PROTECTION=-fcf-protection
> > MTUNE=-mtune=generic
> > CFLAGS=-DVERSION=\"1.23.1\" -flto=auto -ffat-lto-objects -fexceptions -fstack-protector-strong -fasynchronous-unwind-tables -fstack-clash-protection -fno-omit-frame-pointer -fcf-protection -fpie -mtune=generic -m64 -mno-omit-leaf-frame-pointer -Wall -Werror=format-security -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -DUSE_BPF=0 -D_FORTIFY_SOURCE=3 -D_GLIBCXX_ASSERTIONS -DDEBUG_STALLD=0 -g -O2
> > LDFLAGS=-ggdb -znow -pie
> > gcc -DVERSION=\"1.23.1\" -flto=auto -ffat-lto-objects -fexceptions -fstack-protector-strong -fasynchronous-unwind-tables -fstack-clash-protection -fno-omit-frame-pointer -fcf-protection -fpie -mtune=generic -m64 -mno-omit-leaf-frame-pointer -Wall -Werror=format-security -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -DUSE_BPF=0 -D_FORTIFY_SOURCE=3 -D_GLIBCXX_ASSERTIONS -DDEBUG_STALLD=0 -g -O2   -c -o src/stalld.o src/stalld.c
> > gcc: error: /usr/lib/rpm/redhat/redhat-annobin-cc1: No such file or directory
> > make: *** [src/stalld.o] Error 1
> 
> Yeah, that's the "why is a basic 'make' pulling in Red Hat stuff?"
> question.
> 

That's a good question. I'll try to ask around and see if we can just strip this
stuff.

> > [root@rt-qe-06 stalld]# make USE_BPF=0 SOPTS= FCF_PROTECTION=
> > ARCH=x86_64
> > USE_BPF=0
> > FCF_PROTECTION=
> > MTUNE=-mtune=generic
> > CFLAGS=-DVERSION=\"1.23.1\" -flto=auto -ffat-lto-objects -fexceptions -fstack-protector-strong -fasynchronous-unwind-tables -fstack-clash-protection -fno-omit-frame-pointer  -fpie -mtune=generic -m64 -mno-omit-leaf-frame-pointer -Wall -Werror=format-security  -DUSE_BPF=0 -D_FORTIFY_SOURCE=3 -D_GLIBCXX_ASSERTIONS -DDEBUG_STALLD=0 -g -O2
> > LDFLAGS=-ggdb -znow -pie
> > gcc -DVERSION=\"1.23.1\" -flto=auto -ffat-lto-objects -fexceptions -fstack-protector-strong -fasynchronous-unwind-tables -fstack-clash-protection -fno-omit-frame-pointer  -fpie -mtune=generic -m64 -mno-omit-leaf-frame-pointer -Wall -Werror=format-security  -DUSE_BPF=0 -D_FORTIFY_SOURCE=3 -D_GLIBCXX_ASSERTIONS -DDEBUG_STALLD=0 -g -O2   -c -o src/stalld.o src/stalld.c
> > In file included from src/stalld.c:39:0:
> > src/queue_track.h: In function ‘find_queued_task’:
> > src/queue_track.h:61:2: error: ‘for’ loop initial declarations are only allowed in C99 mode
> >   for (unsigned int i = 0;  \
> >   ^
> > src/queue_track.h:118:2: note: in expansion of macro ‘for_each_task_entry’
> >   for_each_task_entry(cpu_data, task) {
> >   ^
> > src/queue_track.h:61:2: note: use option -std=c99 or -std=gnu99 to compile your code
> >   for (unsigned int i = 0;  \
> >   ^
> > src/queue_track.h:118:2: note: in expansion of macro ‘for_each_task_entry’
> >   for_each_task_entry(cpu_data, task) {
> >   ^
> > make: *** [src/stalld.o] Error 1
> > ```
> 
> So I guess the makefile's definition of HDR is wrong, and it turns out
> HDR isn't even used anywhere :-P
> 
> > 
> > Appending c99 to the CFLAGS on the command-line a-la `CFLAGS+=-std=c99`
> > overrides the cflags entirely... so editing the CFLAGS to append -std=c99..
> 
> You're saying that if you do CFLAGS+=-std=c99 the only thing in CFLAGS
> is -std=c99?  That shouldn't happen...

Yeah...

```
[root@rt-qe-06 stalld]# make USE_BPF=0 SOPTS= FCF_PROTECTION= CFLAGS+=-std=c99
ARCH=x86_64
USE_BPF=0
FCF_PROTECTION=
MTUNE=-mtune=generic
CFLAGS=-std=c99
LDFLAGS=-ggdb -znow -pie
gcc -std=c99   -c -o src/stalld.o src/stalld.c
[root@rt-qe-06 stalld]# make USE_BPF=0 SOPTS= FCF_PROTECTION= 
ARCH=x86_64
USE_BPF=0
FCF_PROTECTION=
MTUNE=-mtune=generic
CFLAGS=-DVERSION=\"1.23.1\" -flto=auto -ffat-lto-objects -fexceptions -fstack-protector-strong -fasynchronous-unwind-tables -fstack-clash-protection -fno-omit-frame-pointer  -fpie -mtune=generic -m64 -mno-omit-leaf-frame-pointer -Wall -Werror=format-security  -DUSE_BPF=0 -D_FORTIFY_SOURCE=3 -D_GLIBCXX_ASSERTIONS -DDEBUG_STALLD=0 -g -O2
LDFLAGS=-ggdb -znow -pie
gcc -DVERSION=\"1.23.1\" -flto=auto -ffat-lto-objects -fexceptions -fstack-protector-strong -fasynchronous-unwind-tables -fstack-clash-protection -fno-omit-frame-pointer  -fpie -mtune=generic -m64 -mno-omit-leaf-frame-pointer -Wall -Werror=format-security  -DUSE_BPF=0 -D_FORTIFY_SOURCE=3 -D_GLIBCXX_ASSERTIONS -DDEBUG_STALLD=0 -g -O2   -c -o src/stalld.o src/stalld.c
```
> 
> -Crystal
> 

-- 
Derek <debarbos@redhat.com>

Re: [PATCH 2/3] Makefile: Add support for legacy kernels

[Crystal Wood]

On Wed, 2025-10-08 at 19:52 -0400, Derek Barbosa wrote:
> On Wed, Oct 08, 2025 at 04:38:49PM -0500, Crystal Wood wrote:
> > On Wed, 2025-10-08 at 14:41 -0400, Derek Barbosa wrote:
> > 
> > > 
> > > Appending c99 to the CFLAGS on the command-line a-la `CFLAGS+=-std=c99`
> > > overrides the cflags entirely... so editing the CFLAGS to append -std=c99..
> > 
> > You're saying that if you do CFLAGS+=-std=c99 the only thing in CFLAGS
> > is -std=c99?  That shouldn't happen...
> 
> Yeah...
> 
> ```
> [root@rt-qe-06 stalld]# make USE_BPF=0 SOPTS= FCF_PROTECTION= CFLAGS+=-std=c99

Oh, I missed "on the command-line".  If you want to make that work, the
makefile itself would have to use 
"override CFLAGS += <the normal stuff>" instead of "CFLAGS :=".

There's nothing to add to yet when processing the command line
variables.

-Crystal

Re: [PATCH 2/3] Makefile: Add support for legacy kernels

[Clark Williams]

On Wed, Oct 08, 2025 at 02:41:11PM -0400, Derek Barbosa wrote:
> 
> Of course, we could always go the direction of just dropping the functionality
> of parsing said "old" sched_debug files. Clark said that this may make some
> people upset though :P
> 

I agree that's the "easiest" path, but I think we may want to poll the
community to see if there's someone running an application on a 3.x or
earlier kernel and has a need for stalld. 

We can bump this around a bit and if no one raises their hand, we'll
drop the old format.

Clark

-- 
The United States Coast Guard
Ruining Natural Selection since 1790

Re: [PATCH 2/3] Makefile: Add support for legacy kernels

[Crystal Wood]

On Fri, 2025-10-17 at 08:16 -0500, Clark Williams wrote:
> On Wed, Oct 08, 2025 at 02:41:11PM -0400, Derek Barbosa wrote:
> > 
> > Of course, we could always go the direction of just dropping the functionality
> > of parsing said "old" sched_debug files. Clark said that this may make some
> > people upset though :P
> > 
> 
> I agree that's the "easiest" path, but I think we may want to poll the
> community to see if there's someone running an application on a 3.x or
> earlier kernel and has a need for stalld. 
> 
> We can bump this around a bit and if no one raises their hand, we'll
> drop the old format.

s/3.x/pre-4.14/

But is the task format really the issue here?  This patch is addressing
issues with BPF and compiler flags.

-Crystal

Re: [PATCH 2/3] Makefile: Add support for legacy kernels

[Derek Barbosa]

On Mon, Oct 20, 2025 at 11:54:28AM -0500, Crystal Wood wrote:
> On Fri, 2025-10-17 at 08:16 -0500, Clark Williams wrote:
> > On Wed, Oct 08, 2025 at 02:41:11PM -0400, Derek Barbosa wrote:
> > > 
> > > Of course, we could always go the direction of just dropping the functionality
> > > of parsing said "old" sched_debug files. Clark said that this may make some
> > > people upset though :P
> > > 
> > 
> > I agree that's the "easiest" path, but I think we may want to poll the
> > community to see if there's someone running an application on a 3.x or
> > earlier kernel and has a need for stalld. 
> > 
> > We can bump this around a bit and if no one raises their hand, we'll
> > drop the old format.
> 
> s/3.x/pre-4.14/
> 
> But is the task format really the issue here?  This patch is addressing
> issues with BPF and compiler flags.
> 

It would help fix a good amount of flag checks and cruft we carry in
sched_debug :)

> -Crystal
> 

-- 
Derek <debarbos@redhat.com>

Re: [PATCH 2/3] Makefile: Add support for legacy kernels

[Clark Williams]

Apologies for the duplicate replies but I forgot that I was replying to
linux-rt-users and good-old-gmail helpfully added some unwanted HTML to
my reply, causing it to bounce. Sigh...

On Tue, Oct 07, 2025 at 01:50:57PM -0500, Crystal Wood wrote:
> 
> Speaking of SOPTS, is this tool intended to be Red Hat specific
> (and require redhat-rpm-config even if no RPM building is intended)?
> 

No, I really don't like that we have it there in the main Makefile. Unfortunately we can't push it to the packaging phase since it affects things generated at compilation time. 

Perhaps we need to have a Makefile variable that identifies a "distro" and triggers inclusion of a Makefile.<distro> that modifies the appropriate flags? I know it's ugly but I'd like to give other folks (suse, cannonical, etc) a way to tweak the build. We could default the variable to "redhat" and have a block that includes Makefile.redhat to modify the CFLAGS appropriately. 

Thoughts?

Clark
-- 
The United States Coast Guard
Ruining Natural Selection since 1790

Re: [PATCH 2/3] Makefile: Add support for legacy kernels

[Crystal Wood]

On Fri, 2025-10-17 at 13:01 +0000, Clark Williams wrote:
> On Tue, Oct 7, 2025 at 6:51 PM Crystal Wood <crwood@redhat.com> wrote:
> > 
> > Speaking of SOPTS, is this tool intended to be Red Hat specific
> > (and require redhat-rpm-config even if no RPM building is intended)?
> > 
> > 
> 
> No, I really don't like that we have it there in the main Makefile. 
> Unfortunately we can't push it to the packaging phase since it
> affects things generated at compilation time. 
> 
> Perhaps we need to have a Makefile variable that identifies a
> "distro" and triggers inclusion of a Makefile.<distro> that modifies
> the appropriate flags?  I know it's ugly but I'd like to give other
> folks (suse, cannonical, etc) a way to tweak the build.  We could
> default the variable to "redhat" and have a block that includes
> Makefile.redhat to modify the CFLAGS appropriately. 

Why can't the specfile build phase pass in the cflags it wants to add
(after fixing the makefile so that it appends to cflags rather than
replacing it)?

Ideally upstream shouldn't have to know about specific distros at all...

-Crystal

Re: [PATCH 2/3] Makefile: Add support for legacy kernels

[Clark Williams]

On Mon, Oct 20, 2025 at 11:48:16AM -0500, Crystal Wood wrote:
> On Fri, 2025-10-17 at 13:01 +0000, Clark Williams wrote:
> > On Tue, Oct 7, 2025 at 6:51 PM Crystal Wood <crwood@redhat.com> wrote:
> > > 
> > > Speaking of SOPTS, is this tool intended to be Red Hat specific
> > > (and require redhat-rpm-config even if no RPM building is intended)?
> > > 
> > > 
> > 
> > No, I really don't like that we have it there in the main Makefile. 
> > Unfortunately we can't push it to the packaging phase since it
> > affects things generated at compilation time. 
> > 
> > Perhaps we need to have a Makefile variable that identifies a
> > "distro" and triggers inclusion of a Makefile.<distro> that modifies
> > the appropriate flags?  I know it's ugly but I'd like to give other
> > folks (suse, cannonical, etc) a way to tweak the build.  We could
> > default the variable to "redhat" and have a block that includes
> > Makefile.redhat to modify the CFLAGS appropriately. 
> 
> Why can't the specfile build phase pass in the cflags it wants to add
> (after fixing the makefile so that it appends to cflags rather than
> replacing it)?
> 
> Ideally upstream shouldn't have to know about specific distros at all...
> 
> -Crystal
> 

That's a good point. We can have the specfile pass in additions to the 
CFLAGS (or completely override them, I suppose). 

Since Wander and I usually do the package builds for Fedora and CentOS/RHEL,
I'll talk to him about doing just that. 

Clark

-- 
The United States Coast Guard
Ruining Natural Selection since 1790

[PATCH 3/3] scripts: fix run-local if bashism

[Derek Barbosa]

In older versions of Bash, mainly 4.2.X, a semicolon (or newline) is
requied after a conditional statement and before the `then` keyword.

Add the semicolon to allow testing and quick "local runs" on legacy
kernels.

Signed-off-by: Derek Barbosa <debarbos@redhat.com>
---
 scripts/run-local.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/run-local.sh b/scripts/run-local.sh
index 0ce1476..e65f34d 100755
--- a/scripts/run-local.sh
+++ b/scripts/run-local.sh
@@ -44,7 +44,7 @@ parse_args()
 # script start
 #
 
-if [[ ! -x ./stalld ]] then
+if [[ ! -x ./stalld ]]; then
    echo "No stalld executable in the current directory!"
    exit 1
 fi
-- 
2.50.0

Re: [PATCH 3/3] scripts: fix run-local if bashism

[Clark Williams]

On Thu, Oct 02, 2025 at 04:55:15PM -0400, Derek Barbosa wrote:
> In older versions of Bash, mainly 4.2.X, a semicolon (or newline) is
> requied after a conditional statement and before the `then` keyword.
> 
> Add the semicolon to allow testing and quick "local runs" on legacy
> kernels.
> 
> Signed-off-by: Derek Barbosa <debarbos@redhat.com>
Signed-off-by: Clark Williams <williams@redhat.com>

Thank you
-- 
The United States Coast Guard
Ruining Natural Selection since 1790