linux-kernel - Re: [RFC PATCH v3 3/3] softirq: defer softirq processing to ksoftirqd if CPU is busy with RT

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CANDhNCqOCu4vERKmKoFGJ5=UPfyDdgbWJ7iEFN9dPjfXvt760g@mail.gmail.com>
Date:   Mon, 3 Oct 2022 11:43:50 -0700
From:   John Stultz <jstultz@...gle.com>
To:     Qais Yousef <qais.yousef@....com>
Cc:     LKML <linux-kernel@...r.kernel.org>,
        Pavankumar Kondeti <pkondeti@...eaurora.org>,
        John Dias <joaodias@...gle.com>,
        "Connor O'Brien" <connoro@...gle.com>,
        Rick Yiu <rickyiu@...gle.com>, John Kacur <jkacur@...hat.com>,
        Chris Redpath <chris.redpath@....com>,
        Abhijeet Dharmapurikar <adharmap@...cinc.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...hat.com>,
        Juri Lelli <juri.lelli@...hat.com>,
        Vincent Guittot <vincent.guittot@...aro.org>,
        Dietmar Eggemann <dietmar.eggemann@....com>,
        Steven Rostedt <rostedt@...dmis.org>,
        Thomas Gleixner <tglx@...utronix.de>, kernel-team@...roid.com,
        Satya Durga Srinivasu Prabhala <satyap@...eaurora.org>,
        "J . Avila" <elavila@...gle.com>
Subject: Re: [RFC PATCH v3 3/3] softirq: defer softirq processing to ksoftirqd
 if CPU is busy with RT

On Wed, Sep 28, 2022 at 5:56 AM Qais Yousef <qais.yousef@....com> wrote:
>
> On 09/21/22 01:25, John Stultz wrote:
> > From: Pavankumar Kondeti <pkondeti@...eaurora.org>
> >
> > Defer the softirq processing to ksoftirqd if a RT task is
> > running or queued on the current CPU. This complements the RT
> > task placement algorithm which tries to find a CPU that is not
> > currently busy with softirqs.
> >
> > Currently NET_TX, NET_RX, BLOCK and TASKLET softirqs are only
>
> Should we mention IRQ_POLL?

Ah, yes. Thank you for pointing that out.

> I think TASKLET is debatable as I mentioned in my other email.

Yeah, I've dropped it for now.


> > +#ifdef CONFIG_RT_SOFTIRQ_OPTIMIZATION
> > +/*
> > + * cpupri_check_rt - check if CPU has a RT task
> > + * should be called from rcu-sched read section.
> > + */
> > +bool cpupri_check_rt(void)
> > +{
> > +     int cpu = raw_smp_processor_id();
> > +
> > +     return cpu_rq(cpu)->rd->cpupri.cpu_to_pri[cpu] > CPUPRI_NORMAL;
> > +}
>
> Priorities always mess up with my brain! I always forget which direction to
> look at :D

Yeah, cpu_pri logic in particular (as it also depends on which version
you're looking at - the original version of this patch against an
older kernel had an off by one error that took awhile to find).

> Hmm I was wondering why not do rt_task(current), but if the task is not running
> (which can only indicate there's a DL or a stopper task preempting it), that
> won't work. But I think your code has a similar problem; you'll return true
> even if there's only a DL task running since we set the priority to
> CPUPRI_HIGHER which will cause your condition to return true.
>
> This makes me think if we should enable this optimization for DL tasks too.
> Hmm...
>
> That said, is there a reason why we can't remove this function and just call
> rt_task(current) directly in softirq_deferred_for_rt()?
>

I had thought similarly, but had hesitated to switch in case there was
some subtlety I wasn't seeing.
But I think you've persuaded me to simplify this.

Thanks again for the feedback and suggestions!
-john