mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "Chen, Yu C" <yu.c.chen@intel.com>
To: Hui Su <sh_def@163.com>, Tim Chen <tim.c.chen@linux.intel.com>
Cc: K Prateek Nayak <kprateek.nayak@amd.com>,
	Juri Lelli <juri.lelli@redhat.com>,
	Vincent Guittot <vincent.guittot@linaro.org>,
	Dietmar Eggemann <dietmar.eggemann@arm.com>,
	Valentin Schneider <vschneid@redhat.com>,
	John Stultz <jstultz@google.com>, Ingo Molnar <mingo@redhat.com>,
	Peter Zijlstra <peterz@infradead.org>,
	<linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2 1/2] sched/numa: Drive NUMA task tick from execution context
Date: Tue, 8 Sep 2026 13:38:58 +0800	[thread overview]
Message-ID: <57833d3b-bf10-42b9-ae28-82d65a22a6af@intel.com> (raw)
In-Reply-To: <20260905135843.2818510-1-sh_def@163.com>

On 9/5/2026 9:58 PM, Hui Su wrote:
> On Fri, Sep 4, 2026 at 1:15 PM, Tim Chen wrote:
>> Yes, you have a good point. The code I proposed just look at whether
>> we have consumed our allotment in the current slice.
>>
>> What we should have looked at is whether the donor's total run time has
>> exceeded its quota when doing core scheduling. And we may happen to
>> hit __entity_slice_used() at the front of the slice after advancing
>> the deadline and __entity_slice_used()
>> returns false instead of true, even though I have consumed more than
>> my fair share when looking at longer time period across multiple slices.
>>
>> The accumulated run time of the donor since it was picked for running
>> should be used for selection time baseline.
>>
>> So maybe a patch like the following instead.
> 

[ ... ]

> 
> In this example reweight_eevdf() did not change se->vruntime, so the
> divergence does not depend on a vruntime coordinate adjustment. The
> weight change alone is enough for the accumulated vused and the
> current-weight vslice to no longer necessarily use the same scale.
> 

Makes sense. In the current kernel with a flat cgroup(commit 85570f10a4c6
("sched/eevdf: Move to a single runqueue")), task_tick_fair()
tries to re-calculate the task's h_load.weight - if the cgroup changes
its share at runtime, the task se's h_load.weight changes accordingly.
Thus comparing the delta derived from the snapshot vruntime and the
slice using the latest weight is unreliable. (While before the flat cgroup
was introduced, a task's se->load.weight will not be re-evaluated during 
the tick, even if the cgroup changes its share at runtime.
> diff --git a/include/linux/sched.h b/include/linux/sched.h
> index 8b3d47a325cc..c32d9931129f 100644
> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -590,6 +590,9 @@ struct sched_entity {
>   	u64				sum_exec_runtime;
>   	u64				prev_sum_exec_runtime;
>   	u64				vruntime;
> +#ifdef CONFIG_SCHED_CORE
> +	u64				core_sched_start;
> +#endif
>   	/* Approximated virtual lag: */
>   	s64				vlag;
>   	/* 'Protected' deadline, to give out minimum quantums: */
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index f78275192036..22ae5dc57337 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -4580,6 +4580,9 @@ static void __sched_fork(u64 clone_flags, struct task_struct *p)
>   	p->se.prev_sum_exec_runtime	= 0;
>   	p->se.nr_migrations		= 0;
>   	p->se.vruntime			= 0;
> +#ifdef CONFIG_SCHED_CORE
> +	p->se.core_sched_start		= 0;
> +#endif
>   	p->se.vlag			= 0;
>   	p->se.rel_deadline		= 0;
>   	INIT_LIST_HEAD(&p->se.group_node);
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 8dff37059faf..1bd05c906d3a 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -6502,6 +6502,9 @@ set_next_entity(struct cfs_rq *cfs_rq, struct sched_entity *se)
>   	}
>   
>   	se->prev_sum_exec_runtime = se->sum_exec_runtime;
> +#ifdef CONFIG_SCHED_CORE
> +	se->core_sched_start = se->exec_start;

if (entity_is_task(se)) ?

> +#endif
>   }
>   
>   static bool __dequeue_task(struct rq *rq, struct task_struct *p, int flags);
> @@ -14748,10 +14751,9 @@ static void rq_offline_fair(struct rq *rq)
>   static inline bool
>   __entity_slice_used(struct sched_entity *se, int min_nr_tasks)
>   {
> -	u64 rtime = se->sum_exec_runtime - se->prev_sum_exec_runtime;
> -	u64 slice = se->slice;
> +	u64 rtime = se->exec_start - se->core_sched_start;
> 
> -	return (rtime * min_nr_tasks > slice);
> +	return (rtime * min_nr_tasks > se->slice);
>   }
>   
>   #define MIN_NR_TASKS_DURING_FORCEIDLE	2
> 
> The task-clock version has matched the existing predicate in the
> non-proxy tests so far and fixes the proxy reproducer as well. I am
> still validating reselection, migration, and the remaining proxy
> boundary cases, so I have not posted either implementation.
> 
> Do you think keeping the check in the original real-time/task-clock
> domain is a reasonable direction here, or would you prefer preserving
> the vruntime approach by carrying the accumulated service across
> reweights?
> 

I would vote for the real-time comparison. What do you think, Tim?

thanks,
Chenyu

  reply	other threads:[~2026-09-08  5:39 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-03  4:11 [PATCH v2 0/2] sched: Fix execution-context tick handling under proxy execution Hui Su
2026-09-03  4:11 ` [PATCH v2 1/2] sched/numa: Drive NUMA task tick from execution context Hui Su
2026-09-03 12:41   ` Chen, Yu C
2026-09-03 21:30     ` Tim Chen
2026-09-04  5:16       ` Hui Su
2026-09-04 14:10       ` Hui Su
2026-09-04 20:15         ` Tim Chen
2026-09-05 13:58           ` Hui Su
2026-09-08  5:38             ` Chen, Yu C [this message]
2026-09-08 16:01               ` Tim Chen
2026-09-09 10:55                 ` Hui Su
2026-09-04 16:10       ` Chen Yu
2026-09-04 20:24         ` Tim Chen
2026-09-03  4:11 ` [PATCH v2 2/2] sched/cache: Drive cache " Hui Su
2026-09-03  4:37   ` K Prateek Nayak
2026-09-03  4:51     ` Hui Su
2026-09-03 17:23   ` Tim Chen
2026-09-04  4:03     ` Hui Su

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=57833d3b-bf10-42b9-ae28-82d65a22a6af@intel.com \
    --to=yu.c.chen@intel.com \
    --cc=dietmar.eggemann@arm.com \
    --cc=jstultz@google.com \
    --cc=juri.lelli@redhat.com \
    --cc=kprateek.nayak@amd.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=sh_def@163.com \
    --cc=tim.c.chen@linux.intel.com \
    --cc=vincent.guittot@linaro.org \
    --cc=vschneid@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®