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

[sashiko-bot@kernel.org]

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 <gmonaco@redhat.com>

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/20260929124908.177676-1-gmonaco@redhat.com?part=8