All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrea Righi <arighi@nvidia.com>
To: K Prateek Nayak <kprateek.nayak@amd.com>
Cc: Tejun Heo <tj@kernel.org>, David Vernet <void@manifault.com>,
	Changwoo Min <changwoo@igalia.com>,
	Ingo Molnar <mingo@redhat.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Juri Lelli <juri.lelli@redhat.com>,
	Vincent Guittot <vincent.guittot@linaro.org>,
	Dietmar Eggemann <dietmar.eggemann@arm.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	Ben Segall <bsegall@google.com>, Mel Gorman <mgorman@suse.de>,
	Valentin Schneider <vschneid@redhat.com>,
	Vladimir Vdovin <deliran@verdict.gg>,
	Gregory Price <gourry@gourry.net>,
	David Dai <david.dai@linux.dev>, Emil Tsalapatis <etsal@meta.com>,
	Christian Loehle <christian.loehle@arm.com>,
	Balbir Singh <balbirs@nvidia.com>,
	Lee Trager <ltrager@nvidia.com>,
	sched-ext@lists.linux.dev, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 5/6] sched_ext: Add a kfunc to read a task's preferred NUMA node
Date: Tue, 27 Oct 2026 07:57:36 +0100	[thread overview]
Message-ID: <auBLYFDJ88gTxtLM@gpd4> (raw)
In-Reply-To: <cf3e5585-128e-4ee3-9352-afbddbfd50c1@amd.com>

Hi Prateek,

On Tue, Oct 27, 2026 at 11:42:22AM +0530, K Prateek Nayak wrote:
> Hello Andrea,
> 
> On 10/27/2026 9:43 AM, Andrea Righi wrote:
> > +__bpf_kfunc s32 scx_bpf_task_numa_nid(const struct task_struct *p)
> > +{
> > +#ifdef CONFIG_NUMA_BALANCING
> > +	if (task_on_scx(p)) {
> > +		struct scx_sched *sch;
> > +
> > +		guard(rcu)();
> > +		sch = scx_task_sched_rcu(p);
> > +		if (!sch || !SCX_HAS_OP(sch, set_numa_node))
> > +			return NUMA_NO_NODE;
> > +	}
> > +
> > +	return READ_ONCE(p->numa_preferred_nid);
> 
> Would the ext scheduler need the raw numa_preferred_nid when
> sched_numa_balancing is disabled?
> 
> It may just be pointing to some stale value if NUMA balancing
> was disabled at runtime right?

Right, if NUMA balancing is disabled at runtime the value stays at the
last preference.

This is consistent with fair: disabling NUMA balancing at runtime only
flips the sched_numa_balancing static key, p->numa_preferred_nid is
never reset, fair stops acting on it in most places (e.g.,
migrate_degrades_locality()), but not everywhere, find_idlest_group()
still uses it in the slow wakeup path. So a BPF scheduler sees the same
value that fair keeps and can decide whether to use it or not.

A more accurate approach could be to notify the sched classes when
sched_numa_balancing changes (e.g., resetting the preferred node of all
tasks when it's disabled and recomputing it when it's re-enabled), but
that would be a significant rework, involving fair as well, and it's
outside the scope of this series. I'd rather start by making ext
consistent with fair, and then rework both in a more accurate and
complete way as a follow-up, if we want.

> 
> > +#else
> > +	return NUMA_NO_NODE;
> > +#endif
> > +}
> > +
> >  /**
> >   * scx_bpf_locked_rq - Return the rq currently locked by SCX
> >   * @aux: implicit BPF argument to access bpf_prog_aux hidden from BPF progs
> > @@ -11836,6 +11877,7 @@ BTF_ID_FLAGS(func, scx_bpf_put_cpumask, KF_RELEASE)
> >  BTF_ID_FLAGS(func, scx_bpf_task_running, KF_RCU)
> 
> On a tangential note, scx_bpf_task_running() just does a
> rcu_access_pointer() (resolves to a READ_ONCE()) and doesn't take a
> rcu_read_lock() so does it really need KF_RCU?

Ah yes, that's tricky. KF_RCU doesn't mean that the kfunc takes
rcu_read_lock(), it's a verifier flag that allows the kfunc to accept
RCU-protected pointers as arguments, in addition to trusted (refcounted)
ones.

scx_bpf_task_running() still dereferences p (task_rq(p)->donor), so p
needs to be valid for the duration of the call, and KF_RCU is the
weakest guarantee that the verifier can enforce for that. Same for
scx_bpf_task_numa_nid().

> >  BTF_ID_FLAGS(func, scx_bpf_task_cpu, KF_RCU)
> >  BTF_ID_FLAGS(func, scx_bpf_task_cid, KF_RCU)
> > +BTF_ID_FLAGS(func, scx_bpf_task_numa_nid, KF_RCU)
> >  BTF_ID_FLAGS(func, scx_bpf_locked_rq, KF_IMPLICIT_ARGS | KF_RET_NULL)
> >  BTF_ID_FLAGS(func, scx_bpf_cpu_curr, KF_IMPLICIT_ARGS | KF_RET_NULL | KF_RCU_PROTECTED)
> >  BTF_ID_FLAGS(func, scx_bpf_cid_curr, KF_IMPLICIT_ARGS | KF_RET_NULL | KF_RCU_PROTECTED)

Thanks for taking a look at this series!
-Andrea

  reply	other threads:[~2026-10-27  6:57 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-27  4:13 [PATCHSET v2 sched_ext/for-7.5] sched_ext: Add NUMA balancing support Andrea Righi
2026-10-27  4:13 ` [PATCH 1/6] sched/numa: Let other scheduling classes drive NUMA scanning Andrea Righi
2026-10-27  4:13 ` [PATCH 2/6] sched/numa: Leave the placement of a BPF-scheduled task to its scheduler Andrea Righi
2026-10-27  5:45   ` K Prateek Nayak
2026-10-27  6:43     ` Andrea Righi
2026-10-27  9:00       ` K Prateek Nayak
2026-10-27  9:30         ` Andrea Righi
2026-10-27 10:43   ` Peter Zijlstra
2026-10-27 11:09     ` Andrea Righi
2026-10-27 11:52       ` Peter Zijlstra
2026-10-27  4:13 ` [PATCH 3/6] sched/numa: Notify the scheduling class of preferred node changes Andrea Righi
2026-10-27  5:30   ` K Prateek Nayak
2026-10-27  6:34     ` Andrea Righi
2026-10-27  8:02       ` K Prateek Nayak
2026-10-27  4:13 ` [PATCH 4/6] sched_ext: Report preferred NUMA nodes through ops.set_numa_node() Andrea Righi
2026-10-27 10:49   ` Peter Zijlstra
2026-10-27 11:14     ` Andrea Righi
2026-10-27  4:13 ` [PATCH 5/6] sched_ext: Add a kfunc to read a task's preferred NUMA node Andrea Righi
2026-10-27  6:12   ` K Prateek Nayak
2026-10-27  6:57     ` Andrea Righi [this message]
2026-10-27 10:50       ` Peter Zijlstra
2026-10-27 11:21         ` Andrea Righi
2026-10-27 11:54           ` Peter Zijlstra
2026-10-27  4:13 ` [PATCH 6/6] selftests/sched_ext: Test preferred NUMA node notifications Andrea Righi
2026-10-27 10:52 ` [PATCHSET v2 sched_ext/for-7.5] sched_ext: Add NUMA balancing support Peter Zijlstra
2026-10-27 11:38   ` Andrea Righi
2026-10-27 11:55     ` Peter Zijlstra

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=auBLYFDJ88gTxtLM@gpd4 \
    --to=arighi@nvidia.com \
    --cc=balbirs@nvidia.com \
    --cc=bsegall@google.com \
    --cc=changwoo@igalia.com \
    --cc=christian.loehle@arm.com \
    --cc=david.dai@linux.dev \
    --cc=deliran@verdict.gg \
    --cc=dietmar.eggemann@arm.com \
    --cc=etsal@meta.com \
    --cc=gourry@gourry.net \
    --cc=juri.lelli@redhat.com \
    --cc=kprateek.nayak@amd.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=ltrager@nvidia.com \
    --cc=mgorman@suse.de \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=rostedt@goodmis.org \
    --cc=sched-ext@lists.linux.dev \
    --cc=tj@kernel.org \
    --cc=vincent.guittot@linaro.org \
    --cc=void@manifault.com \
    --cc=vschneid@redhat.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.