Messages in this thread | | | Date | Wed, 2 Dec 2020 10:21:16 +0100 | From | Peter Zijlstra <> | Subject | Re: [GIT pull] locking/urgent for v5.10-rc6 |
| |
On Tue, Dec 01, 2020 at 08:18:56PM +0100, Heiko Carstens wrote:
> Is there a reason why this should be considered broken?
AFAICT this is good.
> diff --git a/arch/s390/kernel/entry.S b/arch/s390/kernel/entry.S > index 26bb0603c5a1..92beb1444644 100644 > --- a/arch/s390/kernel/entry.S > +++ b/arch/s390/kernel/entry.S > @@ -763,12 +763,7 @@ ENTRY(io_int_handler) > xc __PT_FLAGS(8,%r11),__PT_FLAGS(%r11) > TSTMSK __LC_CPU_FLAGS,_CIF_IGNORE_IRQ > jo .Lio_restore > -#if IS_ENABLED(CONFIG_TRACE_IRQFLAGS) > - tmhh %r8,0x300 > - jz 1f > TRACE_IRQS_OFF > -1: > -#endif > xc __SF_BACKCHAIN(8,%r15),__SF_BACKCHAIN(%r15) > .Lio_loop: > lgr %r2,%r11 # pass pointer to pt_regs > @@ -791,12 +786,7 @@ ENTRY(io_int_handler) > TSTMSK __LC_CPU_FLAGS,_CIF_WORK > jnz .Lio_work > .Lio_restore: > -#if IS_ENABLED(CONFIG_TRACE_IRQFLAGS) > - tm __PT_PSW(%r11),3 > - jno 0f > TRACE_IRQS_ON > -0: > -#endif > mvc __LC_RETURN_PSW(16),__PT_PSW(%r11) > tm __PT_PSW+1(%r11),0x01 # returning to user ? > jno .Lio_exit_kernel > @@ -976,12 +966,7 @@ ENTRY(ext_int_handler) > xc __PT_FLAGS(8,%r11),__PT_FLAGS(%r11) > TSTMSK __LC_CPU_FLAGS,_CIF_IGNORE_IRQ > jo .Lio_restore > -#if IS_ENABLED(CONFIG_TRACE_IRQFLAGS) > - tmhh %r8,0x300 > - jz 1f > TRACE_IRQS_OFF > -1: > -#endif > xc __SF_BACKCHAIN(8,%r15),__SF_BACKCHAIN(%r15) > lgr %r2,%r11 # pass pointer to pt_regs > lghi %r3,EXT_INTERRUPT
OK, so with a little help from s390/PoO and Sven, the code removed skips the TRACE_IRQS_OFF when IRQs were enabled in the old PSW (the previous context).
That sounds entirely the right thing. Irrespective of what the previous IRQ state was, the current state is off.
> diff --git a/arch/s390/kernel/idle.c b/arch/s390/kernel/idle.c > index 2b85096964f8..5bd8c1044d09 100644 > --- a/arch/s390/kernel/idle.c > +++ b/arch/s390/kernel/idle.c > @@ -123,7 +123,6 @@ void arch_cpu_idle_enter(void) > void arch_cpu_idle(void) > { > enabled_wait(); > - raw_local_irq_enable(); > }
Currently arch_cpu_idle() is defined as to return with IRQs enabled, however, the very first thing we do when we return is raw_local_irq_disable(), so this change is harmless.
It is also the direction I've been arguing for elsewhere in this thread. So I'm certainly not complaining.
| |