All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrea Righi <arighi@nvidia.com>
To: Tejun Heo <tj@kernel.org>
Cc: David Vernet <void@manifault.com>,
	Changwoo Min <changwoo@igalia.com>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/2] sched_ext/idle: Make scx_bpf_select_cpu_and() usable from any context
Date: Mon, 12 May 2025 20:35:58 +0200	[thread overview]
Message-ID: <aCI_jmub1sLdASLV@gpd3> (raw)
In-Reply-To: <aCItuWh-BLp3jC6h@slm.duckdns.org>

Hi Tejun,

On Mon, May 12, 2025 at 07:19:53AM -1000, Tejun Heo wrote:
> Hello,
> 
> On Mon, May 12, 2025 at 05:14:56PM +0200, Andrea Righi wrote:
> >  #ifdef CONFIG_SMP
> > +	/*
> > +	 * If called from an unlocked context, try to acquire
> > +	 * cpus_read_lock() to avoid races with CPU hotplug.
> > +	 */
> > +	if (scx_kf_allowed_if_unlocked())
> > +		if (!cpus_read_trylock())
> > +			return -EBUSY;
> 
> Is this meaningful? The idle CPU selection is already racy against CPU
> hotplugs and we depend on the scheduler core to fix it up afterwards. Even
> if scx_bpf_select_cpu_and() is not racy, it will drop the cpus lock before
> returning and becomes racy again right there. ie. This doesn't add any
> meaningful protection.

I was concerned that accessing llc_span() / llc_weight() from
scx_select_cpu_dfl() might be problematic if a CPU goes offline underneath,
but we are accessing them with rcu_read_lock() held, we probably don't need
cpus_read_lock() protection.

And about the scheduler picking a bad CPU, that can be fixed by the
sched_ext core, so there's no problem for that.

I'll think more about the CPU hotplugging scenario.

Thanks,
-Andrea

> 
> Thanks.
> 
> -- 
> tejun

  reply	other threads:[~2025-05-12 18:36 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-05-12 15:14 [PATCH sched_ext/for-6.16 0/2] sched_ext: Extend usability of scx_bpf_select_cpu_and() Andrea Righi
2025-05-12 15:14 ` [PATCH 1/2] sched_ext: Make scx_kf_allowed_if_unlocked() available outside ext.c Andrea Righi
2025-05-12 15:14 ` [PATCH 2/2] sched_ext/idle: Make scx_bpf_select_cpu_and() usable from any context Andrea Righi
2025-05-12 16:58   ` David Vernet
2025-05-12 18:29     ` Andrea Righi
2025-05-12 19:07       ` Andrea Righi
2025-05-12 20:00         ` Tejun Heo
2025-05-13  5:23           ` Andrea Righi
2025-05-12 17:19   ` Tejun Heo
2025-05-12 18:35     ` Andrea Righi [this message]
2025-05-13  0:46 ` [PATCH sched_ext/for-6.16 0/2] sched_ext: Extend usability of scx_bpf_select_cpu_and() Changwoo Min
2025-05-13  5:48   ` Andrea Righi

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=aCI_jmub1sLdASLV@gpd3 \
    --to=arighi@nvidia.com \
    --cc=changwoo@igalia.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.