[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <Zx_bkK8N8FmPiggg@gpd3>
Date: Mon, 28 Oct 2024 19:44:32 +0100
From: Andrea Righi <arighi@...dia.com>
To: Tejun Heo <tj@...nel.org>
Cc: David Vernet <void@...ifault.com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4] sched_ext: Introduce NUMA awareness to the default
idle selection policy
On Mon, Oct 28, 2024 at 08:11:55AM -1000, Tejun Heo wrote:
> External email: Use caution opening links or attachments
>
>
> Hello,
>
> On Mon, Oct 28, 2024 at 12:33:38PM +0100, Andrea Righi wrote:
> ...
> > +static void update_selcpu_topology(void)
> > {
> > + bool enable_llc = false, enable_numa = false;
> > + struct sched_domain *sd;
> > + const struct cpumask *cpus;
> > + s32 cpu = cpumask_first(cpu_possible_mask);
>
> On x86, the first CPU is guaranteed to be online but there are archs that
> allow the first CPU to go down in which case the topo information might not
> be available. Use cpumask_first(cpu_online_mask) instead?
Ok, I agree, cpu_online_mask is probably more reliable.
>
> ...
> > @@ -3383,6 +3479,10 @@ static void handle_hotplug(struct rq *rq, bool online)
> >
> > atomic_long_inc(&scx_hotplug_seq);
> >
> > + if ((SCX_HAS_OP(cpu_online) || SCX_HAS_OP(cpu_offline)) &&
> > + static_branch_likely(&scx_builtin_idle_enabled))
> > + update_selcpu_topology();
>
> Hmm... this feels a bit too complicated. Just gate it with scx_enabled()?
Ok, update_selcpu_topology() is not that expensive, so we can probably
just check scx_enabled() here.
>
> > if (online && SCX_HAS_OP(cpu_online))
> > SCX_CALL_OP(SCX_KF_UNLOCKED, cpu_online, cpu);
> > else if (!online && SCX_HAS_OP(cpu_offline))
> > @@ -5202,6 +5302,10 @@ static int scx_ops_enable(struct sched_ext_ops *ops, struct bpf_link *link)
> > static_branch_enable_cpuslocked(&scx_has_op[i]);
> >
> > check_hotplug_seq(ops);
> > +#ifdef CONFIG_SMP
> > + if (!ops->update_idle || (ops->flags & SCX_OPS_KEEP_BUILTIN_IDLE))
> > + update_selcpu_topology();
> > +#endif
>
> And always update here?
Ok.
Thanks,
-Andrea
Powered by blists - more mailing lists