Subject: Re: [PATCH v2] powerpc: Add irqtrace support for 32-bit powerpc

From: Benjamin Herrenschmidt <[email-masked]>
Date: Mon, 07 Apr 2008 14:49:51 +1000

I think I found one:

 .../...

> -	mr	r6,r3
>  	rlwinm	r12,r1,0,0,(31-THREAD_SHIFT)	/* current_thread_info() */
>  	/* disable interrupts so current_thread_info()->flags can't change */
>  	LOAD_MSR_KERNEL(r10,MSR_KERNEL)	/* doesn't include MSR_EE */
>  	SYNC
>  	MTMSRD(r10)
> +#ifdef CONFIG_TRACE_IRQFLAGS
> +	stwu	r1,-16(r1)
> +	stw	r3,12(r1)
> +	bl      trace_hardirqs_off
> +	lwz	r3,12(r1)
> +	addi	r1,r1,16
> +	LOAD_MSR_KERNEL(r10,MSR_KERNEL)
> +#endif

Here, r12 is clobbered, though it's used two lines later:

> +	mr	r6,r3
>  	lwz	r9,TI_FLAGS(r12)

Here.

You can probably just move the rlwinm down as you moved the mr.

Note that I've been wondering wether we should attempt to trace all
those IRQ state change internally to the exception code. I've looked at
not doing it, which simplifies things a bit.

Unfortunately, that will make us occasionally trace redundant
enable/disable (which isn't a big problem per-se, just counters).

The idea is that I only kept the trace of disable in transfer_to_handler
and I modified the enable tracing in restore: moved it lower down, and
made it test for _MSR(r1):MSR_EE. I added a trace_irq_off just before
the preempt_schedule_irq() as well.

Anyway, let me know what you think.

Cheers,
Ben.