Declaring unrecoverable_exception() as __noreturn ?

Declaring unrecoverable_exception() as __noreturn ?

[Christophe Leroy]

As far as I can see, almost all callers of unrecoverable_exception() expect it to never return.

Can we mark it __noreturn ?

Below is interrupt_exit_kernel_prepare() with then without unrecoverable_exception() declared as 
__noreturn. (CONFIG_PREEMPT_NONE, and with the BUG_ON() removed)

With the __noreturn added, we get no stack frame on the likely path

000003a8 <interrupt_exit_kernel_prepare>:
  3a8:	81 43 00 84 	lwz     r10,132(r3)
  3ac:	71 4a 00 02 	andi.   r10,r10,2
  3b0:	41 82 00 30 	beq     3e0 <interrupt_exit_kernel_prepare+0x38>
  3b4:	80 62 00 70 	lwz     r3,112(r2)
  3b8:	74 63 00 01 	andis.  r3,r3,1
  3bc:	40 82 00 34 	bne     3f0 <interrupt_exit_kernel_prepare+0x48>
  3c0:	7d 40 00 a6 	mfmsr   r10
  3c4:	55 4a 04 5e 	rlwinm  r10,r10,0,17,15
  3c8:	7d 40 01 24 	mtmsr   r10
  3cc:	7d 20 00 a6 	mfmsr   r9
  3d0:	55 29 07 fa 	rlwinm  r9,r9,0,31,29
  3d4:	55 29 04 5e 	rlwinm  r9,r9,0,17,15
  3d8:	7d 20 01 24 	mtmsr   r9
  3dc:	4e 80 00 20 	blr
  3e0:	94 21 ff f0 	stwu    r1,-16(r1)
  3e4:	7c 08 02 a6 	mflr    r0
  3e8:	90 01 00 14 	stw     r0,20(r1)
  3ec:	48 00 00 01 	bl      3ec <interrupt_exit_kernel_prepare+0x44>
			3ec: R_PPC_REL24	unrecoverable_exception
  3f0:	38 e2 00 70 	addi    r7,r2,112
  3f4:	3d 00 00 01 	lis     r8,1
  3f8:	7c c0 38 28 	lwarx   r6,0,r7
  3fc:	7c c6 40 78 	andc    r6,r6,r8
  400:	7c c0 39 2d 	stwcx.  r6,0,r7
  404:	40 a2 ff f4 	bne     3f8 <interrupt_exit_kernel_prepare+0x50>
  408:	38 60 00 01 	li      r3,1
  40c:	4b ff ff b4 	b       3c0 <interrupt_exit_kernel_prepare+0x18>

Without the modification:

000003a8 <interrupt_exit_kernel_prepare>:
  3a8:	94 21 ff f0 	stwu    r1,-16(r1)
  3ac:	93 e1 00 0c 	stw     r31,12(r1)
  3b0:	81 23 00 84 	lwz     r9,132(r3)
  3b4:	71 29 00 02 	andi.   r9,r9,2
  3b8:	41 82 00 38 	beq     3f0 <interrupt_exit_kernel_prepare+0x48>
  3bc:	81 22 00 70 	lwz     r9,112(r2)
  3c0:	75 23 00 01 	andis.  r3,r9,1
  3c4:	40 82 00 4c 	bne     410 <interrupt_exit_kernel_prepare+0x68>
  3c8:	7d 20 00 a6 	mfmsr   r9
  3cc:	55 29 04 5e 	rlwinm  r9,r9,0,17,15
  3d0:	7d 20 01 24 	mtmsr   r9
  3d4:	7d 20 00 a6 	mfmsr   r9
  3d8:	55 29 07 fa 	rlwinm  r9,r9,0,31,29
  3dc:	55 29 04 5e 	rlwinm  r9,r9,0,17,15
  3e0:	7d 20 01 24 	mtmsr   r9
  3e4:	83 e1 00 0c 	lwz     r31,12(r1)
  3e8:	38 21 00 10 	addi    r1,r1,16
  3ec:	4e 80 00 20 	blr
  3f0:	7c 08 02 a6 	mflr    r0
  3f4:	90 01 00 14 	stw     r0,20(r1)
  3f8:	48 00 00 01 	bl      3f8 <interrupt_exit_kernel_prepare+0x50>
			3f8: R_PPC_REL24	unrecoverable_exception
  3fc:	81 22 00 70 	lwz     r9,112(r2)
  400:	80 01 00 14 	lwz     r0,20(r1)
  404:	75 23 00 01 	andis.  r3,r9,1
  408:	7c 08 03 a6 	mtlr    r0
  40c:	41 82 ff bc 	beq     3c8 <interrupt_exit_kernel_prepare+0x20>
  410:	39 02 00 70 	addi    r8,r2,112
  414:	3d 20 00 01 	lis     r9,1
  418:	7c e0 40 28 	lwarx   r7,0,r8
  41c:	7c e7 48 78 	andc    r7,r7,r9
  420:	7c e0 41 2d 	stwcx.  r7,0,r8
  424:	40 a2 ff f4 	bne     418 <interrupt_exit_kernel_prepare+0x70>
  428:	38 60 00 01 	li      r3,1
  42c:	7d 20 00 a6 	mfmsr   r9
  430:	55 29 04 5e 	rlwinm  r9,r9,0,17,15
  434:	7d 20 01 24 	mtmsr   r9
  438:	7d 20 00 a6 	mfmsr   r9
  43c:	55 29 07 fa 	rlwinm  r9,r9,0,31,29
  440:	55 29 04 5e 	rlwinm  r9,r9,0,17,15
  444:	7d 20 01 24 	mtmsr   r9
  448:	83 e1 00 0c 	lwz     r31,12(r1)
  44c:	38 21 00 10 	addi    r1,r1,16
  450:	4e 80 00 20 	blr

Re: Declaring unrecoverable_exception() as __noreturn ?

[Nicholas Piggin]

Excerpts from Christophe Leroy's message of February 11, 2021 2:44 am:
> As far as I can see, almost all callers of unrecoverable_exception() expect it to never return.
> 
> Can we mark it __noreturn ?

I don't see why not, do_exit is noreturn. We could make die() noreturn 
as well.

> 
> Below is interrupt_exit_kernel_prepare() with then without unrecoverable_exception() declared as 
> __noreturn. (CONFIG_PREEMPT_NONE, and with the BUG_ON() removed)
> 
> With the __noreturn added, we get no stack frame on the likely path

Nice!

Thanks,
Nick

> 
> 000003a8 <interrupt_exit_kernel_prepare>:
>   3a8:	81 43 00 84 	lwz     r10,132(r3)
>   3ac:	71 4a 00 02 	andi.   r10,r10,2
>   3b0:	41 82 00 30 	beq     3e0 <interrupt_exit_kernel_prepare+0x38>
>   3b4:	80 62 00 70 	lwz     r3,112(r2)
>   3b8:	74 63 00 01 	andis.  r3,r3,1
>   3bc:	40 82 00 34 	bne     3f0 <interrupt_exit_kernel_prepare+0x48>
>   3c0:	7d 40 00 a6 	mfmsr   r10
>   3c4:	55 4a 04 5e 	rlwinm  r10,r10,0,17,15
>   3c8:	7d 40 01 24 	mtmsr   r10
>   3cc:	7d 20 00 a6 	mfmsr   r9
>   3d0:	55 29 07 fa 	rlwinm  r9,r9,0,31,29
>   3d4:	55 29 04 5e 	rlwinm  r9,r9,0,17,15
>   3d8:	7d 20 01 24 	mtmsr   r9
>   3dc:	4e 80 00 20 	blr
>   3e0:	94 21 ff f0 	stwu    r1,-16(r1)
>   3e4:	7c 08 02 a6 	mflr    r0
>   3e8:	90 01 00 14 	stw     r0,20(r1)
>   3ec:	48 00 00 01 	bl      3ec <interrupt_exit_kernel_prepare+0x44>
> 			3ec: R_PPC_REL24	unrecoverable_exception
>   3f0:	38 e2 00 70 	addi    r7,r2,112
>   3f4:	3d 00 00 01 	lis     r8,1
>   3f8:	7c c0 38 28 	lwarx   r6,0,r7
>   3fc:	7c c6 40 78 	andc    r6,r6,r8
>   400:	7c c0 39 2d 	stwcx.  r6,0,r7
>   404:	40 a2 ff f4 	bne     3f8 <interrupt_exit_kernel_prepare+0x50>
>   408:	38 60 00 01 	li      r3,1
>   40c:	4b ff ff b4 	b       3c0 <interrupt_exit_kernel_prepare+0x18>
> 
> Without the modification:
> 
> 000003a8 <interrupt_exit_kernel_prepare>:
>   3a8:	94 21 ff f0 	stwu    r1,-16(r1)
>   3ac:	93 e1 00 0c 	stw     r31,12(r1)
>   3b0:	81 23 00 84 	lwz     r9,132(r3)
>   3b4:	71 29 00 02 	andi.   r9,r9,2
>   3b8:	41 82 00 38 	beq     3f0 <interrupt_exit_kernel_prepare+0x48>
>   3bc:	81 22 00 70 	lwz     r9,112(r2)
>   3c0:	75 23 00 01 	andis.  r3,r9,1
>   3c4:	40 82 00 4c 	bne     410 <interrupt_exit_kernel_prepare+0x68>
>   3c8:	7d 20 00 a6 	mfmsr   r9
>   3cc:	55 29 04 5e 	rlwinm  r9,r9,0,17,15
>   3d0:	7d 20 01 24 	mtmsr   r9
>   3d4:	7d 20 00 a6 	mfmsr   r9
>   3d8:	55 29 07 fa 	rlwinm  r9,r9,0,31,29
>   3dc:	55 29 04 5e 	rlwinm  r9,r9,0,17,15
>   3e0:	7d 20 01 24 	mtmsr   r9
>   3e4:	83 e1 00 0c 	lwz     r31,12(r1)
>   3e8:	38 21 00 10 	addi    r1,r1,16
>   3ec:	4e 80 00 20 	blr
>   3f0:	7c 08 02 a6 	mflr    r0
>   3f4:	90 01 00 14 	stw     r0,20(r1)
>   3f8:	48 00 00 01 	bl      3f8 <interrupt_exit_kernel_prepare+0x50>
> 			3f8: R_PPC_REL24	unrecoverable_exception
>   3fc:	81 22 00 70 	lwz     r9,112(r2)
>   400:	80 01 00 14 	lwz     r0,20(r1)
>   404:	75 23 00 01 	andis.  r3,r9,1
>   408:	7c 08 03 a6 	mtlr    r0
>   40c:	41 82 ff bc 	beq     3c8 <interrupt_exit_kernel_prepare+0x20>
>   410:	39 02 00 70 	addi    r8,r2,112
>   414:	3d 20 00 01 	lis     r9,1
>   418:	7c e0 40 28 	lwarx   r7,0,r8
>   41c:	7c e7 48 78 	andc    r7,r7,r9
>   420:	7c e0 41 2d 	stwcx.  r7,0,r8
>   424:	40 a2 ff f4 	bne     418 <interrupt_exit_kernel_prepare+0x70>
>   428:	38 60 00 01 	li      r3,1
>   42c:	7d 20 00 a6 	mfmsr   r9
>   430:	55 29 04 5e 	rlwinm  r9,r9,0,17,15
>   434:	7d 20 01 24 	mtmsr   r9
>   438:	7d 20 00 a6 	mfmsr   r9
>   43c:	55 29 07 fa 	rlwinm  r9,r9,0,31,29
>   440:	55 29 04 5e 	rlwinm  r9,r9,0,17,15
>   444:	7d 20 01 24 	mtmsr   r9
>   448:	83 e1 00 0c 	lwz     r31,12(r1)
>   44c:	38 21 00 10 	addi    r1,r1,16
>   450:	4e 80 00 20 	blr
> 

Re: Declaring unrecoverable_exception() as __noreturn ?

[Michael Ellerman]

Nicholas Piggin <npiggin@gmail.com> writes:
> Excerpts from Christophe Leroy's message of February 11, 2021 2:44 am:
>> As far as I can see, almost all callers of unrecoverable_exception() expect it to never return.
>> 
>> Can we mark it __noreturn ?
>
> I don't see why not, do_exit is noreturn. We could make die() noreturn 
> as well.

I'm always nervous about that, because we can return if a debugger is
involved:

DEFINE_INTERRUPT_HANDLER(unrecoverable_exception)
{
	pr_emerg("Unrecoverable exception %lx at %lx (msr=%lx)\n",
		 regs->trap, regs->nip, regs->msr);
	die("Unrecoverable exception", regs, SIGABRT);
}

void die(const char *str, struct pt_regs *regs, long err)
{
	unsigned long flags;

	/*
	 * system_reset_excption handles debugger, crash dump, panic, for 0x100
	 */
	if (TRAP(regs) != 0x100) {
		if (debugger(regs))
			return;


We obviously don't want to optimise for that case, but it worries me
slightly if we're marking things noreturn when they can actually return.

cheers

Re: Declaring unrecoverable_exception() as __noreturn ?

[Christophe Leroy]

Le 11/02/2021 à 05:41, Michael Ellerman a écrit :
> Nicholas Piggin <npiggin@gmail.com> writes:
>> Excerpts from Christophe Leroy's message of February 11, 2021 2:44 am:
>>> As far as I can see, almost all callers of unrecoverable_exception() expect it to never return.
>>>
>>> Can we mark it __noreturn ?
>>
>> I don't see why not, do_exit is noreturn. We could make die() noreturn
>> as well.
> 
> I'm always nervous about that, because we can return if a debugger is
> involved:
> 
> DEFINE_INTERRUPT_HANDLER(unrecoverable_exception)

Hum ... Is that correct to define it as an interrupt handler ?

Also, I see it declared a second time in interrupt.c, this time not as an interrupt handler. Is that 
wanted ?

> {
> 	pr_emerg("Unrecoverable exception %lx at %lx (msr=%lx)\n",
> 		 regs->trap, regs->nip, regs->msr);
> 	die("Unrecoverable exception", regs, SIGABRT);
> }
> 
> void die(const char *str, struct pt_regs *regs, long err)
> {
> 	unsigned long flags;
> 
> 	/*
> 	 * system_reset_excption handles debugger, crash dump, panic, for 0x100
> 	 */
> 	if (TRAP(regs) != 0x100) {
> 		if (debugger(regs))
> 			return;
> 
> 
> We obviously don't want to optimise for that case, but it worries me
> slightly if we're marking things noreturn when they can actually return.
> 

I don't think I want to declare die() as __noreturn, need to look at it more in details first.

Christophe