mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v2 1/2] sched/uclamp: Always using uclamp_is_used()
@ 2025-02-19  9:37 Xuewen Yan
  2025-02-19  9:37 ` [PATCH v2 2/2] sched/uclamp: Add uclamp_is_used() check before enable it Xuewen Yan
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Xuewen Yan @ 2025-02-19  9:37 UTC (permalink / raw)
  To: vincent.guittot, mingo, peterz, juri.lelli
  Cc: dietmar.eggemann, rostedt, bsegall, mgorman, vschneid,
	christian.loehle, hongyan.xia2, ke.wang, di.shen, xuewen.yan94,
	linux-kernel

Now, we have the uclamp_is_used() func to judge the uclamp enabled,
so replace the static_branch_unlikely(&sched_uclamp_used) with it.

Signed-off-by: Xuewen Yan <xuewen.yan@unisoc.com>
Reviewed-by: Hongyan Xia <hongyan.xia2@arm.com>
Reviewed-by: Christian Loehle <christian.loehle@arm.com>
Reviewed-by: Vincent Guittot <vincent.guittot@linaro.org>
---
V2:
- Correct the spelling mistakes in the commit message (Hongyan)
- Add Reviewed-by
---
---
 kernel/sched/core.c  |  4 ++--
 kernel/sched/sched.h | 28 ++++++++++++++--------------
 2 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 9aecd914ac69..38a7192bfc19 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -1757,7 +1757,7 @@ static inline void uclamp_rq_inc(struct rq *rq, struct task_struct *p)
 	 * The condition is constructed such that a NOP is generated when
 	 * sched_uclamp_used is disabled.
 	 */
-	if (!static_branch_unlikely(&sched_uclamp_used))
+	if (!uclamp_is_used())
 		return;
 
 	if (unlikely(!p->sched_class->uclamp_enabled))
@@ -1784,7 +1784,7 @@ static inline void uclamp_rq_dec(struct rq *rq, struct task_struct *p)
 	 * The condition is constructed such that a NOP is generated when
 	 * sched_uclamp_used is disabled.
 	 */
-	if (!static_branch_unlikely(&sched_uclamp_used))
+	if (!uclamp_is_used())
 		return;
 
 	if (unlikely(!p->sched_class->uclamp_enabled))
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index b93c8c3dc05a..3624fdce5536 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -3394,6 +3394,19 @@ static inline bool update_other_load_avgs(struct rq *rq) { return false; }
 
 unsigned long uclamp_eff_value(struct task_struct *p, enum uclamp_id clamp_id);
 
+/*
+ * When uclamp is compiled in, the aggregation at rq level is 'turned off'
+ * by default in the fast path and only gets turned on once userspace performs
+ * an operation that requires it.
+ *
+ * Returns true if userspace opted-in to use uclamp and aggregation at rq level
+ * hence is active.
+ */
+static inline bool uclamp_is_used(void)
+{
+	return static_branch_likely(&sched_uclamp_used);
+}
+
 static inline unsigned long uclamp_rq_get(struct rq *rq,
 					  enum uclamp_id clamp_id)
 {
@@ -3417,7 +3430,7 @@ static inline bool uclamp_rq_is_capped(struct rq *rq)
 	unsigned long rq_util;
 	unsigned long max_util;
 
-	if (!static_branch_likely(&sched_uclamp_used))
+	if (!uclamp_is_used())
 		return false;
 
 	rq_util = cpu_util_cfs(cpu_of(rq)) + cpu_util_rt(rq);
@@ -3426,19 +3439,6 @@ static inline bool uclamp_rq_is_capped(struct rq *rq)
 	return max_util != SCHED_CAPACITY_SCALE && rq_util >= max_util;
 }
 
-/*
- * When uclamp is compiled in, the aggregation at rq level is 'turned off'
- * by default in the fast path and only gets turned on once userspace performs
- * an operation that requires it.
- *
- * Returns true if userspace opted-in to use uclamp and aggregation at rq level
- * hence is active.
- */
-static inline bool uclamp_is_used(void)
-{
-	return static_branch_likely(&sched_uclamp_used);
-}
-
 #define for_each_clamp_id(clamp_id) \
 	for ((clamp_id) = 0; (clamp_id) < UCLAMP_CNT; (clamp_id)++)
 
-- 
2.25.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v2 2/2] sched/uclamp: Add uclamp_is_used() check before enable it
  2025-02-19  9:37 [PATCH v2 1/2] sched/uclamp: Always using uclamp_is_used() Xuewen Yan
@ 2025-02-19  9:37 ` Xuewen Yan
  2025-02-19 15:59   ` Christian Loehle
                     ` (3 more replies)
  2025-02-19 15:57 ` [PATCH v2 1/2] sched/uclamp: Always using uclamp_is_used() Christian Loehle
                   ` (2 subsequent siblings)
  3 siblings, 4 replies; 9+ messages in thread
From: Xuewen Yan @ 2025-02-19  9:37 UTC (permalink / raw)
  To: vincent.guittot, mingo, peterz, juri.lelli
  Cc: dietmar.eggemann, rostedt, bsegall, mgorman, vschneid,
	christian.loehle, hongyan.xia2, ke.wang, di.shen, xuewen.yan94,
	linux-kernel

Because the static_branch_enable() would get the cpus_read_lock(),
and sometimes users may frequently set the uclamp value of tasks,
and this operation would call the static_branch_enable()
frequently, so add the uclamp_is_used() check to prevent calling
the cpus_read_lock() frequently.
And to make the code more concise, add a helper function to encapsulate
this and use it everywhere we enable sched_uclamp_used.

Signed-off-by: Xuewen Yan <xuewen.yan@unisoc.com>
---
V2:
- change some commit message;
- Add a helper function and use it everywhere when enable uclamp (Vincent)
---
---
 kernel/sched/core.c     |  6 +++---
 kernel/sched/sched.h    | 14 ++++++++++++++
 kernel/sched/syscalls.c |  2 +-
 3 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 38a7192bfc19..0466a2f61b99 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -1942,12 +1942,12 @@ static int sysctl_sched_uclamp_handler(const struct ctl_table *table, int write,
 	}
 
 	if (update_root_tg) {
-		static_branch_enable(&sched_uclamp_used);
+		sched_uclamp_enable();
 		uclamp_update_root_tg();
 	}
 
 	if (old_min_rt != sysctl_sched_uclamp_util_min_rt_default) {
-		static_branch_enable(&sched_uclamp_used);
+		sched_uclamp_enable();
 		uclamp_sync_util_min_rt_default();
 	}
 
@@ -9295,7 +9295,7 @@ static ssize_t cpu_uclamp_write(struct kernfs_open_file *of, char *buf,
 	if (req.ret)
 		return req.ret;
 
-	static_branch_enable(&sched_uclamp_used);
+	sched_uclamp_enable();
 
 	guard(mutex)(&uclamp_mutex);
 	guard(rcu)();
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index 3624fdce5536..bd22af347d0b 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -3407,6 +3407,18 @@ static inline bool uclamp_is_used(void)
 	return static_branch_likely(&sched_uclamp_used);
 }
 
+/*
+ * Enabling static branches would get the cpus_read_lock(),
+ * check whether uclamp_is_used before enable it to avoid always
+ * calling cpus_read_lock(). Because we never disable this
+ * static key once enable it.
+ */
+static inline void sched_uclamp_enable(void)
+{
+	if (!uclamp_is_used())
+		static_branch_enable(&sched_uclamp_used);
+}
+
 static inline unsigned long uclamp_rq_get(struct rq *rq,
 					  enum uclamp_id clamp_id)
 {
@@ -3486,6 +3498,8 @@ static inline bool uclamp_is_used(void)
 	return false;
 }
 
+static inline void sched_uclamp_enable(void) {}
+
 static inline unsigned long
 uclamp_rq_get(struct rq *rq, enum uclamp_id clamp_id)
 {
diff --git a/kernel/sched/syscalls.c b/kernel/sched/syscalls.c
index 456d339be98f..9100a77e9d79 100644
--- a/kernel/sched/syscalls.c
+++ b/kernel/sched/syscalls.c
@@ -368,7 +368,7 @@ static int uclamp_validate(struct task_struct *p,
 	 * blocking operation which obviously cannot be done while holding
 	 * scheduler locks.
 	 */
-	static_branch_enable(&sched_uclamp_used);
+	sched_uclamp_enable();
 
 	return 0;
 }
-- 
2.25.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 1/2] sched/uclamp: Always using uclamp_is_used()
  2025-02-19  9:37 [PATCH v2 1/2] sched/uclamp: Always using uclamp_is_used() Xuewen Yan
  2025-02-19  9:37 ` [PATCH v2 2/2] sched/uclamp: Add uclamp_is_used() check before enable it Xuewen Yan
@ 2025-02-19 15:57 ` Christian Loehle
  2025-03-14 20:24 ` [tip: sched/core] " tip-bot2 for Xuewen Yan
  2025-03-15  9:37 ` [tip: sched/core] sched/uclamp: Use the uclamp_is_used() helper instead of open-coding it tip-bot2 for Xuewen Yan
  3 siblings, 0 replies; 9+ messages in thread
From: Christian Loehle @ 2025-02-19 15:57 UTC (permalink / raw)
  To: Xuewen Yan, vincent.guittot, mingo, peterz, juri.lelli
  Cc: dietmar.eggemann, rostedt, bsegall, mgorman, vschneid,
	hongyan.xia2, ke.wang, di.shen, xuewen.yan94, linux-kernel

On 2/19/25 09:37, Xuewen Yan wrote:
> Now, we have the uclamp_is_used() func to judge the uclamp enabled,
> so replace the static_branch_unlikely(&sched_uclamp_used) with it.
> 
> Signed-off-by: Xuewen Yan <xuewen.yan@unisoc.com>
> Reviewed-by: Hongyan Xia <hongyan.xia2@arm.com>
> Reviewed-by: Christian Loehle <christian.loehle@arm.com>
> Reviewed-by: Vincent Guittot <vincent.guittot@linaro.org>
> ---
> V2:
> - Correct the spelling mistakes in the commit message (Hongyan)
> - Add Reviewed-by
> ---
> ---
>  kernel/sched/core.c  |  4 ++--
>  kernel/sched/sched.h | 28 ++++++++++++++--------------
>  2 files changed, 16 insertions(+), 16 deletions(-)
> 
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index 9aecd914ac69..38a7192bfc19 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -1757,7 +1757,7 @@ static inline void uclamp_rq_inc(struct rq *rq, struct task_struct *p)
>  	 * The condition is constructed such that a NOP is generated when
>  	 * sched_uclamp_used is disabled.
>  	 */
> -	if (!static_branch_unlikely(&sched_uclamp_used))
> +	if (!uclamp_is_used())
>  		return;
>  
>  	if (unlikely(!p->sched_class->uclamp_enabled))
> @@ -1784,7 +1784,7 @@ static inline void uclamp_rq_dec(struct rq *rq, struct task_struct *p)
>  	 * The condition is constructed such that a NOP is generated when
>  	 * sched_uclamp_used is disabled.
>  	 */
> -	if (!static_branch_unlikely(&sched_uclamp_used))
> +	if (!uclamp_is_used())
>  		return;

v2 includes the above two which inverse the likely annotation FYI.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 2/2] sched/uclamp: Add uclamp_is_used() check before enable it
  2025-02-19  9:37 ` [PATCH v2 2/2] sched/uclamp: Add uclamp_is_used() check before enable it Xuewen Yan
@ 2025-02-19 15:59   ` Christian Loehle
  2025-02-19 16:25   ` Vincent Guittot
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 9+ messages in thread
From: Christian Loehle @ 2025-02-19 15:59 UTC (permalink / raw)
  To: Xuewen Yan, vincent.guittot, mingo, peterz, juri.lelli
  Cc: dietmar.eggemann, rostedt, bsegall, mgorman, vschneid,
	hongyan.xia2, ke.wang, di.shen, xuewen.yan94, linux-kernel

On 2/19/25 09:37, Xuewen Yan wrote:
> Because the static_branch_enable() would get the cpus_read_lock(),
> and sometimes users may frequently set the uclamp value of tasks,
> and this operation would call the static_branch_enable()
> frequently, so add the uclamp_is_used() check to prevent calling
> the cpus_read_lock() frequently.
> And to make the code more concise, add a helper function to encapsulate
> this and use it everywhere we enable sched_uclamp_used.
> 
> Signed-off-by: Xuewen Yan <xuewen.yan@unisoc.com>
> ---
> V2:
> - change some commit message;
> - Add a helper function and use it everywhere when enable uclamp (Vincent)
> ---
> ---
>  kernel/sched/core.c     |  6 +++---
>  kernel/sched/sched.h    | 14 ++++++++++++++
>  kernel/sched/syscalls.c |  2 +-
>  3 files changed, 18 insertions(+), 4 deletions(-)
> 
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index 38a7192bfc19..0466a2f61b99 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -1942,12 +1942,12 @@ static int sysctl_sched_uclamp_handler(const struct ctl_table *table, int write,
>  	}
>  
>  	if (update_root_tg) {
> -		static_branch_enable(&sched_uclamp_used);
> +		sched_uclamp_enable();
>  		uclamp_update_root_tg();
>  	}
>  
>  	if (old_min_rt != sysctl_sched_uclamp_util_min_rt_default) {
> -		static_branch_enable(&sched_uclamp_used);
> +		sched_uclamp_enable();
>  		uclamp_sync_util_min_rt_default();
>  	}
>  
> @@ -9295,7 +9295,7 @@ static ssize_t cpu_uclamp_write(struct kernfs_open_file *of, char *buf,
>  	if (req.ret)
>  		return req.ret;
>  
> -	static_branch_enable(&sched_uclamp_used);
> +	sched_uclamp_enable();
>  
>  	guard(mutex)(&uclamp_mutex);
>  	guard(rcu)();
> diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
> index 3624fdce5536..bd22af347d0b 100644
> --- a/kernel/sched/sched.h
> +++ b/kernel/sched/sched.h
> @@ -3407,6 +3407,18 @@ static inline bool uclamp_is_used(void)
>  	return static_branch_likely(&sched_uclamp_used);
>  }
>  
> +/*
> + * Enabling static branches would get the cpus_read_lock(),
> + * check whether uclamp_is_used before enable it to avoid always
> + * calling cpus_read_lock(). Because we never disable this
> + * static key once enable it.
> + */
> +static inline void sched_uclamp_enable(void)
> +{
> +	if (!uclamp_is_used())
> +		static_branch_enable(&sched_uclamp_used);
> +}
> +
Reviewed-by 
>  static inline unsigned long uclamp_rq_get(struct rq *rq,
>  					  enum uclamp_id clamp_id)
>  {
> @@ -3486,6 +3498,8 @@ static inline bool uclamp_is_used(void)
>  	return false;
>  }
>  
> +static inline void sched_uclamp_enable(void) {}
> +
>  static inline unsigned long
>  uclamp_rq_get(struct rq *rq, enum uclamp_id clamp_id)
>  {
> diff --git a/kernel/sched/syscalls.c b/kernel/sched/syscalls.c
> index 456d339be98f..9100a77e9d79 100644
> --- a/kernel/sched/syscalls.c
> +++ b/kernel/sched/syscalls.c
> @@ -368,7 +368,7 @@ static int uclamp_validate(struct task_struct *p,
>  	 * blocking operation which obviously cannot be done while holding
>  	 * scheduler locks.
>  	 */
> -	static_branch_enable(&sched_uclamp_used);
> +	sched_uclamp_enable();
>  
>  	return 0;
>  }

Reviewed-by: Christian Loehle <christian.loehle@arm.com>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 2/2] sched/uclamp: Add uclamp_is_used() check before enable it
  2025-02-19  9:37 ` [PATCH v2 2/2] sched/uclamp: Add uclamp_is_used() check before enable it Xuewen Yan
  2025-02-19 15:59   ` Christian Loehle
@ 2025-02-19 16:25   ` Vincent Guittot
  2025-03-14 20:24   ` [tip: sched/core] " tip-bot2 for Xuewen Yan
  2025-03-15  9:37   ` [tip: sched/core] sched/uclamp: Optimize sched_uclamp_used static key enabling tip-bot2 for Xuewen Yan
  3 siblings, 0 replies; 9+ messages in thread
From: Vincent Guittot @ 2025-02-19 16:25 UTC (permalink / raw)
  To: Xuewen Yan
  Cc: mingo, peterz, juri.lelli, dietmar.eggemann, rostedt, bsegall,
	mgorman, vschneid, christian.loehle, hongyan.xia2, ke.wang,
	di.shen, xuewen.yan94, linux-kernel

On Wed, 19 Feb 2025 at 10:40, Xuewen Yan <xuewen.yan@unisoc.com> wrote:
>
> Because the static_branch_enable() would get the cpus_read_lock(),
> and sometimes users may frequently set the uclamp value of tasks,
> and this operation would call the static_branch_enable()
> frequently, so add the uclamp_is_used() check to prevent calling
> the cpus_read_lock() frequently.
> And to make the code more concise, add a helper function to encapsulate
> this and use it everywhere we enable sched_uclamp_used.
>
> Signed-off-by: Xuewen Yan <xuewen.yan@unisoc.com>

Apart the small comment below

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

> ---
> V2:
> - change some commit message;
> - Add a helper function and use it everywhere when enable uclamp (Vincent)
> ---
> ---
>  kernel/sched/core.c     |  6 +++---
>  kernel/sched/sched.h    | 14 ++++++++++++++
>  kernel/sched/syscalls.c |  2 +-
>  3 files changed, 18 insertions(+), 4 deletions(-)
>
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index 38a7192bfc19..0466a2f61b99 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -1942,12 +1942,12 @@ static int sysctl_sched_uclamp_handler(const struct ctl_table *table, int write,
>         }
>
>         if (update_root_tg) {
> -               static_branch_enable(&sched_uclamp_used);
> +               sched_uclamp_enable();
>                 uclamp_update_root_tg();
>         }
>
>         if (old_min_rt != sysctl_sched_uclamp_util_min_rt_default) {
> -               static_branch_enable(&sched_uclamp_used);
> +               sched_uclamp_enable();
>                 uclamp_sync_util_min_rt_default();
>         }
>
> @@ -9295,7 +9295,7 @@ static ssize_t cpu_uclamp_write(struct kernfs_open_file *of, char *buf,
>         if (req.ret)
>                 return req.ret;
>
> -       static_branch_enable(&sched_uclamp_used);
> +       sched_uclamp_enable();
>
>         guard(mutex)(&uclamp_mutex);
>         guard(rcu)();
> diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
> index 3624fdce5536..bd22af347d0b 100644
> --- a/kernel/sched/sched.h
> +++ b/kernel/sched/sched.h
> @@ -3407,6 +3407,18 @@ static inline bool uclamp_is_used(void)
>         return static_branch_likely(&sched_uclamp_used);
>  }
>
> +/*
> + * Enabling static branches would get the cpus_read_lock(),
> + * check whether uclamp_is_used before enable it to avoid always
> + * calling cpus_read_lock(). Because we never disable this
> + * static key once enable it.

Maybe, simply:

Enabling static branches would get the cpus_read_lock(),
check uclamp_is_used before enabling it. There is no race issue
because we never disable this static key once enabled.


> + */
> +static inline void sched_uclamp_enable(void)
> +{
> +       if (!uclamp_is_used())
> +               static_branch_enable(&sched_uclamp_used);
> +}
> +
>  static inline unsigned long uclamp_rq_get(struct rq *rq,
>                                           enum uclamp_id clamp_id)
>  {
> @@ -3486,6 +3498,8 @@ static inline bool uclamp_is_used(void)
>         return false;
>  }
>
> +static inline void sched_uclamp_enable(void) {}
> +
>  static inline unsigned long
>  uclamp_rq_get(struct rq *rq, enum uclamp_id clamp_id)
>  {
> diff --git a/kernel/sched/syscalls.c b/kernel/sched/syscalls.c
> index 456d339be98f..9100a77e9d79 100644
> --- a/kernel/sched/syscalls.c
> +++ b/kernel/sched/syscalls.c
> @@ -368,7 +368,7 @@ static int uclamp_validate(struct task_struct *p,
>          * blocking operation which obviously cannot be done while holding
>          * scheduler locks.
>          */
> -       static_branch_enable(&sched_uclamp_used);
> +       sched_uclamp_enable();
>
>         return 0;
>  }
> --
> 2.25.1
>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [tip: sched/core] sched/uclamp: Add uclamp_is_used() check before enable it
  2025-02-19  9:37 ` [PATCH v2 2/2] sched/uclamp: Add uclamp_is_used() check before enable it Xuewen Yan
  2025-02-19 15:59   ` Christian Loehle
  2025-02-19 16:25   ` Vincent Guittot
@ 2025-03-14 20:24   ` tip-bot2 for Xuewen Yan
  2025-03-15  9:37   ` [tip: sched/core] sched/uclamp: Optimize sched_uclamp_used static key enabling tip-bot2 for Xuewen Yan
  3 siblings, 0 replies; 9+ messages in thread
From: tip-bot2 for Xuewen Yan @ 2025-03-14 20:24 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: Xuewen Yan, Peter Zijlstra (Intel),
	Christian Loehle, Vincent Guittot, x86, linux-kernel

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

Commit-ID:     4a14bd0c67d5670844c54356a163aa0ce623dbe1
Gitweb:        https://git.kernel.org/tip/4a14bd0c67d5670844c54356a163aa0ce623dbe1
Author:        Xuewen Yan <xuewen.yan@unisoc.com>
AuthorDate:    Wed, 19 Feb 2025 17:37:47 +08:00
Committer:     Peter Zijlstra <peterz@infradead.org>
CommitterDate: Fri, 14 Mar 2025 21:13:19 +01:00

sched/uclamp: Add uclamp_is_used() check before enable it

Because the static_branch_enable() would get the cpus_read_lock(),
and sometimes users may frequently set the uclamp value of tasks,
and this operation would call the static_branch_enable()
frequently, so add the uclamp_is_used() check to prevent calling
the cpus_read_lock() frequently.
And to make the code more concise, add a helper function to encapsulate
this and use it everywhere we enable sched_uclamp_used.

Signed-off-by: Xuewen Yan <xuewen.yan@unisoc.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Reviewed-by: Christian Loehle <christian.loehle@arm.com>
Reviewed-by: Vincent Guittot <vincent.guittot@linaro.org>
Link: https://lore.kernel.org/r/20250219093747.2612-2-xuewen.yan@unisoc.com
---
 kernel/sched/core.c     |  6 +++---
 kernel/sched/sched.h    | 14 ++++++++++++++
 kernel/sched/syscalls.c |  2 +-
 3 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 45daa41..03d7b63 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -1941,12 +1941,12 @@ static int sysctl_sched_uclamp_handler(const struct ctl_table *table, int write,
 	}
 
 	if (update_root_tg) {
-		static_branch_enable(&sched_uclamp_used);
+		sched_uclamp_enable();
 		uclamp_update_root_tg();
 	}
 
 	if (old_min_rt != sysctl_sched_uclamp_util_min_rt_default) {
-		static_branch_enable(&sched_uclamp_used);
+		sched_uclamp_enable();
 		uclamp_sync_util_min_rt_default();
 	}
 
@@ -9294,7 +9294,7 @@ static ssize_t cpu_uclamp_write(struct kernfs_open_file *of, char *buf,
 	if (req.ret)
 		return req.ret;
 
-	static_branch_enable(&sched_uclamp_used);
+	sched_uclamp_enable();
 
 	guard(mutex)(&uclamp_mutex);
 	guard(rcu)();
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index 8d42d3c..0212a0c 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -3407,6 +3407,18 @@ static inline bool uclamp_is_used(void)
 	return static_branch_likely(&sched_uclamp_used);
 }
 
+/*
+ * Enabling static branches would get the cpus_read_lock(),
+ * check whether uclamp_is_used before enable it to avoid always
+ * calling cpus_read_lock(). Because we never disable this
+ * static key once enable it.
+ */
+static inline void sched_uclamp_enable(void)
+{
+	if (!uclamp_is_used())
+		static_branch_enable(&sched_uclamp_used);
+}
+
 static inline unsigned long uclamp_rq_get(struct rq *rq,
 					  enum uclamp_id clamp_id)
 {
@@ -3486,6 +3498,8 @@ static inline bool uclamp_is_used(void)
 	return false;
 }
 
+static inline void sched_uclamp_enable(void) {}
+
 static inline unsigned long
 uclamp_rq_get(struct rq *rq, enum uclamp_id clamp_id)
 {
diff --git a/kernel/sched/syscalls.c b/kernel/sched/syscalls.c
index 9f40348..c326de1 100644
--- a/kernel/sched/syscalls.c
+++ b/kernel/sched/syscalls.c
@@ -368,7 +368,7 @@ static int uclamp_validate(struct task_struct *p,
 	 * blocking operation which obviously cannot be done while holding
 	 * scheduler locks.
 	 */
-	static_branch_enable(&sched_uclamp_used);
+	sched_uclamp_enable();
 
 	return 0;
 }

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [tip: sched/core] sched/uclamp: Always using uclamp_is_used()
  2025-02-19  9:37 [PATCH v2 1/2] sched/uclamp: Always using uclamp_is_used() Xuewen Yan
  2025-02-19  9:37 ` [PATCH v2 2/2] sched/uclamp: Add uclamp_is_used() check before enable it Xuewen Yan
  2025-02-19 15:57 ` [PATCH v2 1/2] sched/uclamp: Always using uclamp_is_used() Christian Loehle
@ 2025-03-14 20:24 ` tip-bot2 for Xuewen Yan
  2025-03-15  9:37 ` [tip: sched/core] sched/uclamp: Use the uclamp_is_used() helper instead of open-coding it tip-bot2 for Xuewen Yan
  3 siblings, 0 replies; 9+ messages in thread
From: tip-bot2 for Xuewen Yan @ 2025-03-14 20:24 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: Xuewen Yan, Peter Zijlstra (Intel),
	Hongyan Xia, Christian Loehle, Vincent Guittot, x86,
	linux-kernel

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

Commit-ID:     12fc0fdcbd1cc2f906596b77c03d7e5ed58947d1
Gitweb:        https://git.kernel.org/tip/12fc0fdcbd1cc2f906596b77c03d7e5ed58947d1
Author:        Xuewen Yan <xuewen.yan@unisoc.com>
AuthorDate:    Wed, 19 Feb 2025 17:37:46 +08:00
Committer:     Peter Zijlstra <peterz@infradead.org>
CommitterDate: Fri, 14 Mar 2025 21:13:18 +01:00

sched/uclamp: Always using uclamp_is_used()

Now, we have the uclamp_is_used() func to judge the uclamp enabled,
so replace the static_branch_unlikely(&sched_uclamp_used) with it.

Signed-off-by: Xuewen Yan <xuewen.yan@unisoc.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Reviewed-by: Hongyan Xia <hongyan.xia2@arm.com>
Reviewed-by: Christian Loehle <christian.loehle@arm.com>
Reviewed-by: Vincent Guittot <vincent.guittot@linaro.org>
Link: https://lore.kernel.org/r/20250219093747.2612-1-xuewen.yan@unisoc.com
---
 kernel/sched/core.c  |  4 ++--
 kernel/sched/sched.h | 28 ++++++++++++++--------------
 2 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 621cfc7..45daa41 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -1756,7 +1756,7 @@ static inline void uclamp_rq_inc(struct rq *rq, struct task_struct *p)
 	 * The condition is constructed such that a NOP is generated when
 	 * sched_uclamp_used is disabled.
 	 */
-	if (!static_branch_unlikely(&sched_uclamp_used))
+	if (!uclamp_is_used())
 		return;
 
 	if (unlikely(!p->sched_class->uclamp_enabled))
@@ -1783,7 +1783,7 @@ static inline void uclamp_rq_dec(struct rq *rq, struct task_struct *p)
 	 * The condition is constructed such that a NOP is generated when
 	 * sched_uclamp_used is disabled.
 	 */
-	if (!static_branch_unlikely(&sched_uclamp_used))
+	if (!uclamp_is_used())
 		return;
 
 	if (unlikely(!p->sched_class->uclamp_enabled))
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index 023b844..8d42d3c 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -3394,6 +3394,19 @@ static inline bool update_other_load_avgs(struct rq *rq) { return false; }
 
 unsigned long uclamp_eff_value(struct task_struct *p, enum uclamp_id clamp_id);
 
+/*
+ * When uclamp is compiled in, the aggregation at rq level is 'turned off'
+ * by default in the fast path and only gets turned on once userspace performs
+ * an operation that requires it.
+ *
+ * Returns true if userspace opted-in to use uclamp and aggregation at rq level
+ * hence is active.
+ */
+static inline bool uclamp_is_used(void)
+{
+	return static_branch_likely(&sched_uclamp_used);
+}
+
 static inline unsigned long uclamp_rq_get(struct rq *rq,
 					  enum uclamp_id clamp_id)
 {
@@ -3417,7 +3430,7 @@ static inline bool uclamp_rq_is_capped(struct rq *rq)
 	unsigned long rq_util;
 	unsigned long max_util;
 
-	if (!static_branch_likely(&sched_uclamp_used))
+	if (!uclamp_is_used())
 		return false;
 
 	rq_util = cpu_util_cfs(cpu_of(rq)) + cpu_util_rt(rq);
@@ -3426,19 +3439,6 @@ static inline bool uclamp_rq_is_capped(struct rq *rq)
 	return max_util != SCHED_CAPACITY_SCALE && rq_util >= max_util;
 }
 
-/*
- * When uclamp is compiled in, the aggregation at rq level is 'turned off'
- * by default in the fast path and only gets turned on once userspace performs
- * an operation that requires it.
- *
- * Returns true if userspace opted-in to use uclamp and aggregation at rq level
- * hence is active.
- */
-static inline bool uclamp_is_used(void)
-{
-	return static_branch_likely(&sched_uclamp_used);
-}
-
 #define for_each_clamp_id(clamp_id) \
 	for ((clamp_id) = 0; (clamp_id) < UCLAMP_CNT; (clamp_id)++)
 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [tip: sched/core] sched/uclamp: Optimize sched_uclamp_used static key enabling
  2025-02-19  9:37 ` [PATCH v2 2/2] sched/uclamp: Add uclamp_is_used() check before enable it Xuewen Yan
                     ` (2 preceding siblings ...)
  2025-03-14 20:24   ` [tip: sched/core] " tip-bot2 for Xuewen Yan
@ 2025-03-15  9:37   ` tip-bot2 for Xuewen Yan
  3 siblings, 0 replies; 9+ messages in thread
From: tip-bot2 for Xuewen Yan @ 2025-03-15  9:37 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: Xuewen Yan, Peter Zijlstra (Intel),
	Ingo Molnar, Christian Loehle, Vincent Guittot, x86,
	linux-kernel

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

Commit-ID:     4bc45824149ea83a1e293c674e7f542b8127afb6
Gitweb:        https://git.kernel.org/tip/4bc45824149ea83a1e293c674e7f542b8127afb6
Author:        Xuewen Yan <xuewen.yan@unisoc.com>
AuthorDate:    Wed, 19 Feb 2025 17:37:47 +08:00
Committer:     Ingo Molnar <mingo@kernel.org>
CommitterDate: Sat, 15 Mar 2025 10:28:50 +01:00

sched/uclamp: Optimize sched_uclamp_used static key enabling

Repeat calls of static_branch_enable() to an already enabled
static key introduce overhead, because it calls cpus_read_lock().

Users may frequently set the uclamp value of tasks, triggering
the repeat enabling of the sched_uclamp_used static key.

Optimize this and avoid repeat calls to static_branch_enable()
by checking whether it's enabled already.

[ mingo: Rewrote the changelog for legibility ]

Signed-off-by: Xuewen Yan <xuewen.yan@unisoc.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Reviewed-by: Christian Loehle <christian.loehle@arm.com>
Reviewed-by: Vincent Guittot <vincent.guittot@linaro.org>
Link: https://lore.kernel.org/r/20250219093747.2612-2-xuewen.yan@unisoc.com
---
 kernel/sched/core.c     |  6 +++---
 kernel/sched/sched.h    | 14 ++++++++++++++
 kernel/sched/syscalls.c |  2 +-
 3 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 45daa41..03d7b63 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -1941,12 +1941,12 @@ static int sysctl_sched_uclamp_handler(const struct ctl_table *table, int write,
 	}
 
 	if (update_root_tg) {
-		static_branch_enable(&sched_uclamp_used);
+		sched_uclamp_enable();
 		uclamp_update_root_tg();
 	}
 
 	if (old_min_rt != sysctl_sched_uclamp_util_min_rt_default) {
-		static_branch_enable(&sched_uclamp_used);
+		sched_uclamp_enable();
 		uclamp_sync_util_min_rt_default();
 	}
 
@@ -9294,7 +9294,7 @@ static ssize_t cpu_uclamp_write(struct kernfs_open_file *of, char *buf,
 	if (req.ret)
 		return req.ret;
 
-	static_branch_enable(&sched_uclamp_used);
+	sched_uclamp_enable();
 
 	guard(mutex)(&uclamp_mutex);
 	guard(rcu)();
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index 8d42d3c..0212a0c 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -3407,6 +3407,18 @@ static inline bool uclamp_is_used(void)
 	return static_branch_likely(&sched_uclamp_used);
 }
 
+/*
+ * Enabling static branches would get the cpus_read_lock(),
+ * check whether uclamp_is_used before enable it to avoid always
+ * calling cpus_read_lock(). Because we never disable this
+ * static key once enable it.
+ */
+static inline void sched_uclamp_enable(void)
+{
+	if (!uclamp_is_used())
+		static_branch_enable(&sched_uclamp_used);
+}
+
 static inline unsigned long uclamp_rq_get(struct rq *rq,
 					  enum uclamp_id clamp_id)
 {
@@ -3486,6 +3498,8 @@ static inline bool uclamp_is_used(void)
 	return false;
 }
 
+static inline void sched_uclamp_enable(void) {}
+
 static inline unsigned long
 uclamp_rq_get(struct rq *rq, enum uclamp_id clamp_id)
 {
diff --git a/kernel/sched/syscalls.c b/kernel/sched/syscalls.c
index 9f40348..c326de1 100644
--- a/kernel/sched/syscalls.c
+++ b/kernel/sched/syscalls.c
@@ -368,7 +368,7 @@ static int uclamp_validate(struct task_struct *p,
 	 * blocking operation which obviously cannot be done while holding
 	 * scheduler locks.
 	 */
-	static_branch_enable(&sched_uclamp_used);
+	sched_uclamp_enable();
 
 	return 0;
 }

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [tip: sched/core] sched/uclamp: Use the uclamp_is_used() helper instead of open-coding it
  2025-02-19  9:37 [PATCH v2 1/2] sched/uclamp: Always using uclamp_is_used() Xuewen Yan
                   ` (2 preceding siblings ...)
  2025-03-14 20:24 ` [tip: sched/core] " tip-bot2 for Xuewen Yan
@ 2025-03-15  9:37 ` tip-bot2 for Xuewen Yan
  3 siblings, 0 replies; 9+ messages in thread
From: tip-bot2 for Xuewen Yan @ 2025-03-15  9:37 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: Xuewen Yan, Peter Zijlstra (Intel),
	Ingo Molnar, Hongyan Xia, Christian Loehle, Vincent Guittot, x86,
	linux-kernel

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

Commit-ID:     5fca5a4cf97313dd2f5eae29624ceb1dc48f258a
Gitweb:        https://git.kernel.org/tip/5fca5a4cf97313dd2f5eae29624ceb1dc48f258a
Author:        Xuewen Yan <xuewen.yan@unisoc.com>
AuthorDate:    Wed, 19 Feb 2025 17:37:46 +08:00
Committer:     Ingo Molnar <mingo@kernel.org>
CommitterDate: Sat, 15 Mar 2025 10:26:37 +01:00

sched/uclamp: Use the uclamp_is_used() helper instead of open-coding it

Don't open-code static_branch_unlikely(&sched_uclamp_used), we have
the uclamp_is_used() wrapper around it.

[ mingo: Clean up the changelog ]

Signed-off-by: Xuewen Yan <xuewen.yan@unisoc.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Reviewed-by: Hongyan Xia <hongyan.xia2@arm.com>
Reviewed-by: Christian Loehle <christian.loehle@arm.com>
Reviewed-by: Vincent Guittot <vincent.guittot@linaro.org>
Link: https://lore.kernel.org/r/20250219093747.2612-1-xuewen.yan@unisoc.com
---
 kernel/sched/core.c  |  4 ++--
 kernel/sched/sched.h | 28 ++++++++++++++--------------
 2 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 621cfc7..45daa41 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -1756,7 +1756,7 @@ static inline void uclamp_rq_inc(struct rq *rq, struct task_struct *p)
 	 * The condition is constructed such that a NOP is generated when
 	 * sched_uclamp_used is disabled.
 	 */
-	if (!static_branch_unlikely(&sched_uclamp_used))
+	if (!uclamp_is_used())
 		return;
 
 	if (unlikely(!p->sched_class->uclamp_enabled))
@@ -1783,7 +1783,7 @@ static inline void uclamp_rq_dec(struct rq *rq, struct task_struct *p)
 	 * The condition is constructed such that a NOP is generated when
 	 * sched_uclamp_used is disabled.
 	 */
-	if (!static_branch_unlikely(&sched_uclamp_used))
+	if (!uclamp_is_used())
 		return;
 
 	if (unlikely(!p->sched_class->uclamp_enabled))
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index 023b844..8d42d3c 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -3394,6 +3394,19 @@ static inline bool update_other_load_avgs(struct rq *rq) { return false; }
 
 unsigned long uclamp_eff_value(struct task_struct *p, enum uclamp_id clamp_id);
 
+/*
+ * When uclamp is compiled in, the aggregation at rq level is 'turned off'
+ * by default in the fast path and only gets turned on once userspace performs
+ * an operation that requires it.
+ *
+ * Returns true if userspace opted-in to use uclamp and aggregation at rq level
+ * hence is active.
+ */
+static inline bool uclamp_is_used(void)
+{
+	return static_branch_likely(&sched_uclamp_used);
+}
+
 static inline unsigned long uclamp_rq_get(struct rq *rq,
 					  enum uclamp_id clamp_id)
 {
@@ -3417,7 +3430,7 @@ static inline bool uclamp_rq_is_capped(struct rq *rq)
 	unsigned long rq_util;
 	unsigned long max_util;
 
-	if (!static_branch_likely(&sched_uclamp_used))
+	if (!uclamp_is_used())
 		return false;
 
 	rq_util = cpu_util_cfs(cpu_of(rq)) + cpu_util_rt(rq);
@@ -3426,19 +3439,6 @@ static inline bool uclamp_rq_is_capped(struct rq *rq)
 	return max_util != SCHED_CAPACITY_SCALE && rq_util >= max_util;
 }
 
-/*
- * When uclamp is compiled in, the aggregation at rq level is 'turned off'
- * by default in the fast path and only gets turned on once userspace performs
- * an operation that requires it.
- *
- * Returns true if userspace opted-in to use uclamp and aggregation at rq level
- * hence is active.
- */
-static inline bool uclamp_is_used(void)
-{
-	return static_branch_likely(&sched_uclamp_used);
-}
-
 #define for_each_clamp_id(clamp_id) \
 	for ((clamp_id) = 0; (clamp_id) < UCLAMP_CNT; (clamp_id)++)
 

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2025-03-15  9:37 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-02-19  9:37 [PATCH v2 1/2] sched/uclamp: Always using uclamp_is_used() Xuewen Yan
2025-02-19  9:37 ` [PATCH v2 2/2] sched/uclamp: Add uclamp_is_used() check before enable it Xuewen Yan
2025-02-19 15:59   ` Christian Loehle
2025-02-19 16:25   ` Vincent Guittot
2025-03-14 20:24   ` [tip: sched/core] " tip-bot2 for Xuewen Yan
2025-03-15  9:37   ` [tip: sched/core] sched/uclamp: Optimize sched_uclamp_used static key enabling tip-bot2 for Xuewen Yan
2025-02-19 15:57 ` [PATCH v2 1/2] sched/uclamp: Always using uclamp_is_used() Christian Loehle
2025-03-14 20:24 ` [tip: sched/core] " tip-bot2 for Xuewen Yan
2025-03-15  9:37 ` [tip: sched/core] sched/uclamp: Use the uclamp_is_used() helper instead of open-coding it tip-bot2 for Xuewen Yan

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®