* [PATCH v2] sched/fair: Sync se's load_avg with cfs_rq in reweight_task @ 2024-07-20 5:12 Chuyi Zhou 2024-07-22 5:17 ` K Prateek Nayak 2024-07-22 6:34 ` Dietmar Eggemann 0 siblings, 2 replies; 8+ messages in thread From: Chuyi Zhou @ 2024-07-20 5:12 UTC (permalink / raw) To: mingo, peterz, juri.lelli, vincent.guittot, dietmar.eggemann, rostedt, bsegall, mgorman, vschneid Cc: chengming.zhou, linux-kernel, Chuyi Zhou In reweight_task(), there are two situations: 1. The task was on_rq, then the task's load_avg is accurate because we synchronized it with cfs_rq through update_load_avg() in dequeue_task(). 2. The task is sleeping, its load_avg might not have been updated for some time, which can result in inaccurate dequeue_load_avg() in reweight_entity(). This patch solves this by using update_load_avg() to synchronize the load_avg of se with cfs_rq. For tasks were on_rq, since we already update load_avg to accurate values in dequeue_task(), this change will not have other effects due to the short time interval between the two updates. Signed-off-by: Chuyi Zhou <zhouchuyi@bytedance.com> --- Changes in v2: - change the description in commit log. - use update_load_avg() in reweight_task() rather than in reweight_entity suggested by chengming. - Link to v1: https://lore.kernel.org/lkml/20240716150840.23061-1-zhouchuyi@bytedance.com/ --- kernel/sched/fair.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c index 9057584ec06d..b1e07ce90284 100644 --- a/kernel/sched/fair.c +++ b/kernel/sched/fair.c @@ -3835,12 +3835,15 @@ static void reweight_entity(struct cfs_rq *cfs_rq, struct sched_entity *se, } } +static inline void update_load_avg(struct cfs_rq *cfs_rq, struct sched_entity *se, int flags); + void reweight_task(struct task_struct *p, const struct load_weight *lw) { struct sched_entity *se = &p->se; struct cfs_rq *cfs_rq = cfs_rq_of(se); struct load_weight *load = &se->load; + update_load_avg(cfs_rq, se, 0); reweight_entity(cfs_rq, se, lw->weight); load->inv_weight = lw->inv_weight; } -- 2.20.1 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] sched/fair: Sync se's load_avg with cfs_rq in reweight_task 2024-07-20 5:12 [PATCH v2] sched/fair: Sync se's load_avg with cfs_rq in reweight_task Chuyi Zhou @ 2024-07-22 5:17 ` K Prateek Nayak 2024-07-22 6:40 ` Chengming Zhou 2024-07-28 20:14 ` Qais Yousef 2024-07-22 6:34 ` Dietmar Eggemann 1 sibling, 2 replies; 8+ messages in thread From: K Prateek Nayak @ 2024-07-22 5:17 UTC (permalink / raw) To: Chuyi Zhou Cc: mingo, peterz, juri.lelli, vincent.guittot, dietmar.eggemann, rostedt, bsegall, mgorman, vschneid, chengming.zhou, linux-kernel, Qais Yousef (+ Qais) Hello Chuyi, On 7/20/2024 10:42 AM, Chuyi Zhou wrote: > In reweight_task(), there are two situations: > > 1. The task was on_rq, then the task's load_avg is accurate because we > synchronized it with cfs_rq through update_load_avg() in dequeue_task(). > > 2. The task is sleeping, its load_avg might not have been updated for some > time, which can result in inaccurate dequeue_load_avg() in > reweight_entity(). > > This patch solves this by using update_load_avg() to synchronize the > load_avg of se with cfs_rq. For tasks were on_rq, since we already update > load_avg to accurate values in dequeue_task(), this change will not have > other effects due to the short time interval between the two updates. > > Signed-off-by: Chuyi Zhou <zhouchuyi@bytedance.com> > --- > Changes in v2: > - change the description in commit log. > - use update_load_avg() in reweight_task() rather than in reweight_entity > suggested by chengming. > - Link to v1: https://lore.kernel.org/lkml/20240716150840.23061-1-zhouchuyi@bytedance.com/ > --- > kernel/sched/fair.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c > index 9057584ec06d..b1e07ce90284 100644 > --- a/kernel/sched/fair.c > +++ b/kernel/sched/fair.c > @@ -3835,12 +3835,15 @@ static void reweight_entity(struct cfs_rq *cfs_rq, struct sched_entity *se, > } > } > > +static inline void update_load_avg(struct cfs_rq *cfs_rq, struct sched_entity *se, int flags); > + > void reweight_task(struct task_struct *p, const struct load_weight *lw) > { > struct sched_entity *se = &p->se; > struct cfs_rq *cfs_rq = cfs_rq_of(se); > struct load_weight *load = &se->load; > > + update_load_avg(cfs_rq, se, 0); Seems to be necessary when we reach here from __setscheduler_params() or set_user_nice() for a sleeping task. Please feel free to add: Reviewed-by: K Prateek Nayak <kprateek.nayak@amd.com> But since we are on the subject of accurate PELT accounting, one question I have here is whether a reweight_task() for a sleeping task race with its wakeup? Something like the following scenario: CPU0 CPU1 ==== ==== /* No rq locks held until ttwu_queue() */ try_to_wake_up(p) { ... /* Migrating task */ set_task_cpu(p, cpu) { /* p->sched_class->migrate_task_rq(p, new_cpu); */ /* Called with task_cpu(p)'s rq lock held */ migrate_task_rq_fair() { reweight_task(p) { /* p is still sleeping */ ... if (!task_on_rq_migrating(p)) { dequeue_load_avg(cfs_rq, se); remove_entity_load_avg(se); update_load_set(&se->load, weight); ... enqueue_load_avg(cfs_rq, se); } ... } } /* task_cpu() is updated here */ __set_task_cpu(p, new_cpu); } ttwu_queue(); } In theory, the remove_entity_load_avg() could record stale value of "load_avg" that gets removed at the next dequeue if I'm not mistaken? But I believe these small inaccuracies are tolerable since they'll decay in a while anyways? > reweight_entity(cfs_rq, se, lw->weight); > load->inv_weight = lw->inv_weight; > } -- Thanks and Regards, Prateek ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] sched/fair: Sync se's load_avg with cfs_rq in reweight_task 2024-07-22 5:17 ` K Prateek Nayak @ 2024-07-22 6:40 ` Chengming Zhou 2024-07-22 8:15 ` K Prateek Nayak 2024-07-28 20:14 ` Qais Yousef 1 sibling, 1 reply; 8+ messages in thread From: Chengming Zhou @ 2024-07-22 6:40 UTC (permalink / raw) To: K Prateek Nayak, Chuyi Zhou Cc: mingo, peterz, juri.lelli, vincent.guittot, dietmar.eggemann, rostedt, bsegall, mgorman, vschneid, linux-kernel, Qais Yousef On 2024/7/22 13:17, K Prateek Nayak wrote: > (+ Qais) > > Hello Chuyi, > > On 7/20/2024 10:42 AM, Chuyi Zhou wrote: >> In reweight_task(), there are two situations: >> >> 1. The task was on_rq, then the task's load_avg is accurate because we >> synchronized it with cfs_rq through update_load_avg() in dequeue_task(). >> >> 2. The task is sleeping, its load_avg might not have been updated for >> some >> time, which can result in inaccurate dequeue_load_avg() in >> reweight_entity(). >> >> This patch solves this by using update_load_avg() to synchronize the >> load_avg of se with cfs_rq. For tasks were on_rq, since we already update >> load_avg to accurate values in dequeue_task(), this change will not have >> other effects due to the short time interval between the two updates. >> >> Signed-off-by: Chuyi Zhou <zhouchuyi@bytedance.com> >> --- >> Changes in v2: >> - change the description in commit log. >> - use update_load_avg() in reweight_task() rather than in reweight_entity >> suggested by chengming. >> - Link to v1: >> https://lore.kernel.org/lkml/20240716150840.23061-1-zhouchuyi@bytedance.com/ >> --- >> kernel/sched/fair.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c >> index 9057584ec06d..b1e07ce90284 100644 >> --- a/kernel/sched/fair.c >> +++ b/kernel/sched/fair.c >> @@ -3835,12 +3835,15 @@ static void reweight_entity(struct cfs_rq >> *cfs_rq, struct sched_entity *se, >> } >> } >> +static inline void update_load_avg(struct cfs_rq *cfs_rq, struct >> sched_entity *se, int flags); >> + >> void reweight_task(struct task_struct *p, const struct load_weight *lw) >> { >> struct sched_entity *se = &p->se; >> struct cfs_rq *cfs_rq = cfs_rq_of(se); >> struct load_weight *load = &se->load; >> + update_load_avg(cfs_rq, se, 0); > > Seems to be necessary when we reach here from __setscheduler_params() or > set_user_nice() for a sleeping task. Please feel free to add: > > Reviewed-by: K Prateek Nayak <kprateek.nayak@amd.com> > > But since we are on the subject of accurate PELT accounting, one question > I have here is whether a reweight_task() for a sleeping task race with > its wakeup? Something like the following scenario: These two paths are impossible to race, since we have the task->pi_lock. Thanks. > > CPU0 CPU1 > ==== ==== > /* No rq locks held until ttwu_queue() */ > try_to_wake_up(p) { > ... > /* Migrating task */ > set_task_cpu(p, cpu) { > /* p->sched_class->migrate_task_rq(p, new_cpu); */ /* > Called with task_cpu(p)'s rq lock held */ > migrate_task_rq_fair() { reweight_task(p) { > /* p is still sleeping */ ... > if (!task_on_rq_migrating(p)) { > dequeue_load_avg(cfs_rq, se); > remove_entity_load_avg(se); > update_load_set(&se->load, weight); > ... enqueue_load_avg(cfs_rq, > se); > } ... > } } > /* task_cpu() is updated here */ > __set_task_cpu(p, new_cpu); > } > ttwu_queue(); > } > > In theory, the remove_entity_load_avg() could record stale value of > "load_avg" that gets removed at the next dequeue if I'm not mistaken? > But I believe these small inaccuracies are tolerable since they'll decay > in a while anyways? > >> reweight_entity(cfs_rq, se, lw->weight); >> load->inv_weight = lw->inv_weight; >> } > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] sched/fair: Sync se's load_avg with cfs_rq in reweight_task 2024-07-22 6:40 ` Chengming Zhou @ 2024-07-22 8:15 ` K Prateek Nayak 0 siblings, 0 replies; 8+ messages in thread From: K Prateek Nayak @ 2024-07-22 8:15 UTC (permalink / raw) To: Chengming Zhou, Chuyi Zhou Cc: mingo, peterz, juri.lelli, vincent.guittot, dietmar.eggemann, rostedt, bsegall, mgorman, vschneid, linux-kernel, Qais Yousef Hello Chengming, On 7/22/2024 12:10 PM, Chengming Zhou wrote: > On 2024/7/22 13:17, K Prateek Nayak wrote: >> (+ Qais) >> >> Hello Chuyi, >> >> On 7/20/2024 10:42 AM, Chuyi Zhou wrote: >>> In reweight_task(), there are two situations: >>> >>> 1. The task was on_rq, then the task's load_avg is accurate because we >>> synchronized it with cfs_rq through update_load_avg() in dequeue_task(). >>> >>> 2. The task is sleeping, its load_avg might not have been updated for some >>> time, which can result in inaccurate dequeue_load_avg() in >>> reweight_entity(). >>> >>> This patch solves this by using update_load_avg() to synchronize the >>> load_avg of se with cfs_rq. For tasks were on_rq, since we already update >>> load_avg to accurate values in dequeue_task(), this change will not have >>> other effects due to the short time interval between the two updates. >>> >>> Signed-off-by: Chuyi Zhou <zhouchuyi@bytedance.com> >>> --- >>> Changes in v2: >>> - change the description in commit log. >>> - use update_load_avg() in reweight_task() rather than in reweight_entity >>> suggested by chengming. >>> - Link to v1: https://lore.kernel.org/lkml/20240716150840.23061-1-zhouchuyi@bytedance.com/ >>> --- >>> kernel/sched/fair.c | 3 +++ >>> 1 file changed, 3 insertions(+) >>> >>> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c >>> index 9057584ec06d..b1e07ce90284 100644 >>> --- a/kernel/sched/fair.c >>> +++ b/kernel/sched/fair.c >>> @@ -3835,12 +3835,15 @@ static void reweight_entity(struct cfs_rq *cfs_rq, struct sched_entity *se, >>> } >>> } >>> +static inline void update_load_avg(struct cfs_rq *cfs_rq, struct sched_entity *se, int flags); >>> + >>> void reweight_task(struct task_struct *p, const struct load_weight *lw) >>> { >>> struct sched_entity *se = &p->se; >>> struct cfs_rq *cfs_rq = cfs_rq_of(se); >>> struct load_weight *load = &se->load; >>> + update_load_avg(cfs_rq, se, 0); >> >> Seems to be necessary when we reach here from __setscheduler_params() or >> set_user_nice() for a sleeping task. Please feel free to add: >> >> Reviewed-by: K Prateek Nayak <kprateek.nayak@amd.com> >> >> But since we are on the subject of accurate PELT accounting, one question >> I have here is whether a reweight_task() for a sleeping task race with >> its wakeup? Something like the following scenario: > > These two paths are impossible to race, since we have the task->pi_lock. Ah! Yes, you are right. Thank you for the clarification :) -- Thanks and Regards, Prateek > > Thanks. > >> >> [..snip..] >> ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] sched/fair: Sync se's load_avg with cfs_rq in reweight_task 2024-07-22 5:17 ` K Prateek Nayak 2024-07-22 6:40 ` Chengming Zhou @ 2024-07-28 20:14 ` Qais Yousef 1 sibling, 0 replies; 8+ messages in thread From: Qais Yousef @ 2024-07-28 20:14 UTC (permalink / raw) To: K Prateek Nayak Cc: Chuyi Zhou, mingo, peterz, juri.lelli, vincent.guittot, dietmar.eggemann, rostedt, bsegall, mgorman, vschneid, chengming.zhou, linux-kernel On 07/22/24 10:47, K Prateek Nayak wrote: > (+ Qais) > > Hello Chuyi, > > On 7/20/2024 10:42 AM, Chuyi Zhou wrote: > > In reweight_task(), there are two situations: > > > > 1. The task was on_rq, then the task's load_avg is accurate because we > > synchronized it with cfs_rq through update_load_avg() in dequeue_task(). > > > > 2. The task is sleeping, its load_avg might not have been updated for some > > time, which can result in inaccurate dequeue_load_avg() in > > reweight_entity(). > > > > This patch solves this by using update_load_avg() to synchronize the > > load_avg of se with cfs_rq. For tasks were on_rq, since we already update > > load_avg to accurate values in dequeue_task(), this change will not have > > other effects due to the short time interval between the two updates. > > > > Signed-off-by: Chuyi Zhou <zhouchuyi@bytedance.com> > > --- > > Changes in v2: > > - change the description in commit log. > > - use update_load_avg() in reweight_task() rather than in reweight_entity > > suggested by chengming. > > - Link to v1: https://lore.kernel.org/lkml/20240716150840.23061-1-zhouchuyi@bytedance.com/ > > --- > > kernel/sched/fair.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c > > index 9057584ec06d..b1e07ce90284 100644 > > --- a/kernel/sched/fair.c > > +++ b/kernel/sched/fair.c > > @@ -3835,12 +3835,15 @@ static void reweight_entity(struct cfs_rq *cfs_rq, struct sched_entity *se, > > } > > } > > +static inline void update_load_avg(struct cfs_rq *cfs_rq, struct sched_entity *se, int flags); > > + > > void reweight_task(struct task_struct *p, const struct load_weight *lw) > > { > > struct sched_entity *se = &p->se; > > struct cfs_rq *cfs_rq = cfs_rq_of(se); > > struct load_weight *load = &se->load; > > + update_load_avg(cfs_rq, se, 0); White space and a comment perhaps? LGTM anyway. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] sched/fair: Sync se's load_avg with cfs_rq in reweight_task 2024-07-20 5:12 [PATCH v2] sched/fair: Sync se's load_avg with cfs_rq in reweight_task Chuyi Zhou 2024-07-22 5:17 ` K Prateek Nayak @ 2024-07-22 6:34 ` Dietmar Eggemann 2024-07-22 9:39 ` Chuyi Zhou 1 sibling, 1 reply; 8+ messages in thread From: Dietmar Eggemann @ 2024-07-22 6:34 UTC (permalink / raw) To: Chuyi Zhou, mingo, peterz, juri.lelli, vincent.guittot, rostedt, bsegall, mgorman, vschneid Cc: chengming.zhou, linux-kernel On 20/07/2024 07:12, Chuyi Zhou wrote: > In reweight_task(), there are two situations: > > 1. The task was on_rq, then the task's load_avg is accurate because we > synchronized it with cfs_rq through update_load_avg() in dequeue_task(). > > 2. The task is sleeping, its load_avg might not have been updated for some > time, which can result in inaccurate dequeue_load_avg() in > reweight_entity(). > > This patch solves this by using update_load_avg() to synchronize the > load_avg of se with cfs_rq. For tasks were on_rq, since we already update > load_avg to accurate values in dequeue_task(), this change will not have > other effects due to the short time interval between the two updates. > > Signed-off-by: Chuyi Zhou <zhouchuyi@bytedance.com> > --- > Changes in v2: > - change the description in commit log. > - use update_load_avg() in reweight_task() rather than in reweight_entity > suggested by chengming. > - Link to v1: https://lore.kernel.org/lkml/20240716150840.23061-1-zhouchuyi@bytedance.com/ > --- > kernel/sched/fair.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c > index 9057584ec06d..b1e07ce90284 100644 > --- a/kernel/sched/fair.c > +++ b/kernel/sched/fair.c > @@ -3835,12 +3835,15 @@ static void reweight_entity(struct cfs_rq *cfs_rq, struct sched_entity *se, > } > } > > +static inline void update_load_avg(struct cfs_rq *cfs_rq, struct sched_entity *se, int flags); > + > void reweight_task(struct task_struct *p, const struct load_weight *lw) > { > struct sched_entity *se = &p->se; > struct cfs_rq *cfs_rq = cfs_rq_of(se); > struct load_weight *load = &se->load; > > + update_load_avg(cfs_rq, se, 0); IIUC, you only want to sync the sleeping task with its cfs_rq. IMHO, sync_entity_load_avg() should be used here instead of update_load_avg(). The latter is doing much more than this. > reweight_entity(cfs_rq, se, lw->weight); > load->inv_weight = lw->inv_weight; > } Maybe even do this in reweight_entity()?. You would have to do it under 'if (!se->on_rq) in reweight_task() anyway I assume. -->8-- diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c index 9057584ec06d..555392be4e82 100644 --- a/kernel/sched/fair.c +++ b/kernel/sched/fair.c @@ -3669,11 +3669,31 @@ dequeue_load_avg(struct cfs_rq *cfs_rq, struct sched_entity *se) cfs_rq->avg.load_sum = max_t(u32, cfs_rq->avg.load_sum, cfs_rq->avg.load_avg * PELT_MIN_DIVIDER); } + +static inline u64 cfs_rq_last_update_time(struct cfs_rq *cfs_rq) +{ + return u64_u32_load_copy(cfs_rq->avg.last_update_time, + cfs_rq->last_update_time_copy); +} + +/* + * Synchronize entity load avg of dequeued entity without locking + * the previous rq. + */ +static void sync_entity_load_avg(struct sched_entity *se) +{ + struct cfs_rq *cfs_rq = cfs_rq_of(se); + u64 last_update_time; + + last_update_time = cfs_rq_last_update_time(cfs_rq); + __update_load_avg_blocked_se(last_update_time, se); +} #else static inline void enqueue_load_avg(struct cfs_rq *cfs_rq, struct sched_entity *se) { } static inline void dequeue_load_avg(struct cfs_rq *cfs_rq, struct sched_entity *se) { } +static void sync_entity_load_avg(struct sched_entity *se) { } #endif static void reweight_eevdf(struct sched_entity *se, u64 avruntime, @@ -3795,7 +3815,10 @@ static void reweight_entity(struct cfs_rq *cfs_rq, struct sched_entity *se, if (!curr) __dequeue_entity(cfs_rq, se); update_load_sub(&cfs_rq->load, se->load.weight); + } else if (entity_is_task(se)) { + sync_entity_load_avg(se); } + dequeue_load_avg(cfs_rq, se); if (se->on_rq) { @@ -4033,12 +4056,6 @@ static inline bool load_avg_is_decayed(struct sched_avg *sa) return true; } - -static inline u64 cfs_rq_last_update_time(struct cfs_rq *cfs_rq) -{ - return u64_u32_load_copy(cfs_rq->avg.last_update_time, - cfs_rq->last_update_time_copy); -} #ifdef CONFIG_FAIR_GROUP_SCHED /* * Because list_add_leaf_cfs_rq always places a child cfs_rq on the list @@ -4773,19 +4790,6 @@ static inline void update_load_avg(struct cfs_rq *cfs_rq, struct sched_entity *s } } -/* - * Synchronize entity load avg of dequeued entity without locking - * the previous rq. - */ -static void sync_entity_load_avg(struct sched_entity *se) -{ - struct cfs_rq *cfs_rq = cfs_rq_of(se); - u64 last_update_time; - - last_update_time = cfs_rq_last_update_time(cfs_rq); - __update_load_avg_blocked_se(last_update_time, se); -} - /* * Task first catches up with cfs_rq, and then subtract * itself from the cfs_rq (task must be off the queue now). ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] sched/fair: Sync se's load_avg with cfs_rq in reweight_task 2024-07-22 6:34 ` Dietmar Eggemann @ 2024-07-22 9:39 ` Chuyi Zhou 0 siblings, 0 replies; 8+ messages in thread From: Chuyi Zhou @ 2024-07-22 9:39 UTC (permalink / raw) To: Dietmar Eggemann, mingo, peterz, juri.lelli, vincent.guittot, rostedt, bsegall, mgorman, vschneid Cc: chengming.zhou, linux-kernel Hello, 在 2024/7/22 14:34, Dietmar Eggemann 写道: > On 20/07/2024 07:12, Chuyi Zhou wrote: >> In reweight_task(), there are two situations: >> >> 1. The task was on_rq, then the task's load_avg is accurate because we >> synchronized it with cfs_rq through update_load_avg() in dequeue_task(). >> >> 2. The task is sleeping, its load_avg might not have been updated for some >> time, which can result in inaccurate dequeue_load_avg() in >> reweight_entity(). >> >> This patch solves this by using update_load_avg() to synchronize the >> load_avg of se with cfs_rq. For tasks were on_rq, since we already update >> load_avg to accurate values in dequeue_task(), this change will not have >> other effects due to the short time interval between the two updates. >> >> Signed-off-by: Chuyi Zhou <zhouchuyi@bytedance.com> >> --- >> Changes in v2: >> - change the description in commit log. >> - use update_load_avg() in reweight_task() rather than in reweight_entity >> suggested by chengming. >> - Link to v1: https://lore.kernel.org/lkml/20240716150840.23061-1-zhouchuyi@bytedance.com/ >> --- >> kernel/sched/fair.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c >> index 9057584ec06d..b1e07ce90284 100644 >> --- a/kernel/sched/fair.c >> +++ b/kernel/sched/fair.c >> @@ -3835,12 +3835,15 @@ static void reweight_entity(struct cfs_rq *cfs_rq, struct sched_entity *se, >> } >> } >> >> +static inline void update_load_avg(struct cfs_rq *cfs_rq, struct sched_entity *se, int flags); >> + >> void reweight_task(struct task_struct *p, const struct load_weight *lw) >> { >> struct sched_entity *se = &p->se; >> struct cfs_rq *cfs_rq = cfs_rq_of(se); >> struct load_weight *load = &se->load; >> >> + update_load_avg(cfs_rq, se, 0); > > IIUC, you only want to sync the sleeping task with its cfs_rq. IMHO, > sync_entity_load_avg() should be used here instead of update_load_avg(). > The latter is doing much more than this. Indeed, sync_entity_load_avg() is better. > >> reweight_entity(cfs_rq, se, lw->weight); >> load->inv_weight = lw->inv_weight; >> } > > Maybe even do this in reweight_entity()?. You would have to do it under > 'if (!se->on_rq) in reweight_task() anyway I assume. > Yes, we can do it reweight_entity() and it's more clear. Thanks for your suggestion. ^ permalink raw reply [flat|nested] 8+ messages in thread
[parent not found: <66ee1520-48b8-4229-8bdd-9746c8bea5fe@bytedance.com>]
* Re: [PATCH v2] sched/fair: Sync se's load_avg with cfs_rq in reweight_task [not found] <66ee1520-48b8-4229-8bdd-9746c8bea5fe@bytedance.com> @ 2024-07-30 10:26 ` Chuyi Zhou 0 siblings, 0 replies; 8+ messages in thread From: Chuyi Zhou @ 2024-07-30 10:26 UTC (permalink / raw) To: linux-kernel -------- 转发的消息 -------- 主题: Re: [PATCH v2] sched/fair: Sync se's load_avg with cfs_rq in reweight_task 日期: Mon, 29 Jul 2024 11:06:33 +0800 From: Chuyi Zhou <zhouchuyi@bytedance.com> 收件人: Qais Yousef <qyousef@layalina.io>, K Prateek Nayak <kprateek.nayak@amd.com> 抄送: mingo@redhat.com, peterz@infradead.org, juri.lelli@redhat.com, vincent.guittot@linaro.org, dietmar.eggemann@arm.com, rostedt@goodmis.org, bsegall@google.com, mgorman@suse.de, vschneid@redhat.com, chengming.zhou@linux.dev Hello Qais, 在 2024/7/29 04:14, Qais Yousef 写道: > On 07/22/24 10:47, K Prateek Nayak wrote: >> (+ Qais) >> >> Hello Chuyi, >> >> On 7/20/2024 10:42 AM, Chuyi Zhou wrote: >>> In reweight_task(), there are two situations: >>> >>> 1. The task was on_rq, then the task's load_avg is accurate because we >>> synchronized it with cfs_rq through update_load_avg() in dequeue_task(). >>> >>> 2. The task is sleeping, its load_avg might not have been updated for some >>> time, which can result in inaccurate dequeue_load_avg() in >>> reweight_entity(). >>> >>> This patch solves this by using update_load_avg() to synchronize the >>> load_avg of se with cfs_rq. For tasks were on_rq, since we already update >>> load_avg to accurate values in dequeue_task(), this change will not have >>> other effects due to the short time interval between the two updates. >>> >>> Signed-off-by: Chuyi Zhou <zhouchuyi@bytedance.com> >>> --- >>> Changes in v2: >>> - change the description in commit log. >>> - use update_load_avg() in reweight_task() rather than in reweight_entity >>> suggested by chengming. >>> - Link to v1: https://lore.kernel.org/lkml/20240716150840.23061-1-zhouchuyi@bytedance.com/ >>> --- >>> kernel/sched/fair.c | 3 +++ >>> 1 file changed, 3 insertions(+) >>> >>> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c >>> index 9057584ec06d..b1e07ce90284 100644 >>> --- a/kernel/sched/fair.c >>> +++ b/kernel/sched/fair.c >>> @@ -3835,12 +3835,15 @@ static void reweight_entity(struct cfs_rq *cfs_rq, struct sched_entity *se, >>> } >>> } >>> +static inline void update_load_avg(struct cfs_rq *cfs_rq, struct sched_entity *se, int flags); >>> + >>> void reweight_task(struct task_struct *p, const struct load_weight *lw) >>> { >>> struct sched_entity *se = &p->se; >>> struct cfs_rq *cfs_rq = cfs_rq_of(se); >>> struct load_weight *load = &se->load; >>> + update_load_avg(cfs_rq, se, 0); > > White space and a comment perhaps? > > LGTM anyway. Thanks for taking a look, will send another patch later. (V3 link:https://lore.kernel.org/lkml/20240723114247.104848-1-zhouchuyi@bytedance.com/) ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2024-07-30 10:26 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-07-20 5:12 [PATCH v2] sched/fair: Sync se's load_avg with cfs_rq in reweight_task Chuyi Zhou
2024-07-22 5:17 ` K Prateek Nayak
2024-07-22 6:40 ` Chengming Zhou
2024-07-22 8:15 ` K Prateek Nayak
2024-07-28 20:14 ` Qais Yousef
2024-07-22 6:34 ` Dietmar Eggemann
2024-07-22 9:39 ` Chuyi Zhou
[not found] <66ee1520-48b8-4229-8bdd-9746c8bea5fe@bytedance.com>
2024-07-30 10:26 ` Chuyi Zhou
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®