Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 18 Aug 2023 03:39:55 +0100 (BST)
From:   "Maciej W. Rozycki" <macro@orcam.me.uk>
To:     Tiezhu Yang <yangtiezhu@loongson.cn>
cc:     Thomas Bogendoerfer <tsbogend@alpha.franken.de>,
        linux-mips@vger.kernel.org, linux-kernel@vger.kernel.org,
        loongson-kernel@lists.loongnix.cn
Subject: Re: [PATCH v3 1/3] MIPS: Remove noreturn attribute for
 nmi_exception_handler()
In-Reply-To: <1692005246-18399-2-git-send-email-yangtiezhu@loongson.cn>
Message-ID: <alpine.DEB.2.21.2308180227580.8596@angie.orcam.me.uk>
References: <1692005246-18399-1-git-send-email-yangtiezhu@loongson.cn> <1692005246-18399-2-git-send-email-yangtiezhu@loongson.cn>
User-Agent: Alpine 2.21 (DEB 202 2017-01-01)
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Precedence: bulk

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