[PATCH V3 0/2] sched: Align uclamp and util_est

[PATCH V3 0/2] sched: Align uclamp and util_est

[Xuewen Yan]

Now, both uclamp and util_est have been adapted for DELAYED_DEQUEUE,
and the double enqueue/dequeue issue no longer exists.
However, there is still room for optimization in both uclamp and util_est.
Previous discussions as following:
https://lore.kernel.org/all/CAB8ipk8pEvOtCm-d0o1rsekwxPWUHk9iBGtt9TLTWW-iWTQKiA@mail.gmail.com/
https://lore.kernel.org/all/84441660bef0a5e67fd09dc3787178d0276dad31.1740664400.git.hongyan.xia2@arm.com/T/#u
https://lore.kernel.org/all/CAB8ipk9LpbiUDnbcV6+59+Sa=Ai7tFzO===mpLD3obNdV4=J-A@mail.gmail.com/T/#u
https://lore.kernel.org/all/aa8baf67-a8ec-4ad8-a6a8-afdcd7036771@arm.com/
https://lore.kernel.org/all/20250325014733.18405-1-xuewen.yan@unisoc.com/

patch[1]: Simply the condition for util_est_dequeue/enqueue; 
patch[2] aligns uclamp and util_est and call before freq update to improve
the performance and power.

Xuewen Yan (2):
  sched/util_est: Simply the condition for util_est_dequeue/enqueue
  sched/uclamp: Align uclamp and util_est and call before freq update

 kernel/sched/core.c | 17 ++++++++++-------
 kernel/sched/fair.c |  4 ++--
 2 files changed, 12 insertions(+), 9 deletions(-)

---
v3:
- split previous patch in 2 patches.
 
v2:
- simply the util-est's en/dequeue check;
---
-- 
2.25.1

[PATCH V3 1/2] sched/util_est: Simply the condition for util_est_dequeue/enqueue

[Xuewen Yan]

To prevent double enqueue/dequeue of the util-est for sched_delayed tasks,
commit 729288bc6856 ("kernel/sched: Fix util_est accounting for DELAY_DEQUEUE")
added the corresponding check. This check excludes double en/dequeue during
task migration and priority changes.

In fact, these conditions can be simplified.

For util_est_dequeue, we know that sched_delayed flag is set in dequeue_entity.
When the task is sleeping, we need to call util_est_dequeue to subtract
util-est from the cfs_rq. At this point, sched_delayed has not yet been set.
If we find that sched_delayed is already set, it indicates that this task
has already called dequeue_task_fair once. In this case, there is no need to
call util_est_dequeue again. Therefore, simply checking the sched_delayed flag
should be sufficient to prevent unnecessary util_est updates during the dequeue.

For util_est_enqueue, our goal is to add the util_est to the cfs_rq
when task enqueue. However, we don't want to add the util_est of a
sched_delayed task to the cfs_rq because the task is sleeping.
Therefore, we can exclude the util_est_enqueue for sched_delayed tasks
by checking the sched_delayed flag. However, when waking up a delayed task,
the sched_delayed flag is cleared after util_est_enqueue. As a result,
if we only check the sched_delayed flag, we would miss the util_est_enqueue.
Since waking up a sched_delayed task calls enqueue_task with the ENQUEUE_DELAYED flag,
we can determine whether to call util_est_enqueue by checking if the
enqueue_flag contains ENQUEUE_DELAYED.

Signed-off-by: Xuewen Yan <xuewen.yan@unisoc.com>
---
v3:
- Separated from the previous patch.
- rework the commit message.
---
 kernel/sched/fair.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index e43993a4e580..18c85857bff0 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -6941,7 +6941,7 @@ enqueue_task_fair(struct rq *rq, struct task_struct *p, int flags)
 	 * Let's add the task's estimated utilization to the cfs_rq's
 	 * estimated utilization, before we update schedutil.
 	 */
-	if (!(p->se.sched_delayed && (task_on_rq_migrating(p) || (flags & ENQUEUE_RESTORE))))
+	if (!p->se.sched_delayed || (flags & ENQUEUE_DELAYED))
 		util_est_enqueue(&rq->cfs, p);
 
 	if (flags & ENQUEUE_DELAYED) {
@@ -7183,7 +7183,7 @@ static int dequeue_entities(struct rq *rq, struct sched_entity *se, int flags)
  */
 static bool dequeue_task_fair(struct rq *rq, struct task_struct *p, int flags)
 {
-	if (!(p->se.sched_delayed && (task_on_rq_migrating(p) || (flags & DEQUEUE_SAVE))))
+	if (!p->se.sched_delayed)
 		util_est_dequeue(&rq->cfs, p);
 
 	util_est_update(&rq->cfs, p, flags & DEQUEUE_SLEEP);
-- 
2.25.1

Re: [PATCH V3 1/2] sched/util_est: Simply the condition for util_est_dequeue/enqueue

[Vincent Guittot]

On Thu, 17 Apr 2025 at 06:36, Xuewen Yan <xuewen.yan@unisoc.com> wrote:
>
> To prevent double enqueue/dequeue of the util-est for sched_delayed tasks,
> commit 729288bc6856 ("kernel/sched: Fix util_est accounting for DELAY_DEQUEUE")
> added the corresponding check. This check excludes double en/dequeue during
> task migration and priority changes.
>
> In fact, these conditions can be simplified.
>
> For util_est_dequeue, we know that sched_delayed flag is set in dequeue_entity.
> When the task is sleeping, we need to call util_est_dequeue to subtract
> util-est from the cfs_rq. At this point, sched_delayed has not yet been set.
> If we find that sched_delayed is already set, it indicates that this task
> has already called dequeue_task_fair once. In this case, there is no need to
> call util_est_dequeue again. Therefore, simply checking the sched_delayed flag
> should be sufficient to prevent unnecessary util_est updates during the dequeue.
>
> For util_est_enqueue, our goal is to add the util_est to the cfs_rq
> when task enqueue. However, we don't want to add the util_est of a
> sched_delayed task to the cfs_rq because the task is sleeping.
> Therefore, we can exclude the util_est_enqueue for sched_delayed tasks
> by checking the sched_delayed flag. However, when waking up a delayed task,
> the sched_delayed flag is cleared after util_est_enqueue. As a result,
> if we only check the sched_delayed flag, we would miss the util_est_enqueue.
> Since waking up a sched_delayed task calls enqueue_task with the ENQUEUE_DELAYED flag,
> we can determine whether to call util_est_enqueue by checking if the
> enqueue_flag contains ENQUEUE_DELAYED.
>
> Signed-off-by: Xuewen Yan <xuewen.yan@unisoc.com>

Reviewed-by: Vincent Guittot <vincent.guittot@linaro.org>


> ---
> v3:
> - Separated from the previous patch.
> - rework the commit message.
> ---
>  kernel/sched/fair.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index e43993a4e580..18c85857bff0 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -6941,7 +6941,7 @@ enqueue_task_fair(struct rq *rq, struct task_struct *p, int flags)
>          * Let's add the task's estimated utilization to the cfs_rq's
>          * estimated utilization, before we update schedutil.
>          */
> -       if (!(p->se.sched_delayed && (task_on_rq_migrating(p) || (flags & ENQUEUE_RESTORE))))
> +       if (!p->se.sched_delayed || (flags & ENQUEUE_DELAYED))
>                 util_est_enqueue(&rq->cfs, p);
>
>         if (flags & ENQUEUE_DELAYED) {
> @@ -7183,7 +7183,7 @@ static int dequeue_entities(struct rq *rq, struct sched_entity *se, int flags)
>   */
>  static bool dequeue_task_fair(struct rq *rq, struct task_struct *p, int flags)
>  {
> -       if (!(p->se.sched_delayed && (task_on_rq_migrating(p) || (flags & DEQUEUE_SAVE))))
> +       if (!p->se.sched_delayed)
>                 util_est_dequeue(&rq->cfs, p);
>
>         util_est_update(&rq->cfs, p, flags & DEQUEUE_SLEEP);
> --
> 2.25.1
>
>

Re: [PATCH V3 1/2] sched/util_est: Simply the condition for util_est_dequeue/enqueue

[Dietmar Eggemann]

On 17/04/2025 06:34, Xuewen Yan wrote:

Nit: Change the subject text to:

"sched/fair: Simplify condition for util_est_dequeue/enqueue"

[...]

[tip: sched/core] sched/util_est: Simplify condition for util_est_{en,de}queue()

[tip-bot2 for Xuewen Yan]

The following commit has been merged into the sched/core branch of tip:

Commit-ID:     0212696a844631a923aa6cedd74ebbb3cf434e51
Gitweb:        https://git.kernel.org/tip/0212696a844631a923aa6cedd74ebbb3cf434e51
Author:        Xuewen Yan <xuewen.yan@unisoc.com>
AuthorDate:    Thu, 17 Apr 2025 12:34:56 +08:00
Committer:     Peter Zijlstra <peterz@infradead.org>
CommitterDate: Wed, 21 May 2025 13:57:38 +02:00

sched/util_est: Simplify condition for util_est_{en,de}queue()

To prevent double enqueue/dequeue of the util-est for sched_delayed tasks,
commit 729288bc6856 ("kernel/sched: Fix util_est accounting for DELAY_DEQUEUE")
added the corresponding check. This check excludes double en/dequeue during
task migration and priority changes.

In fact, these conditions can be simplified.

For util_est_dequeue, we know that sched_delayed flag is set in dequeue_entity.
When the task is sleeping, we need to call util_est_dequeue to subtract
util-est from the cfs_rq. At this point, sched_delayed has not yet been set.
If we find that sched_delayed is already set, it indicates that this task
has already called dequeue_task_fair once. In this case, there is no need to
call util_est_dequeue again. Therefore, simply checking the sched_delayed flag
should be sufficient to prevent unnecessary util_est updates during the dequeue.

For util_est_enqueue, our goal is to add the util_est to the cfs_rq
when task enqueue. However, we don't want to add the util_est of a
sched_delayed task to the cfs_rq because the task is sleeping.
Therefore, we can exclude the util_est_enqueue for sched_delayed tasks
by checking the sched_delayed flag. However, when waking up a delayed task,
the sched_delayed flag is cleared after util_est_enqueue. As a result,
if we only check the sched_delayed flag, we would miss the util_est_enqueue.
Since waking up a sched_delayed task calls enqueue_task with the ENQUEUE_DELAYED flag,
we can determine whether to call util_est_enqueue by checking if the
enqueue_flag contains ENQUEUE_DELAYED.

Signed-off-by: Xuewen Yan <xuewen.yan@unisoc.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Reviewed-by: Vincent Guittot <vincent.guittot@linaro.org>
Reviewed-by: Dietmar Eggemann <dietmar.eggemann@arm.com>
Tested-by: Dietmar Eggemann <dietmar.eggemann@arm.com>
Link: https://lore.kernel.org/r/20250417043457.10632-2-xuewen.yan@unisoc.com
---
 kernel/sched/fair.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index b00f167..a028c29 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -6936,7 +6936,7 @@ enqueue_task_fair(struct rq *rq, struct task_struct *p, int flags)
 	 * Let's add the task's estimated utilization to the cfs_rq's
 	 * estimated utilization, before we update schedutil.
 	 */
-	if (!(p->se.sched_delayed && (task_on_rq_migrating(p) || (flags & ENQUEUE_RESTORE))))
+	if (!p->se.sched_delayed || (flags & ENQUEUE_DELAYED))
 		util_est_enqueue(&rq->cfs, p);
 
 	if (flags & ENQUEUE_DELAYED) {
@@ -7178,7 +7178,7 @@ static int dequeue_entities(struct rq *rq, struct sched_entity *se, int flags)
  */
 static bool dequeue_task_fair(struct rq *rq, struct task_struct *p, int flags)
 {
-	if (!(p->se.sched_delayed && (task_on_rq_migrating(p) || (flags & DEQUEUE_SAVE))))
+	if (!p->se.sched_delayed)
 		util_est_dequeue(&rq->cfs, p);
 
 	util_est_update(&rq->cfs, p, flags & DEQUEUE_SLEEP);

[PATCH V3 2/2] sched/uclamp: Align uclamp and util_est and call before freq update

[Xuewen Yan]

The commit dfa0a574cbc47 ("sched/uclamg: Handle delayed dequeue")
has add the sched_delayed check to prevent double uclamp_dec/inc.
However, it put the uclamp_rq_inc() after enqueue_task().
This may lead to the following issues:
When a task with uclamp goes through enqueue_task() and could trigger
cpufreq update, its uclamp won't even be considered in the cpufreq
update. It is only after enqueue will the uclamp be added to rq
buckets, and cpufreq will only pick it up at the next update.
This could cause a delay in frequency updating. It may affect
the performance(uclamp_min > 0) or power(uclamp_max < 1024).

So, just like util_est, put the uclamp_rq_inc() before enqueue_task().
And as for the sched_delayed_task, same as util_est, using the
sched_delayed flag to prevent inc the sched_delayed_task's uclamp,
using the ENQUEUE_DELAYED flag to allow inc the sched_delayed_task's uclamp
which is being woken up.

Signed-off-by: Xuewen Yan <xuewen.yan@unisoc.com>
---
v3:
- rework the commit message;
- remove the util_est change;
v2:
- simply the util-est's en/dequeue check;
---
 kernel/sched/core.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index c81cf642dba0..0f4ab0c17c58 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -1752,7 +1752,7 @@ static inline void uclamp_rq_dec_id(struct rq *rq, struct task_struct *p,
 	}
 }
 
-static inline void uclamp_rq_inc(struct rq *rq, struct task_struct *p)
+static inline void uclamp_rq_inc(struct rq *rq, struct task_struct *p, int flags)
 {
 	enum uclamp_id clamp_id;
 
@@ -1768,7 +1768,8 @@ static inline void uclamp_rq_inc(struct rq *rq, struct task_struct *p)
 	if (unlikely(!p->sched_class->uclamp_enabled))
 		return;
 
-	if (p->se.sched_delayed)
+	/* Only inc the delayed task which being woken up. */
+	if (p->se.sched_delayed && !(flags & ENQUEUE_DELAYED))
 		return;
 
 	for_each_clamp_id(clamp_id)
@@ -2036,7 +2037,7 @@ static void __init init_uclamp(void)
 }
 
 #else /* !CONFIG_UCLAMP_TASK */
-static inline void uclamp_rq_inc(struct rq *rq, struct task_struct *p) { }
+static inline void uclamp_rq_inc(struct rq *rq, struct task_struct *p, int flags) { }
 static inline void uclamp_rq_dec(struct rq *rq, struct task_struct *p) { }
 static inline void uclamp_fork(struct task_struct *p) { }
 static inline void uclamp_post_fork(struct task_struct *p) { }
@@ -2072,12 +2073,14 @@ void enqueue_task(struct rq *rq, struct task_struct *p, int flags)
 	if (!(flags & ENQUEUE_NOCLOCK))
 		update_rq_clock(rq);
 
-	p->sched_class->enqueue_task(rq, p, flags);
 	/*
-	 * Must be after ->enqueue_task() because ENQUEUE_DELAYED can clear
-	 * ->sched_delayed.
+	 * Can be before ->enqueue_task() because uclamp considers the
+	 * ENQUEUE_DELAYED task before its ->sched_delayed gets cleared
+	 * in ->enqueue_task().
 	 */
-	uclamp_rq_inc(rq, p);
+	uclamp_rq_inc(rq, p, flags);
+
+	p->sched_class->enqueue_task(rq, p, flags);
 
 	psi_enqueue(p, flags);
 
-- 
2.25.1

Re: [PATCH V3 2/2] sched/uclamp: Align uclamp and util_est and call before freq update

[Vincent Guittot]

On Thu, 17 Apr 2025 at 06:36, Xuewen Yan <xuewen.yan@unisoc.com> wrote:
>
> The commit dfa0a574cbc47 ("sched/uclamg: Handle delayed dequeue")
> has add the sched_delayed check to prevent double uclamp_dec/inc.
> However, it put the uclamp_rq_inc() after enqueue_task().
> This may lead to the following issues:
> When a task with uclamp goes through enqueue_task() and could trigger
> cpufreq update, its uclamp won't even be considered in the cpufreq
> update. It is only after enqueue will the uclamp be added to rq
> buckets, and cpufreq will only pick it up at the next update.
> This could cause a delay in frequency updating. It may affect
> the performance(uclamp_min > 0) or power(uclamp_max < 1024).
>
> So, just like util_est, put the uclamp_rq_inc() before enqueue_task().
> And as for the sched_delayed_task, same as util_est, using the
> sched_delayed flag to prevent inc the sched_delayed_task's uclamp,
> using the ENQUEUE_DELAYED flag to allow inc the sched_delayed_task's uclamp
> which is being woken up.
>
> Signed-off-by: Xuewen Yan <xuewen.yan@unisoc.com>

Reviewed-by: Vincent Guittot <vincent.guittot@linaro.org>


> ---
> v3:
> - rework the commit message;
> - remove the util_est change;
> v2:
> - simply the util-est's en/dequeue check;
> ---
>  kernel/sched/core.c | 17 ++++++++++-------
>  1 file changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index c81cf642dba0..0f4ab0c17c58 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -1752,7 +1752,7 @@ static inline void uclamp_rq_dec_id(struct rq *rq, struct task_struct *p,
>         }
>  }
>
> -static inline void uclamp_rq_inc(struct rq *rq, struct task_struct *p)
> +static inline void uclamp_rq_inc(struct rq *rq, struct task_struct *p, int flags)
>  {
>         enum uclamp_id clamp_id;
>
> @@ -1768,7 +1768,8 @@ static inline void uclamp_rq_inc(struct rq *rq, struct task_struct *p)
>         if (unlikely(!p->sched_class->uclamp_enabled))
>                 return;
>
> -       if (p->se.sched_delayed)
> +       /* Only inc the delayed task which being woken up. */
> +       if (p->se.sched_delayed && !(flags & ENQUEUE_DELAYED))
>                 return;
>
>         for_each_clamp_id(clamp_id)
> @@ -2036,7 +2037,7 @@ static void __init init_uclamp(void)
>  }
>
>  #else /* !CONFIG_UCLAMP_TASK */
> -static inline void uclamp_rq_inc(struct rq *rq, struct task_struct *p) { }
> +static inline void uclamp_rq_inc(struct rq *rq, struct task_struct *p, int flags) { }
>  static inline void uclamp_rq_dec(struct rq *rq, struct task_struct *p) { }
>  static inline void uclamp_fork(struct task_struct *p) { }
>  static inline void uclamp_post_fork(struct task_struct *p) { }
> @@ -2072,12 +2073,14 @@ void enqueue_task(struct rq *rq, struct task_struct *p, int flags)
>         if (!(flags & ENQUEUE_NOCLOCK))
>                 update_rq_clock(rq);
>
> -       p->sched_class->enqueue_task(rq, p, flags);
>         /*
> -        * Must be after ->enqueue_task() because ENQUEUE_DELAYED can clear
> -        * ->sched_delayed.
> +        * Can be before ->enqueue_task() because uclamp considers the
> +        * ENQUEUE_DELAYED task before its ->sched_delayed gets cleared
> +        * in ->enqueue_task().
>          */
> -       uclamp_rq_inc(rq, p);
> +       uclamp_rq_inc(rq, p, flags);
> +
> +       p->sched_class->enqueue_task(rq, p, flags);
>
>         psi_enqueue(p, flags);
>
> --
> 2.25.1
>
>

[tip: sched/core] sched/uclamp: Align uclamp and util_est and call before freq update

[tip-bot2 for Xuewen Yan]

The following commit has been merged into the sched/core branch of tip:

Commit-ID:     90ca9410dab21c407706726b86b6e50c6698b5af
Gitweb:        https://git.kernel.org/tip/90ca9410dab21c407706726b86b6e50c6698b5af
Author:        Xuewen Yan <xuewen.yan@unisoc.com>
AuthorDate:    Thu, 17 Apr 2025 12:34:57 +08:00
Committer:     Peter Zijlstra <peterz@infradead.org>
CommitterDate: Wed, 21 May 2025 13:57:38 +02:00

sched/uclamp: Align uclamp and util_est and call before freq update

The commit dfa0a574cbc47 ("sched/uclamg: Handle delayed dequeue")
has add the sched_delayed check to prevent double uclamp_dec/inc.
However, it put the uclamp_rq_inc() after enqueue_task().
This may lead to the following issues:
When a task with uclamp goes through enqueue_task() and could trigger
cpufreq update, its uclamp won't even be considered in the cpufreq
update. It is only after enqueue will the uclamp be added to rq
buckets, and cpufreq will only pick it up at the next update.
This could cause a delay in frequency updating. It may affect
the performance(uclamp_min > 0) or power(uclamp_max < 1024).

So, just like util_est, put the uclamp_rq_inc() before enqueue_task().
And as for the sched_delayed_task, same as util_est, using the
sched_delayed flag to prevent inc the sched_delayed_task's uclamp,
using the ENQUEUE_DELAYED flag to allow inc the sched_delayed_task's uclamp
which is being woken up.

Signed-off-by: Xuewen Yan <xuewen.yan@unisoc.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Reviewed-by: Vincent Guittot <vincent.guittot@linaro.org>
Reviewed-by: Dietmar Eggemann <dietmar.eggemann@arm.com>
Tested-by: Dietmar Eggemann <dietmar.eggemann@arm.com>
Link: https://lore.kernel.org/r/20250417043457.10632-3-xuewen.yan@unisoc.com
---
 kernel/sched/core.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index bece0ba..64c8757 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -1753,7 +1753,7 @@ static inline void uclamp_rq_dec_id(struct rq *rq, struct task_struct *p,
 	}
 }
 
-static inline void uclamp_rq_inc(struct rq *rq, struct task_struct *p)
+static inline void uclamp_rq_inc(struct rq *rq, struct task_struct *p, int flags)
 {
 	enum uclamp_id clamp_id;
 
@@ -1769,7 +1769,8 @@ static inline void uclamp_rq_inc(struct rq *rq, struct task_struct *p)
 	if (unlikely(!p->sched_class->uclamp_enabled))
 		return;
 
-	if (p->se.sched_delayed)
+	/* Only inc the delayed task which being woken up. */
+	if (p->se.sched_delayed && !(flags & ENQUEUE_DELAYED))
 		return;
 
 	for_each_clamp_id(clamp_id)
@@ -2037,7 +2038,7 @@ static void __init init_uclamp(void)
 }
 
 #else /* !CONFIG_UCLAMP_TASK */
-static inline void uclamp_rq_inc(struct rq *rq, struct task_struct *p) { }
+static inline void uclamp_rq_inc(struct rq *rq, struct task_struct *p, int flags) { }
 static inline void uclamp_rq_dec(struct rq *rq, struct task_struct *p) { }
 static inline void uclamp_fork(struct task_struct *p) { }
 static inline void uclamp_post_fork(struct task_struct *p) { }
@@ -2073,12 +2074,14 @@ void enqueue_task(struct rq *rq, struct task_struct *p, int flags)
 	if (!(flags & ENQUEUE_NOCLOCK))
 		update_rq_clock(rq);
 
-	p->sched_class->enqueue_task(rq, p, flags);
 	/*
-	 * Must be after ->enqueue_task() because ENQUEUE_DELAYED can clear
-	 * ->sched_delayed.
+	 * Can be before ->enqueue_task() because uclamp considers the
+	 * ENQUEUE_DELAYED task before its ->sched_delayed gets cleared
+	 * in ->enqueue_task().
 	 */
-	uclamp_rq_inc(rq, p);
+	uclamp_rq_inc(rq, p, flags);
+
+	p->sched_class->enqueue_task(rq, p, flags);
 
 	psi_enqueue(p, flags);
 

Re: [PATCH V3 0/2] sched: Align uclamp and util_est

[Dietmar Eggemann]

On 17/04/2025 06:34, Xuewen Yan wrote:
> Now, both uclamp and util_est have been adapted for DELAYED_DEQUEUE,
> and the double enqueue/dequeue issue no longer exists.
> However, there is still room for optimization in both uclamp and util_est.
> Previous discussions as following:
> https://lore.kernel.org/all/CAB8ipk8pEvOtCm-d0o1rsekwxPWUHk9iBGtt9TLTWW-iWTQKiA@mail.gmail.com/
> https://lore.kernel.org/all/84441660bef0a5e67fd09dc3787178d0276dad31.1740664400.git.hongyan.xia2@arm.com/T/#u
> https://lore.kernel.org/all/CAB8ipk9LpbiUDnbcV6+59+Sa=Ai7tFzO===mpLD3obNdV4=J-A@mail.gmail.com/T/#u
> https://lore.kernel.org/all/aa8baf67-a8ec-4ad8-a6a8-afdcd7036771@arm.com/
> https://lore.kernel.org/all/20250325014733.18405-1-xuewen.yan@unisoc.com/
> 
> patch[1]: Simply the condition for util_est_dequeue/enqueue; 
> patch[2] aligns uclamp and util_est and call before freq update to improve
> the performance and power.
> 
> Xuewen Yan (2):
>   sched/util_est: Simply the condition for util_est_dequeue/enqueue
>   sched/uclamp: Align uclamp and util_est and call before freq update
> 
>  kernel/sched/core.c | 17 ++++++++++-------
>  kernel/sched/fair.c |  4 ++--
>  2 files changed, 12 insertions(+), 9 deletions(-)
> 
> ---
> v3:
> - split previous patch in 2 patches.
>  
> v2:
> - simply the util-est's en/dequeue check;
> ---

LGTM.

Reviewed-by: Dietmar Eggemann <dietmar.eggemann@arm.com>
Tested-by: Dietmar Eggemann <dietmar.eggemann@arm.com>

I assume you can add Hongyan's test tag from:
https://lkml.kernel.org/r/be0cace9-d173-4de3-959e-861876ad77fc@arm.com
as well.

Re: [PATCH V3 0/2] sched: Align uclamp and util_est

[Xuewen Yan]

Gentle ping if forgotten.

Sorry to ask, but may I know if this patch can be merged into the mainline?

Thanks!

On Tue, Apr 22, 2025 at 11:27 PM Dietmar Eggemann
<dietmar.eggemann@arm.com> wrote:
>
> On 17/04/2025 06:34, Xuewen Yan wrote:
> > Now, both uclamp and util_est have been adapted for DELAYED_DEQUEUE,
> > and the double enqueue/dequeue issue no longer exists.
> > However, there is still room for optimization in both uclamp and util_est.
> > Previous discussions as following:
> > https://lore.kernel.org/all/CAB8ipk8pEvOtCm-d0o1rsekwxPWUHk9iBGtt9TLTWW-iWTQKiA@mail.gmail.com/
> > https://lore.kernel.org/all/84441660bef0a5e67fd09dc3787178d0276dad31.1740664400.git.hongyan.xia2@arm.com/T/#u
> > https://lore.kernel.org/all/CAB8ipk9LpbiUDnbcV6+59+Sa=Ai7tFzO===mpLD3obNdV4=J-A@mail.gmail.com/T/#u
> > https://lore.kernel.org/all/aa8baf67-a8ec-4ad8-a6a8-afdcd7036771@arm.com/
> > https://lore.kernel.org/all/20250325014733.18405-1-xuewen.yan@unisoc.com/
> >
> > patch[1]: Simply the condition for util_est_dequeue/enqueue;
> > patch[2] aligns uclamp and util_est and call before freq update to improve
> > the performance and power.
> >
> > Xuewen Yan (2):
> >   sched/util_est: Simply the condition for util_est_dequeue/enqueue
> >   sched/uclamp: Align uclamp and util_est and call before freq update
> >
> >  kernel/sched/core.c | 17 ++++++++++-------
> >  kernel/sched/fair.c |  4 ++--
> >  2 files changed, 12 insertions(+), 9 deletions(-)
> >
> > ---
> > v3:
> > - split previous patch in 2 patches.
> >
> > v2:
> > - simply the util-est's en/dequeue check;
> > ---
>
> LGTM.
>
> Reviewed-by: Dietmar Eggemann <dietmar.eggemann@arm.com>
> Tested-by: Dietmar Eggemann <dietmar.eggemann@arm.com>
>
> I assume you can add Hongyan's test tag from:
> https://lkml.kernel.org/r/be0cace9-d173-4de3-959e-861876ad77fc@arm.com
> as well.
>