Re: [PATCH 08/10] rv: Add dl_server specific monitors

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] rv/boost: false positive deadline violation on priority change
- [Medium] rv/laxity: unhandled dl_server_stop from zero_laxity_wait panics monitor
- [Medium] rv/laxity: unhandled dl_replenish_running from idle_wait panics monitor
--

commit c4d2df85586e4b58ec40171970939b56d0e05ab9
Author: Gabriele Monaco [off-list ref]

rv: Add dl_server specific monitors
    
Add monitors to validate the behaviour of the deadline server.

The currently implemented monitors are:
* boost
    fair tasks run either independently or boosted
* laxity
    deferrable servers wait for zero-laxity and run

[ ... ]

+static void handle_sched_enqueue(void *data, struct task_struct *tsk, int cpu)
+{
+	struct sched_dl_entity *dl_se = NULL;
+	u8 type = get_server_type(tsk);
+
+	if (is_server_type(type))
+		dl_se = get_server(tsk, type);
+	if (dl_se) {
+		da_handle_event(EXPAND_ID(dl_se, cpu, type),
+				dl_se->runtime > 0 && !idle_cpu(cpu) ?
+					dl_server_resume_boost :
+					dl_server_resume_throttled_boost);
+	}
+}

[Severity: Medium]
Will this lead to a false positive deadline violation if a task changes
priority?  If a running task changes its priority (e.g., via set_user_nice()),
it is dequeued and re-enqueued, which triggers handle_sched_enqueue() and
transitions the monitor to ready_boost.

If the task continues running without being preempted, __schedule() will
bypass the sched_switch tracepoint:

kernel/sched/core.c:__schedule() {
    ...
    if (likely(prev != next)) {
        trace_sched_switch(preempt, prev, next, prev_state);
        ...
    }
    ...
}

Could this leave the monitor trapped in the ready_boost state until its
timer expires, causing an unwarranted violation?

[ ... ]

[ ... ]

+static void handle_dl_replenish(void *data, struct sched_dl_entity *dl_se,
+				int cpu, u8 type)
+{
+	if (!is_server_type(type))
+		return;
+	/* Special replenish happening after throttle, ignore it */
+	if (dl_se->dl_defer_running && dl_se->dl_throttled)
+		return;
+	if (dl_se->dl_defer_running)
+		da_handle_event(EXPAND_ID(dl_se, cpu, type), dl_replenish_running_laxity);

[Severity: Medium]
Is it possible for dl_replenish_running_laxity to be emitted while the monitor
is in the idle_wait state, causing an INVALID_STATE panic?

When the CPU is idle, the monitor enters idle_wait_laxity. If a fair task wakes
up, dl_server_start() clears dl_defer_idle but returns without emitting a
tracepoint because dl_server_active is already true.

If the dl_timer fires before update_curr_idle() advances exec_start
significantly, dl_server_timer() proceeds with dl_defer_idle == 0, sets
dl_defer_running = 1, and emits dl_replenish_running.

Since the generated automaton lacks a transition for dl_replenish_running from
idle_wait_laxity, won't this cause a state machine panic?

+	else if (idle_cpu(cpu))
+		da_handle_event(EXPAND_ID(dl_se, cpu, type), dl_replenish_idle_laxity);
+	else
+		da_handle_event(EXPAND_ID(dl_se, cpu, type), dl_replenish_laxity);
+}

[ ... ]

+static void handle_dl_server_stop(void *data, struct sched_dl_entity *dl_se,
+				  int cpu, u8 type)
+{
+	if (is_server_type(type))
+		da_handle_start_event(EXPAND_ID(dl_se, cpu, type), dl_server_stop_laxity);
+}

[Severity: Medium]
Can dl_server_stop_laxity be emitted while the monitor is in zero_laxity_wait,
resulting in an INVALID_STATE panic?

The monitor enters zero_laxity_wait when the server starts. If all fair tasks
on the runqueue are dequeued (e.g., they sleep) before the server runs,
dl_server_active is cleared and dl_server_stop is called.

The generated automaton for the laxity monitor lacks a transition for
dl_server_stop from zero_laxity_wait. Does this cause a crash?

[ ... ]

[ ... ]

+	.function = {

[ ... ]

+		{
+			zero_laxity_wait_laxity,
+			idle_wait_laxity,
+			INVALID_STATE,
+			INVALID_STATE,
+			INVALID_STATE,
+			stopped_laxity,
+			INVALID_STATE,
+			zero_laxity_wait_laxity,
+			INVALID_STATE,
+		},

[Severity: Medium]
The transition for dl_replenish_running_laxity (index 2) is missing
(INVALID_STATE) from idle_wait_laxity (index 1), which causes the panic
described above in handle_dl_replenish().

[ ... ]

+		{
+			zero_laxity_wait_laxity,
+			idle_wait_laxity,
+			running_laxity,
+			INVALID_STATE,
+			INVALID_STATE,
+			INVALID_STATE,
+			INVALID_STATE,
+			zero_laxity_wait_laxity,
+			INVALID_STATE,
+		},

[Severity: Medium]
The transition for dl_server_stop_laxity (index 5) is missing
(INVALID_STATE) from zero_laxity_wait_laxity (index 4), which causes
the panic described above in handle_dl_server_stop().

+	},
+	.initial_state = stopped_laxity,
+	.final_states = { 1, 0, 0, 0, 0 },
+};

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260929124908.177676-1-gmonaco@redhat.com?part=8