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

[Philippe Gerum <rpm@xenomai.org>]

Jan Kiszka <jan.kiszka@siemens.com> writes:

> On 15.06.26 09:14, Philippe Gerum wrote:
>> Jan Kiszka <jan.kiszka@siemens.com> writes:
>> 
>>> On 15.06.26 08:42, Philippe Gerum wrote:
>>>> Philippe Gerum <rpm@xenomai.org> writes:
>>>>
>>>>> Jan Kiszka <jan.kiszka@siemens.com> writes:
>>>>>
>>>>>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>>>>>
>>>>>> 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 <jan.kiszka@siemens.com>
>>>>>> ---
>>>>>>  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 <rpm@xenomai.org>
>>>> 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 <rpm@xenomai.org>
>>>>
>>>> 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.
>
> But that will call out even on old == new - does not sound right.
>

Call to determine that next ends up being prev, then filtered out by
quota_pick() as a consequence. This is strictly identical
performance-wise.

-- 
Philippe.