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 <[email protected]> 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 > diff --git a/kernel/trace/rv/monitors/boost/boost.c > b/kernel/trace/rv/monitors/boost/boost.c > index 0000000000000..3533d23279eeb 100644 > --- /dev/null > +++ b/kernel/trace/rv/monitors/boost/boost.c [ ... ] > +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? [ ... ] > diff --git a/kernel/trace/rv/monitors/laxity/laxity.c > b/kernel/trace/rv/monitors/laxity/laxity.c > index 0000000000000..751d318224366 100644 > --- /dev/null > +++ b/kernel/trace/rv/monitors/laxity/laxity.c [ ... ] > +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? [ ... ] > diff --git a/kernel/trace/rv/monitors/laxity/laxity.h > b/kernel/trace/rv/monitors/laxity/laxity.h > index 0000000000000..69a818c0f6cfe 100644 > --- /dev/null > +++ b/kernel/trace/rv/monitors/laxity/laxity.h [ ... ] > + .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/[email protected]?part=8
