Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.35 as permitted sender) client-ip=23.128.96.35;
Date:   Wed, 15 Nov 2023 10:57:44 -0500
From:   Frederic Weisbecker <frederic@kernel.org>
To:     Peter Zijlstra <peterz@infradead.org>
Cc:     LKML <linux-kernel@vger.kernel.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
        Dave Hansen <dave.hansen@linux.intel.com>, x86@kernel.org,
        "Rafael J . Wysocki" <rafael@kernel.org>
Subject: Re: [PATCH 2/4] x86: Fix CPUIDLE_FLAG_IRQ_ENABLE leaking timer
 reprogram
Message-ID: <ZVTqeKYHdCZdVNDU@localhost.localdomain>
References: <20231115151325.6262-1-frederic@kernel.org>
 <20231115151325.6262-3-frederic@kernel.org>
 <20231115155232.GD8262@noisy.programming.kicks-ass.net>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20231115155232.GD8262@noisy.programming.kicks-ass.net>
Precedence: bulk

Le Wed, Nov 15, 2023 at 04:52:32PM +0100, Peter Zijlstra a ?crit :
> On Wed, Nov 15, 2023 at 10:13:23AM -0500, Frederic Weisbecker wrote:
> > From: Peter Zijlstra <peterz@infradead.org>
> > 
> > intel_idle_irq() re-enables IRQs very early. As a result, an interrupt
> > may fire before mwait() is eventually called. If such an interrupt queues
> > a timer, it may go unnoticed until mwait returns and the idle loop
> > handles the tick re-evaluation. And monitoring TIF_NEED_RESCHED doesn't
> > help because a local timer enqueue doesn't set that flag.
> > 
> > The issue is mitigated by the fact that this idle handler is only invoked
> > for shallow C-states when, presumably, the next tick is supposed to be
> > close enough. There may still be rare cases though when the next tick
> > is far away and the selected C-state is shallow, resulting in a timer
> > getting ignored for a while.
> > 
> > Fix this with using sti_mwait() whose IRQ-reenablement only triggers
> > upon calling mwait(), dealing with the race while keeping the interrupt
> > latency within acceptable bounds.
> > 
> > Fixes: c227233ad64c (intel_idle: enable interrupts before C1 on Xeons)
> > Not-yet-signed-off-by: Peter Zijlstra <peterz@infradead.org>
> 
> Feel free to change to normal SOB, I'm assuming it actually compiles and
> works by now :-)

Not sure, I might have tested it at some point ;-)

Thanks!