Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Wed, 2 Dec 2020 08:54:27 +0100
From:   Heiko Carstens <hca@linux.ibm.com>
To:     Peter Zijlstra <peterz@infradead.org>
Cc:     Sven Schnelle <svens@linux.ibm.com>,
        Linus Torvalds <torvalds@linux-foundation.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        "Paul E. McKenney" <paulmck@kernel.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        the arch/x86 maintainers <x86@kernel.org>
Subject: Re: [GIT pull] locking/urgent for v5.10-rc6
Message-ID: <20201202075427.GA5838@osiris>
References: <160665707945.2808.5384034634184489471.tglx@nanos>
 <160665708065.2808.15317906761841446715.tglx@nanos>
 <CAHk-=wi3o-wwFVbAXb7YZZViDBsZ_yMVqyOAEZsx5qcskLsOcg@mail.gmail.com>
 <20201130075651.GJ2414@hirez.programming.kicks-ass.net>
 <yt9dh7p78d8l.fsf@linux.ibm.com>
 <yt9dpn3v3u1m.fsf@linux.ibm.com>
 <20201130125211.GN2414@hirez.programming.kicks-ass.net>
 <20201130130315.GJ3092@hirez.programming.kicks-ass.net>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20201130130315.GJ3092@hirez.programming.kicks-ass.net>
Precedence: bulk

> > But but but...
> > 
> >   do_idle()			# IRQs on
> >     local_irq_disable();	# IRQs off
> >     defaul_idle_call()	# IRQs off
> 	lockdep_hardirqs_on();	# IRQs off, but lockdep things they're on
> >       arch_cpu_idle()		# IRQs off
> >         enabled_wait()	# IRQs off
> > 	  raw_local_save()	# still off
> > 	  psw_idle()		# very much off
> > 	    ext_int_handler	# get an interrupt ?!?!
> 	      rcu_irq_enter()	# lockdep thinks IRQs are on <- FAIL
> 
> I can't much read s390 assembler, but ext_int_handler() has a
> TRACE_IRQS_OFF, which would be sufficient to re-align the lockdep state
> with the actual state, but there's some condition before it, what's that
> test and is that right?

After digging a bit into our asm code: no, it is not right, and only
for psw_idle() it is wrong.

What happens with the current code:

- default_idle_call() calls lockdep_hardirqs_on() before calling into
  arch_cpu_idle()

- our arch_cpu_idle() calls psw_idle() which enables irqs. the irq
  handler will call/use the SWITCH_ASYNC macro which clears the
  interrupt enabled bits in the old program status word (_only_ for
  psw_idle)

- this again causes the interrupt handler to _not_ call TRACE_IRQS_OFF
  and therefore lockdep thinks interrupts are enabled within the
  interrupt handler

So I guess my patch which I sent yesterday evening should fix all that
mess - plus an explicit trace_hardirqs_off() call in our udelay
implementation is required now.

I'll send a proper patch later.