Re: [PATCH 1/2] evl/sched: Add sched_out handler to sched_class

Jan Kiszka <[email protected]> writes:

> On 15.06.26 08:42, Philippe Gerum wrote:
>> Philippe Gerum <[email protected]> writes:
>> 
>>> Jan Kiszka <[email protected]> writes:
>>>
>>>> From: Jan Kiszka <[email protected]>
>>>>
>>>> This shall be invoked before a thread switch, providing both the current
>>>> and the next thread as arguments. Some scheduling classes may need it to
>>>> correctly handle their state as the sched_pick may not be invoked when a
>>>> higher-weighted class is providing the next thread.
>>>>
>>>> Signed-off-by: Jan Kiszka <[email protected]>
>>>> ---
>>>>  include/evl/sched.h     | 2 ++
>>>>  kernel/evl/sched/core.c | 5 +++++
>>>>  2 files changed, 7 insertions(+)
>>>>
>>>> diff --git a/include/evl/sched.h b/include/evl/sched.h
>>>> index ae9690860146c..0b16f1b1cf626 100644
>>>> --- a/include/evl/sched.h
>>>> +++ b/include/evl/sched.h
>>>> @@ -120,6 +120,8 @@ struct evl_sched_class {
>>>>  	void (*sched_dequeue)(struct evl_thread *thread);
>>>>  	void (*sched_requeue)(struct evl_thread *thread);
>>>>  	struct evl_thread *(*sched_pick)(struct evl_rq *rq);
>>>> +	void (*sched_out)(struct evl_thread *thread,
>>>> +			  struct evl_thread *next);
>>>>  	void (*sched_yield)(struct evl_thread *thread);
>>>>  	void (*sched_migrate)(struct evl_thread *thread,
>>>>  			      struct evl_rq *rq);
>>>> diff --git a/kernel/evl/sched/core.c b/kernel/evl/sched/core.c
>>>> index eb133e334d30f..0d49fc16bd67e 100644
>>>> --- a/kernel/evl/sched/core.c
>>>> +++ b/kernel/evl/sched/core.c
>>>> @@ -910,6 +910,7 @@ static __always_inline bool test_resched(struct evl_rq *this_rq)
>>>>   */
>>>>  void __evl_schedule(void) /* oob or/and hard irqs off (CPU migration-safe) */
>>>>  {
>>>> +	struct evl_sched_class *prev_schedclass;
>>>>  	struct evl_rq *this_rq = this_evl_rq();
>>>>  	struct evl_thread *prev, *next, *curr;
>>>>  	bool leaving_inband, inband_tail;
>>>> @@ -990,6 +991,10 @@ void __evl_schedule(void) /* oob or/and hard irqs off (CPU migration-safe) */
>>>>  	this_rq->curr = next;
>>>>  	leaving_inband = false;
>>>>  
>>>> +	prev_schedclass = prev->sched_class;
>>>> +	if (prev_schedclass->sched_out)
>>>> +		prev_schedclass->sched_out(prev, next);
>>>> +
>>>>  	/*
>>>>  	 * Careful: we _must_ have updated this_rq->curr before
>>>>  	 * performing the rest of the context switch code
>>>
>>> I've been working on this lately too. It turns out that we need more
>>> than this, although this is definitely part of the solution. I'll follow
>>> up on this issue.
>> 
>> This is still wip, I'm sharing this early to discuss details for
>> reconciling both proposals (the hunk in __evl_schedule() is merely
>> cosmetic, no functional change).
>> 
>> commit f49dbd63c1389a6b99508ef0da852545b102d1cc (HEAD -> wip/fix-quota-sched)
>> Author: Philippe Gerum <[email protected]>
>> Date:   Sun Jun 14 12:08:59 2026 +0200
>> 
>>     evl: sched/quota: fix budget tracking on preemption
>>     
>>     Upon preemption of a SCHED_QUOTA thread by a SCHED_FIFO one, the
>>     runtime budget of the former is inaccurately tracked. This is due to
>>     the fifo->pick() handler returning a valid thread, which prevents the
>>     quota->pick() handler from being called. As a result, the last runtime
>>     period of the outgoing thread is not accounted for.
>>     
>>     To fix this, we introduce a new sched_out() handler which is called
>>     for the outgoing thread, which the quota policy uses to update the
>>     remaining budget of preempted threads appropriately. In addition, the
>>     implementation no longer shares the runnable thread queue with
>>     SCHED_FIFO.
>>     
>>     Signed-off-by: Philippe Gerum <[email protected]>
>> 
>> diff --git a/include/evl/sched.h b/include/evl/sched.h
>> index ae9690860146..cc824c28004b 100644
>> --- a/include/evl/sched.h
>> +++ b/include/evl/sched.h
>> @@ -120,6 +120,7 @@ struct evl_sched_class {
>>  	void (*sched_dequeue)(struct evl_thread *thread);
>>  	void (*sched_requeue)(struct evl_thread *thread);
>>  	struct evl_thread *(*sched_pick)(struct evl_rq *rq);
>> +	void (*sched_out)(struct evl_thread *thread);
>
> The out handler also needs the target thread in order to identify if
> there is a tg change or not.
>

Your implementation needs this because the sequence is pick() -> out(),
mine is out() -> pick(), with the latter using the budget updated by the
former to figure out what to do next.

-- 
Philippe.