Re: [PATCH v3 04/11] x86/irq: Factor out common NMI handling code

[Jacob Pan <jacob.jun.pan@linux.intel.com>]

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