Re: [PATCH v7 1/1] sched: update the rq->avg_idle when a task is moved to an idle CPU

[Valentin Schneider]

On 26/12/25 14:32, Huang Shijie wrote:
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -3609,6 +3609,21 @@ static inline void ttwu_do_wakeup(struct task_struct *p)
>       trace_sched_wakeup(p);
>  }
>
> +void update_rq_avg_idle(struct rq *rq)
> +{
> +	if (rq->idle_stamp) {
> +		u64 delta = rq_clock(rq) - rq->idle_stamp;
> +		u64 max = 2*rq->max_idle_balance_cost;
> +
> +		update_avg(&rq->avg_idle, delta);
> +
> +		if (rq->avg_idle > max)
> +			rq->avg_idle = max;
> +
> +		rq->idle_stamp = 0;
> +	}
> +}
> +

So if we have this invoked every time we switch to the idle task via
put_prev_task_idle(), do we want to move sched_balance_newidle()'s update
of rq->idle_stamp() to set_next_task_idle()?

That does change the behaviour as we'd now record any idle duration as
opposed to only idle-from-fair duration, but that would mean we'd
unconditionally record a rq->idle_stamp and could thus ditch the if{} clause.

>  static void
>  ttwu_do_activate(struct rq *rq, struct task_struct *p, int wake_flags,
>                struct rq_flags *rf)
> @@ -3644,18 +3659,6 @@ ttwu_do_activate(struct rq *rq, struct task_struct *p, int wake_flags,
>               p->sched_class->task_woken(rq, p);
>               rq_repin_lock(rq, rf);
>       }
> -
> -	if (rq->idle_stamp) {
> -		u64 delta = rq_clock(rq) - rq->idle_stamp;
> -		u64 max = 2*rq->max_idle_balance_cost;
> -
> -		update_avg(&rq->avg_idle, delta);
> -
> -		if (rq->avg_idle > max)
> -			rq->avg_idle = max;
> -
> -		rq->idle_stamp = 0;
> -	}
>  }
>
>  /*
> diff --git a/kernel/sched/idle.c b/kernel/sched/idle.c
> index 65eb8f8c1a5d..aba5ad53c07d 100644
> --- a/kernel/sched/idle.c
> +++ b/kernel/sched/idle.c
> @@ -460,6 +460,7 @@ static void put_prev_task_idle(struct rq *rq, struct task_struct *prev, struct t
>  {
>       update_curr_idle(rq);
>       scx_update_idle(rq, false, true);
> +	update_rq_avg_idle(rq);

AFAICT we can't have put_prev_task_idle() immediately followed by
set_next_task_idle(); put_prev_set_next_task() especially already handles
this, so I think we're good, but maybe worth mentioning in the changelog?

>  }
>
>  static void set_next_task_idle(struct rq *rq, struct task_struct *next, bool first)
> diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
> index 3ceaa9dc9a9e..6e3dd8c975e0 100644
> --- a/kernel/sched/sched.h
> +++ b/kernel/sched/sched.h
> @@ -1651,6 +1651,7 @@ static inline struct cfs_rq *group_cfs_rq(struct sched_entity *grp)
>
>  #endif /* !CONFIG_FAIR_GROUP_SCHED */
>
> +extern void update_rq_avg_idle(struct rq *rq);
>  extern void update_rq_clock(struct rq *rq);
>
>  /*
> --
> 2.43.0