diff mbox series

[v3,1/3] MIPS: Remove noreturn attribute for nmi_exception_handler()

Message ID 1692005246-18399-2-git-send-email-yangtiezhu@loongson.cn (mailing list archive)
State Superseded
Headers show
Series Modify die() for MIPS | expand

Commit Message

Tiezhu Yang Aug. 14, 2023, 9:27 a.m. UTC
In the later patch, we will remove noreturn attribute for die(), in order
to make each patch can be built without errors and warnings, just remove
noreturn attribute for nmi_exception_handler() earlier because it calls
die(), otherwise there exists the following build error after the later
patch:

  arch/mips/kernel/traps.c:2001:1: error: 'noreturn' function does return [-Werror]

Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
---
 arch/mips/kernel/traps.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Maciej W. Rozycki Aug. 18, 2023, 2:39 a.m. UTC | #1
On Mon, 14 Aug 2023, Tiezhu Yang wrote:

> In the later patch, we will remove noreturn attribute for die(), in order
> to make each patch can be built without errors and warnings, just remove
> noreturn attribute for nmi_exception_handler() earlier because it calls
> die(), otherwise there exists the following build error after the later
> patch:

 I find the wording a bit odd here, but you'll have to rewrite the change 
description for the update requested below, so let's defer any style fixes 
to v4.

>   arch/mips/kernel/traps.c:2001:1: error: 'noreturn' function does return [-Werror]

 Now that I've looked into it in detail, this change is incomplete and 
will make the kernel go astray if `nmi_exception_handler' actually ever 
does return.  See code in arch/mips/kernel/genex.S, which calls this 
function and doesn't expect it to return.  It has to be fixed before 2/3 
can be considered.  I wonder how you didn't catch it: you did check how 
this code is used, didn't you?

 Before submitting an updated version can you actually arrange for the 
NOTIFY_STOP condition to happen in your lab and verify it is handled as 
expected?  And what was the motivation for this code update, just a 
hypothetical scenario?

  Maciej
Tiezhu Yang Aug. 19, 2023, 2:38 a.m. UTC | #2
On 08/18/2023 10:39 AM, Maciej W. Rozycki wrote:
> On Mon, 14 Aug 2023, Tiezhu Yang wrote:
>
>> In the later patch, we will remove noreturn attribute for die(), in order
>> to make each patch can be built without errors and warnings, just remove
>> noreturn attribute for nmi_exception_handler() earlier because it calls
>> die(), otherwise there exists the following build error after the later
>> patch:
>
>  I find the wording a bit odd here, but you'll have to rewrite the change
> description for the update requested below, so let's defer any style fixes
> to v4.
>
>>   arch/mips/kernel/traps.c:2001:1: error: 'noreturn' function does return [-Werror]
>
>  Now that I've looked into it in detail, this change is incomplete and
> will make the kernel go astray if `nmi_exception_handler' actually ever
> does return.  See code in arch/mips/kernel/genex.S, which calls this
> function and doesn't expect it to return.  It has to be fixed before 2/3
> can be considered.  I wonder how you didn't catch it: you did check how
> this code is used, didn't you?
>

I think the proper way is to keep the noreturn attribute for
nmi_exception_handler(), and add a noreturn function BUG() at
the end of nmi_exception_handler() to make sure it does not
return.

>  Before submitting an updated version can you actually arrange for the
> NOTIFY_STOP condition to happen in your lab and verify it is handled as
> expected?  And what was the motivation for this code update, just a
> hypothetical scenario?

Yes, just a hypothetical scenario.

Thanks,
Tiezhu
diff mbox series

Patch

diff --git a/arch/mips/kernel/traps.c b/arch/mips/kernel/traps.c
index 246c6a6..7a34674 100644
--- a/arch/mips/kernel/traps.c
+++ b/arch/mips/kernel/traps.c
@@ -1986,7 +1986,7 @@  int register_nmi_notifier(struct notifier_block *nb)
 	return raw_notifier_chain_register(&nmi_chain, nb);
 }
 
-void __noreturn nmi_exception_handler(struct pt_regs *regs)
+void nmi_exception_handler(struct pt_regs *regs)
 {
 	char str[100];