Re: [RFC PATCH v3 2/3] sched: Avoid placing RT threads on cores handling long softirqs
From: John Stultz
Date: Mon Oct 03 2022 - 13:47:36 EST
On Mon, Oct 3, 2022 at 9:55 AM John Stultz <jstultz@xxxxxxxxxx> wrote:
> On Wed, Sep 28, 2022 at 5:55 AM Qais Yousef <qais.yousef@xxxxxxx> wrote:
> > On 09/21/22 01:25, John Stultz wrote:
> > > @@ -1641,9 +1683,10 @@ select_task_rq_rt(struct task_struct *p, int cpu, int flags)
> > > * requirement of the task - which is only important on heterogeneous
> > > * systems like big.LITTLE.
> > > */
> > > - test = curr &&
> > > - unlikely(rt_task(curr)) &&
> > > - (curr->nr_cpus_allowed < 2 || curr->prio <= p->prio);
> > > + may_not_preempt = !task_may_preempt(curr, cpu);
> > > + test = (curr && (may_not_preempt ||
> > > + (unlikely(rt_task(curr)) &&
> > > + (curr->nr_cpus_allowed < 2 || curr->prio <= p->prio))));
> >
> > I think this is unnecesary if you create new rt_task_fits_cpu() and ...
> >
> > >
> > > if (test || !rt_task_fits_capacity(p, cpu)) {
> >
> > ... replace the call to rt_task_fits_capacity() with the new
> > rt_task_fits_cpu()?
>
>
> But is that really the same logic? Above we're doing:
> if ((!task_may_preempt(curr, cpu)|| <other stuff >) ||
> !rt_task_fits_capacity(p, cpu))
>
> And you're suggestion switching it to
> if (<other stuff> || !rt_task_fits_cpu(p, cpu))
> which would expand to:
> if( <other stuff > || !(task_may_preempt(p, cpu) &&
> rt_task_fits_capacity(p, cpu)))
>
> I worry we would be skipping the part where we compare against curr.
Ignore this bit, I've not finished my coffee.
I was mixing up an earlier version of the patch where the task passed
in to task_may_preempt() was compared with the ksoftirqd (which didn't
seem right), and I've since switched it to comparing curr on the cpu
with the ksoftirqd, making the task passed in unused.
I'm reworking this to be less confusing (renaming this to
cpu_busy_with_softirqs()), and will try to take your larger suggestion
here.
thanks
-john