In preparation for handling NMIs with explicit source reporting, factor
out common code for reuse.

Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com>
---
 arch/x86/kernel/nmi.c | 28 ++++++++++++++++------------
 1 file changed, 16 insertions(+), 12 deletions(-)

diff --git a/arch/x86/kernel/nmi.c b/arch/x86/kernel/nmi.c
index 1ebe93edba7a..639a34e78bc9 100644
--- a/arch/x86/kernel/nmi.c
+++ b/arch/x86/kernel/nmi.c
@@ -135,6 +135,20 @@ static void nmi_check_duration(struct nmiaction *action, u64 duration)
 		action->handler, duration, decimal_msecs);
 }
 
+static inline int do_handle_nmi(struct nmiaction *a, struct pt_regs *regs, unsigned int type)
+{
+	int thishandled;
+	u64 delta;
+
+	delta = sched_clock();
+	thishandled = a->handler(type, regs);
+	delta = sched_clock() - delta;
+	trace_nmi_handler(a->handler, (int)delta, thishandled);
+	nmi_check_duration(a, delta);
+
+	return thishandled;
+}
+
 static int nmi_handle(unsigned int type, struct pt_regs *regs)
 {
 	struct nmi_desc *desc = nmi_to_desc(type);
@@ -149,18 +163,8 @@ static int nmi_handle(unsigned int type, struct pt_regs *regs)
 	 * can be latched at any given time.  Walk the whole list
 	 * to handle those situations.
 	 */
-	list_for_each_entry_rcu(a, &desc->head, list) {
-		int thishandled;
-		u64 delta;
-
-		delta = sched_clock();
-		thishandled = a->handler(type, regs);
-		handled += thishandled;
-		delta = sched_clock() - delta;
-		trace_nmi_handler(a->handler, (int)delta, thishandled);
-
-		nmi_check_duration(a, delta);
-	}
+	list_for_each_entry_rcu(a, &desc->head, list)
+		handled += do_handle_nmi(a, regs, type);
 
 	rcu_read_unlock();
 
-- 
2.25.1

On 6/28/2024 1:18 PM, Jacob Pan wrote:
> In preparation for handling NMIs with explicit source reporting, factor
> out common code for reuse.
> 

My read is that this patch has no functional change, right?

If yes, please add "No functional change intended."

> Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com>
> ---
>   arch/x86/kernel/nmi.c | 28 ++++++++++++++++------------
>   1 file changed, 16 insertions(+), 12 deletions(-)
> 
> diff --git a/arch/x86/kernel/nmi.c b/arch/x86/kernel/nmi.c
> index 1ebe93edba7a..639a34e78bc9 100644
> --- a/arch/x86/kernel/nmi.c
> +++ b/arch/x86/kernel/nmi.c
> @@ -135,6 +135,20 @@ static void nmi_check_duration(struct nmiaction *action, u64 duration)
>   		action->handler, duration, decimal_msecs);
>   }
>   
> +static inline int do_handle_nmi(struct nmiaction *a, struct pt_regs *regs, unsigned int type)
> +{
> +	int thishandled;
> +	u64 delta;
> +
> +	delta = sched_clock();
> +	thishandled = a->handler(type, regs);
> +	delta = sched_clock() - delta;
> +	trace_nmi_handler(a->handler, (int)delta, thishandled);
> +	nmi_check_duration(a, delta);
> +
> +	return thishandled;
> +}
> +
>   static int nmi_handle(unsigned int type, struct pt_regs *regs)
>   {
>   	struct nmi_desc *desc = nmi_to_desc(type);
> @@ -149,18 +163,8 @@ static int nmi_handle(unsigned int type, struct pt_regs *regs)
>   	 * can be latched at any given time.  Walk the whole list
>   	 * to handle those situations.
>   	 */
> -	list_for_each_entry_rcu(a, &desc->head, list) {
> -		int thishandled;
> -		u64 delta;
> -
> -		delta = sched_clock();
> -		thishandled = a->handler(type, regs);
> -		handled += thishandled;
> -		delta = sched_clock() - delta;
> -		trace_nmi_handler(a->handler, (int)delta, thishandled);
> -
> -		nmi_check_duration(a, delta);
> -	}
> +	list_for_each_entry_rcu(a, &desc->head, list)
> +		handled += do_handle_nmi(a, regs, type);
>   
>   	rcu_read_unlock();
>   

As this is a preparation patch, better move it earlier before any actual 
NMI source changes, maybe the first patch of this series.

Thanks!
     Xin

On Fri, 28 Jun 2024 17:31:50 -0700, Xin Li <xin@zytor.com> wrote:

> On 6/28/2024 1:18 PM, Jacob Pan wrote:
> > In preparation for handling NMIs with explicit source reporting, factor
> > out common code for reuse.
> >   
> 
> My read is that this patch has no functional change, right?
> 
> If yes, please add "No functional change intended."
will do.

> > Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com>
> > ---
> >   arch/x86/kernel/nmi.c | 28 ++++++++++++++++------------
> >   1 file changed, 16 insertions(+), 12 deletions(-)
> > 
> > diff --git a/arch/x86/kernel/nmi.c b/arch/x86/kernel/nmi.c
> > index 1ebe93edba7a..639a34e78bc9 100644
> > --- a/arch/x86/kernel/nmi.c
> > +++ b/arch/x86/kernel/nmi.c
> > @@ -135,6 +135,20 @@ static void nmi_check_duration(struct nmiaction
> > *action, u64 duration) action->handler, duration, decimal_msecs);
> >   }
> >   
> > +static inline int do_handle_nmi(struct nmiaction *a, struct pt_regs
> > *regs, unsigned int type) +{
> > +	int thishandled;
> > +	u64 delta;
> > +
> > +	delta = sched_clock();
> > +	thishandled = a->handler(type, regs);
> > +	delta = sched_clock() - delta;
> > +	trace_nmi_handler(a->handler, (int)delta, thishandled);
> > +	nmi_check_duration(a, delta);
> > +
> > +	return thishandled;
> > +}
> > +
> >   static int nmi_handle(unsigned int type, struct pt_regs *regs)
> >   {
> >   	struct nmi_desc *desc = nmi_to_desc(type);
> > @@ -149,18 +163,8 @@ static int nmi_handle(unsigned int type, struct
> > pt_regs *regs)
> >   	 * can be latched at any given time.  Walk the whole list
> >   	 * to handle those situations.
> >   	 */
> > -	list_for_each_entry_rcu(a, &desc->head, list) {
> > -		int thishandled;
> > -		u64 delta;
> > -
> > -		delta = sched_clock();
> > -		thishandled = a->handler(type, regs);
> > -		handled += thishandled;
> > -		delta = sched_clock() - delta;
> > -		trace_nmi_handler(a->handler, (int)delta, thishandled);
> > -
> > -		nmi_check_duration(a, delta);
> > -	}
> > +	list_for_each_entry_rcu(a, &desc->head, list)
> > +		handled += do_handle_nmi(a, regs, type);
> >   
> >   	rcu_read_unlock();
> >     
> 
> As this is a preparation patch, better move it earlier before any actual 
> NMI source changes, maybe the first patch of this series.
This preparatory patch is utilized immediately by the subsequent patch,
enhancing the narrative flow, in my opinion.

Thanks,

Jacob