All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrea Righi <arighi@nvidia.com>
To: Tejun Heo <tj@kernel.org>, David Vernet <void@manifault.com>,
	Changwoo Min <changwoo@igalia.com>
Cc: Joel Fernandes <joelagnelf@nvidia.com>, linux-kernel@vger.kernel.org
Subject: Re: [PATCH sched_ext/for-6.15] sched_ext: initialize built-in idle state before ops.init()
Date: Tue, 25 Mar 2025 10:04:55 +0100	[thread overview]
Message-ID: <Z-Jxt3n6clbABIr9@gpd3> (raw)
In-Reply-To: <20250324085753.27112-1-arighi@nvidia.com>

On Mon, Mar 24, 2025 at 09:57:53AM +0100, Andrea Righi wrote:
...
> diff --git a/kernel/sched/ext.c b/kernel/sched/ext.c
> index 06561d6717c9a..1ba02755ae8ad 100644
> --- a/kernel/sched/ext.c
> +++ b/kernel/sched/ext.c
> @@ -5361,6 +5361,8 @@ static int scx_ops_enable(struct sched_ext_ops *ops, struct bpf_link *link)
>  	 */
>  	cpus_read_lock();
>  
> +	scx_idle_enable(ops);
> +

Actually, I just noticed a problem: if we call scx_idle_enable() under
cpus_read_lock() we may re-acquire cpu_hotplug_lock because of the
static_branch_enable/disable() calls, that are trying to re-acquire the
lock, which is not correct.

So, we either need to use static_branch_enable/disable_cpuslocked() or
place scx_idle_enable() outside of cpus_read_lock().

I just notice this from a lockdep splat on an arm64 machine (not sure why
lockdep was happy when I was testing this in vng):

[   65.974439] WARNING: possible recursive locking detected
...
[   65.983540] --------------------------------------------
[   65.989039] scx_bpfland/3883 is trying to acquire lock:
[   65.994447] ffffb80a490991d8 (cpu_hotplug_lock){++++}-{0:0}, at: cpus_read_lock+0x18/0x30
[   66.002941]
               but task is already holding lock:
[   66.008978] ffffb80a490991d8 (cpu_hotplug_lock){++++}-{0:0}, at: cpus_read_lock+0x18/0x30
[   66.017455]
               other info that might help us debug this:
[   66.024212]  Possible unsafe locking scenario:

[   66.030338]        CPU0
[   66.032855]        ----
[   66.035372]   lock(cpu_hotplug_lock);
[   66.039154]   lock(cpu_hotplug_lock);
[   66.042935]
                *** DEADLOCK ***

Anyway, please ignore this patch, I'll send a new one soon.

-Andrea

      parent reply	other threads:[~2025-03-25  9:05 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-03-24  8:57 [PATCH sched_ext/for-6.15] sched_ext: initialize built-in idle state before ops.init() Andrea Righi
2025-03-24 12:48 ` Changwoo Min
2025-03-25  9:04 ` Andrea Righi [this message]

Reply instructions:

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

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

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

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

  git send-email \
    --in-reply-to=Z-Jxt3n6clbABIr9@gpd3 \
    --to=arighi@nvidia.com \
    --cc=changwoo@igalia.com \
    --cc=joelagnelf@nvidia.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=tj@kernel.org \
    --cc=void@manifault.com \
    /path/to/YOUR_REPLY

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

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