Message-ID: <20220616142656.4b1acc4a@jacob-builder>
Date:   Thu, 16 Jun 2022 14:26:56 -0700
From:   Jacob Pan <jacob.jun.pan@...ux.intel.com>
To:     Peter Zijlstra <peterz@...radead.org>
Cc:     rth@...ddle.net, ink@...assic.park.msu.ru, mattst88@...il.com,
        vgupta@...nel.org, linux@...linux.org.uk,
        ulli.kroll@...glemail.com, linus.walleij@...aro.org,
        shawnguo@...nel.org, Sascha Hauer <s.hauer@...gutronix.de>,
        kernel@...gutronix.de, festevam@...il.com, linux-imx@....com,
        tony@...mide.com, khilman@...nel.org, catalin.marinas@....com,
        will@...nel.org, guoren@...nel.org, bcain@...cinc.com,
        chenhuacai@...nel.org, kernel@...0n.name, geert@...ux-m68k.org,
        sammy@...my.net, monstr@...str.eu, tsbogend@...ha.franken.de,
        dinguyen@...nel.org, jonas@...thpole.se,
        stefan.kristiansson@...nalahti.fi, shorne@...il.com,
        James.Bottomley@...senpartnership.com, deller@....de,
        mpe@...erman.id.au, benh@...nel.crashing.org, paulus@...ba.org,
        paul.walmsley@...ive.com, palmer@...belt.com,
        aou@...s.berkeley.edu, hca@...ux.ibm.com, gor@...ux.ibm.com,
        agordeev@...ux.ibm.com, borntraeger@...ux.ibm.com,
        svens@...ux.ibm.com, ysato@...rs.sourceforge.jp, dalias@...c.org,
        davem@...emloft.net, richard@....at,
        anton.ivanov@...bridgegreys.com, johannes@...solutions.net,
        tglx@...utronix.de, mingo@...hat.com, bp@...en8.de,
        dave.hansen@...ux.intel.com, x86@...nel.org, hpa@...or.com,
        acme@...nel.org, mark.rutland@....com,
        alexander.shishkin@...ux.intel.com, jolsa@...nel.org,
        namhyung@...nel.org, jgross@...e.com, srivatsa@...il.mit.edu,
        amakhalov@...are.com, pv-drivers@...are.com,
        boris.ostrovsky@...cle.com, chris@...kel.net, jcmvbkbc@...il.com,
        rafael@...nel.org, lenb@...nel.org, pavel@....cz,
        gregkh@...uxfoundation.org, mturquette@...libre.com,
        sboyd@...nel.org, daniel.lezcano@...aro.org, lpieralisi@...nel.org,
        sudeep.holla@....com, agross@...nel.org,
        bjorn.andersson@...aro.org, anup@...infault.org,
        thierry.reding@...il.com, jonathanh@...dia.com,
        Arnd Bergmann <arnd@...db.de>, yury.norov@...il.com,
        andriy.shevchenko@...ux.intel.com, linux@...musvillemoes.dk,
        rostedt@...dmis.org, pmladek@...e.com, senozhatsky@...omium.org,
        john.ogness@...utronix.de, paulmck@...nel.org, frederic@...nel.org,
        quic_neeraju@...cinc.com, josh@...htriplett.org,
        mathieu.desnoyers@...icios.com, jiangshanlai@...il.com,
        joel@...lfernandes.org, juri.lelli@...hat.com,
        vincent.guittot@...aro.org, dietmar.eggemann@....com,
        bsegall@...gle.com, mgorman@...e.de, bristot@...hat.com,
        vschneid@...hat.com, jpoimboe@...nel.org,
        linux-alpha@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-snps-arc@...ts.infradead.org,
        linux-arm-kernel@...ts.infradead.org, linux-omap@...r.kernel.org,
        linux-csky@...r.kernel.org, linux-hexagon@...r.kernel.org,
        linux-ia64@...r.kernel.org, linux-m68k@...ts.linux-m68k.org,
        linux-mips@...r.kernel.org, openrisc@...ts.librecores.org,
        linux-parisc@...r.kernel.org, linuxppc-dev@...ts.ozlabs.org,
        linux-riscv@...ts.infradead.org, linux-s390@...r.kernel.org,
        linux-sh@...r.kernel.org, sparclinux@...r.kernel.org,
        linux-um@...ts.infradead.org, linux-perf-users@...r.kernel.org,
        virtualization@...ts.linux-foundation.org,
        xen-devel@...ts.xenproject.org, linux-xtensa@...ux-xtensa.org,
        linux-acpi@...r.kernel.org, linux-pm@...r.kernel.org,
        linux-clk@...r.kernel.org, linux-arm-msm@...r.kernel.org,
        linux-tegra@...r.kernel.org, linux-arch@...r.kernel.org,
        rcu@...r.kernel.org, jacob.jun.pan@...ux.intel.com
Subject: Re: [PATCH 04/36] cpuidle,intel_idle: Fix CPUIDLE_FLAG_IRQ_ENABLE

Hi Peter,

On Mon, 13 Jun 2022 10:44:22 +0200, Peter Zijlstra <peterz@...radead.org>
wrote:

> On Thu, Jun 09, 2022 at 04:49:21PM -0700, Jacob Pan wrote:
> > Hi Peter,
> > 
> > On Wed, 08 Jun 2022 16:27:27 +0200, Peter Zijlstra
> > <peterz@...radead.org> wrote:
> >   
> > > Commit c227233ad64c ("intel_idle: enable interrupts before C1 on
> > > Xeons") wrecked intel_idle in two ways:
> > > 
> > >  - must not have tracing in idle functions
> > >  - must return with IRQs disabled
> > > 
> > > Additionally, it added a branch for no good reason.
> > > 
> > > Fixes: c227233ad64c ("intel_idle: enable interrupts before C1 on
> > > Xeons") Signed-off-by: Peter Zijlstra (Intel) <peterz@...radead.org>
> > > ---
> > >  drivers/idle/intel_idle.c |   48
> > > +++++++++++++++++++++++++++++++++++----------- 1 file changed, 37
> > > insertions(+), 11 deletions(-)
> > > 
> > > --- a/drivers/idle/intel_idle.c
> > > +++ b/drivers/idle/intel_idle.c
> > > @@ -129,21 +137,37 @@ static unsigned int mwait_substates __in
> > >   *
> > >   * Must be called under local_irq_disable().
> > >   */  
> > nit: this comment is no long true, right?  
> 
> It still is, all the idle routines are called with interrupts disabled,
> but must also exit with interrupts disabled.
> 
> If the idle method requires interrupts to be enabled, it must be sure to
> disable them again before returning. Given all the RCU/tracing concerns
> it must use raw_local_irq_*() for this though.
Makes sense, it is just little confusing when the immediate caller does
raw_local_irq_enable() which does not cancel out local_irq_disable().

Thanks,

Jacob

Powered by blists - more mailing lists