The Linux Kernel Mailing List
 help / color / mirror / Atom feed
* [PATCH sched_ext/for-7.1-fixes] sched_ext: Move scx_error() out of scx_link_sched()'s lock region
@ 2026-05-06 18:02 Tejun Heo
  2026-05-07  4:31 ` Andrea Righi
  2026-05-07 21:12 ` Tejun Heo
  0 siblings, 2 replies; 3+ messages in thread
From: Tejun Heo @ 2026-05-06 18:02 UTC (permalink / raw)
  To: David Vernet, Andrea Righi, Changwoo Min, sched-ext
  Cc: linux-kernel, Emil Tsalapatis

scx_link_sched() holds scx_sched_lock. The scx_error() calls inside take the
same lock through scx_claim_exit() and deadlock. Move them out of the guard.

Fixes: 6b4576b09714 ("sched_ext: Reject sub-sched attachment to a disabled parent")
Signed-off-by: Tejun Heo <tj@kernel.org>
---
 kernel/sched/ext.c |   22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

--- a/kernel/sched/ext.c
+++ b/kernel/sched/ext.c
@@ -5661,10 +5661,12 @@ static void refresh_watchdog(void)
 
 static s32 scx_link_sched(struct scx_sched *sch)
 {
+	const char *err_msg;
+	s32 ret = 0;
+
 	scoped_guard(raw_spinlock_irq, &scx_sched_lock) {
 #ifdef CONFIG_EXT_SUB_SCHED
 		struct scx_sched *parent = scx_parent(sch);
-		s32 ret;
 
 		if (parent) {
 			/*
@@ -5674,15 +5676,16 @@ static s32 scx_link_sched(struct scx_sch
 			 * parent can shoot us down.
 			 */
 			if (atomic_read(&parent->exit_kind) != SCX_EXIT_NONE) {
-				scx_error(sch, "parent disabled");
-				return -ENOENT;
+				err_msg = "parent disabled";
+				ret = -ENOENT;
+				break;
 			}
 
 			ret = rhashtable_lookup_insert_fast(&scx_sched_hash,
 					&sch->hash_node, scx_sched_hash_params);
 			if (ret) {
-				scx_error(sch, "failed to insert into scx_sched_hash (%d)", ret);
-				return ret;
+				err_msg = "failed to insert into scx_sched_hash";
+				break;
 			}
 
 			list_add_tail(&sch->sibling, &parent->children);
@@ -5692,6 +5695,15 @@ static s32 scx_link_sched(struct scx_sch
 		list_add_tail_rcu(&sch->all, &scx_sched_all);
 	}
 
+	/*
+	 * scx_error() takes scx_sched_lock via scx_claim_exit(), so it must run after
+	 * the guard above is released.
+	 */
+	if (ret) {
+		scx_error(sch, "%s (%d)", err_msg, ret);
+		return ret;
+	}
+
 	refresh_watchdog();
 	return 0;
 }

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

* Re: [PATCH sched_ext/for-7.1-fixes] sched_ext: Move scx_error() out of scx_link_sched()'s lock region
  2026-05-06 18:02 [PATCH sched_ext/for-7.1-fixes] sched_ext: Move scx_error() out of scx_link_sched()'s lock region Tejun Heo
@ 2026-05-07  4:31 ` Andrea Righi
  2026-05-07 21:12 ` Tejun Heo
  1 sibling, 0 replies; 3+ messages in thread
From: Andrea Righi @ 2026-05-07  4:31 UTC (permalink / raw)
  To: Tejun Heo
  Cc: David Vernet, Changwoo Min, sched-ext, linux-kernel,
	Emil Tsalapatis

Hi Tejun,

On Wed, May 06, 2026 at 08:02:46AM -1000, Tejun Heo wrote:
> scx_link_sched() holds scx_sched_lock. The scx_error() calls inside take the
> same lock through scx_claim_exit() and deadlock. Move them out of the guard.
> 
> Fixes: 6b4576b09714 ("sched_ext: Reject sub-sched attachment to a disabled parent")
> Signed-off-by: Tejun Heo <tj@kernel.org>

Looks good.

Reviewed-by: Andrea Righi <arighi@nvidia.com>

Thanks,
-Andrea

> ---
>  kernel/sched/ext.c |   22 +++++++++++++++++-----
>  1 file changed, 17 insertions(+), 5 deletions(-)
> 
> --- a/kernel/sched/ext.c
> +++ b/kernel/sched/ext.c
> @@ -5661,10 +5661,12 @@ static void refresh_watchdog(void)
>  
>  static s32 scx_link_sched(struct scx_sched *sch)
>  {
> +	const char *err_msg;
> +	s32 ret = 0;
> +
>  	scoped_guard(raw_spinlock_irq, &scx_sched_lock) {
>  #ifdef CONFIG_EXT_SUB_SCHED
>  		struct scx_sched *parent = scx_parent(sch);
> -		s32 ret;
>  
>  		if (parent) {
>  			/*
> @@ -5674,15 +5676,16 @@ static s32 scx_link_sched(struct scx_sch
>  			 * parent can shoot us down.
>  			 */
>  			if (atomic_read(&parent->exit_kind) != SCX_EXIT_NONE) {
> -				scx_error(sch, "parent disabled");
> -				return -ENOENT;
> +				err_msg = "parent disabled";
> +				ret = -ENOENT;
> +				break;
>  			}
>  
>  			ret = rhashtable_lookup_insert_fast(&scx_sched_hash,
>  					&sch->hash_node, scx_sched_hash_params);
>  			if (ret) {
> -				scx_error(sch, "failed to insert into scx_sched_hash (%d)", ret);
> -				return ret;
> +				err_msg = "failed to insert into scx_sched_hash";
> +				break;
>  			}
>  
>  			list_add_tail(&sch->sibling, &parent->children);
> @@ -5692,6 +5695,15 @@ static s32 scx_link_sched(struct scx_sch
>  		list_add_tail_rcu(&sch->all, &scx_sched_all);
>  	}
>  
> +	/*
> +	 * scx_error() takes scx_sched_lock via scx_claim_exit(), so it must run after
> +	 * the guard above is released.
> +	 */
> +	if (ret) {
> +		scx_error(sch, "%s (%d)", err_msg, ret);
> +		return ret;
> +	}
> +
>  	refresh_watchdog();
>  	return 0;
>  }

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

* Re: [PATCH sched_ext/for-7.1-fixes] sched_ext: Move scx_error() out of scx_link_sched()'s lock region
  2026-05-06 18:02 [PATCH sched_ext/for-7.1-fixes] sched_ext: Move scx_error() out of scx_link_sched()'s lock region Tejun Heo
  2026-05-07  4:31 ` Andrea Righi
@ 2026-05-07 21:12 ` Tejun Heo
  1 sibling, 0 replies; 3+ messages in thread
From: Tejun Heo @ 2026-05-07 21:12 UTC (permalink / raw)
  To: Andrea Righi
  Cc: David Vernet, Changwoo Min, sched-ext, linux-kernel,
	Emil Tsalapatis

Applied to sched_ext/for-7.1-fixes.

Thanks.

--
tejun

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

end of thread, other threads:[~2026-05-07 21:12 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-06 18:02 [PATCH sched_ext/for-7.1-fixes] sched_ext: Move scx_error() out of scx_link_sched()'s lock region Tejun Heo
2026-05-07  4:31 ` Andrea Righi
2026-05-07 21:12 ` Tejun Heo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox