Re: [PATCH v1 1/2] evl/sched: core: introduce schedule out handler

On 17.06.26 16:20, Philippe Gerum wrote:
> From: Philippe Gerum <[email protected]>
> 
> Some scheduling classes may need a way to perform specific
> fixup/accounting work for a thread which is being scheduled out.  We
> cannot rely on the sched_pick() handler for this since the first one
> to successfully pick a thread prevents other handlers down the class
> hierarchy to run.
> 
> To address this issue, we introduce the sched_out() handler which is
> called for the current thread when scheduled out. This handler is
> invoked unconditionally when present prior to picking a new thread.
> 
> Signed-off-by: Philippe Gerum <[email protected]>
> ---
>  include/evl/sched.h     |  1 +
>  kernel/evl/sched/core.c | 20 +++++++++++++-------
>  2 files changed, 14 insertions(+), 7 deletions(-)
> 
> 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);
>  	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 d1d025e06a5d..7127d4e52df9 100644
> --- a/kernel/evl/sched/core.c
> +++ b/kernel/evl/sched/core.c
> @@ -773,7 +773,8 @@ static inline void set_next_running(struct evl_rq *rq,
>  		evl_stop_timer(&rq->rrbtimer);
>  }
>  
> -static struct evl_thread *__pick_next_thread(struct evl_rq *rq)
> +static __always_inline struct evl_thread *
> +__pick_next_thread(struct evl_rq *rq)
>  {
>  	struct evl_sched_class *sched_class;
>  	struct evl_thread *curr = rq->curr;
> @@ -821,8 +822,14 @@ static struct evl_thread *__pick_next_thread(struct evl_rq *rq)
>  /* rq->curr->lock + rq->lock held, hard irqs off. */
>  static struct evl_thread *pick_next_thread(struct evl_rq *rq)
>  {
> -	struct evl_thread *next = __pick_next_thread(rq);
> +	struct evl_thread *next, *prev = rq->curr;
> +	struct evl_sched_class *prev_class = prev->sched_class;
>  
> +	if (prev_class->sched_out)
> +		prev_class->sched_out(prev);
> +
> +	next = __pick_next_thread(rq);
> +	trace_evl_pick_thread(next);
>  	set_next_running(rq, next);
>  
>  	return next;
> @@ -972,21 +979,20 @@ void __evl_schedule(void) /* oob or/and hard irqs off (CPU migration-safe) */
>  		return;
>  	}
>  
> +	prev = curr;
>  	next = pick_next_thread(this_rq);
> -	trace_evl_pick_thread(next);
> -	if (next == curr) {
> -		if (unlikely(next->state & EVL_T_ROOT)) {
> +	if (next == prev) {
> +		if (unlikely(prev->state & EVL_T_ROOT)) {
>  			if (this_rq->local_flags & RQ_TPROXY)
>  				evl_notify_proxy_tick(this_rq);
>  			if (this_rq->local_flags & RQ_TDEFER)
>  				evl_program_local_tick(&evl_mono_clock);
>  		}
>  		raw_spin_unlock(&this_rq->lock);
> -		raw_spin_unlock_irqrestore(&curr->lock, flags);
> +		raw_spin_unlock_irqrestore(&prev->lock, flags);
>  		return;
>  	}
> - 
> -	prev = curr;

This hunk subtly look like a pure cosmetic change, without any logical
impact. If it does have one which I missed, it should be explained in
the commit message at least.

Jan

>  	this_rq->curr = next;
>  	leaving_inband = false;
>  


-- 
Siemens AG, Foundational Technologies
Linux Expert Center