Re: [PATCH] sparc64: Fix irqtrace warnings on Ultra-S

From: Guenter Roeck <linux@roeck-us.net>
To: peterz@infradead.org
Cc: linux-kernel@vger.kernel.org, mingo@kernel.org, will@kernel.org,
	npiggin@gmail.com, elver@google.com, jgross@suse.com,
	paulmck@kernel.org, rostedt@goodmis.org, rjw@rjwysocki.net,
	joel@joelfernandes.org, svens@linux.ibm.com, tglx@linutronix.de,
	davem@davemloft.net
Subject: Re: [PATCH] sparc64: Fix irqtrace warnings on Ultra-S
Date: Tue, 8 Sep 2020 09:12:10 -0700	[thread overview]
Message-ID: <20200908161210.GA167915@roeck-us.net> (raw)

On Tue, Sep 08, 2020 at 05:41:57PM +0200, peterz@infradead.org wrote:
> On Tue, Sep 08, 2020 at 07:40:23AM -0700, Guenter Roeck wrote:
> > qemu-system-sparc64 -M sun4u -cpu "TI UltraSparc IIi" -m 512 \
> >     -initrd rootfs.cpio \
> >     -kernel arch/sparc/boot/image -no-reboot \
> >     -append "panic=-1 slub_debug=FZPUA rdinit=/sbin/init console=ttyS0" \
> >     -nographic -monitor none
> 
> Thanks I got it. Also enabling DEBUG_LOCKDEP helps (-:
> 
> ---
> Subject: sparc64: Fix irqtrace warnings on Ultra-S
> 
> Recent changes in Lockdep's IRQTRACE broke Ultra-S.
> 
> In order avoid redundant IRQ state changes, local_irq_restore() lost the
> ability to trace a disable. Change the code to use local_irq_save() to
> disable IRQs and then use arch_local_irq_restore() to further disable
> NMIs.
> 
> This result in slightly suboptimal code, but given this code uses a
> global spinlock, performance cannot be its primary purpose.
> 
> Fixes: 044d0d6de9f5 ("lockdep: Only trace IRQ edges")
> Reported-by: Guenter Roeck <linux@roeck-us.net>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>

Tested-by: Guenter Roeck <linux@roeck-us.net>

> ---
> A possible alternative would be:
> 
> 	local_save_flags(flags);
> 	arch_local_irq_restore((unsigned long)PIL_NMI);
> 	if (IS_ENABLED(CONFIG_TRACE_IRQFLAGS))
> 		trace_hardirqs_off();
> 
> which generates optimal code, but is more verbose.
> 
>  arch/sparc/prom/p1275.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/sparc/prom/p1275.c b/arch/sparc/prom/p1275.c
> index 889aa602f8d8..e22233fcf741 100644
> --- a/arch/sparc/prom/p1275.c
> +++ b/arch/sparc/prom/p1275.c
> @@ -37,16 +37,15 @@ void p1275_cmd_direct(unsigned long *args)
>  {
>  	unsigned long flags;
>  
> -	local_save_flags(flags);
> -	local_irq_restore((unsigned long)PIL_NMI);
> +	local_irq_save(flags);
> +	arch_local_irq_restore((unsigned long)PIL_NMI);
>  	raw_spin_lock(&prom_entry_lock);
>  
>  	prom_world(1);
>  	prom_cif_direct(args);
>  	prom_world(0);
>  
> -	raw_spin_unlock(&prom_entry_lock);
> -	local_irq_restore(flags);
> +	raw_spin_unlock_irqrestore(&prom_entry_lock, flags);
>  }
>  
>  void prom_cif_init(void *cif_handler, void *cif_stack)