Re: [PATCHv2 3/5] kernel/irq: make irq_{enter,exit}() in handle_domain_irq() arch optional

From: Pingfan Liu <piliu@redhat.com>
To: Mark Rutland <mark.rutland@arm.com>
Cc: Pingfan Liu <kernelfans@gmail.com>,
	linux-kernel@vger.kernel.org,
	Thomas Gleixner <tglx@linutronix.de>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will@kernel.org>, Marc Zyngier <maz@kernel.org>,
	Joey Gouly <joey.gouly@arm.com>,
	Sami Tolvanen <samitolvanen@google.com>,
	Julien Thierry <julien.thierry@arm.com>,
	Yuichi Ito <ito-yuichi@fujitsu.com>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCHv2 3/5] kernel/irq: make irq_{enter,exit}() in handle_domain_irq() arch optional
Date: Wed, 29 Sep 2021 11:15:09 +0800	[thread overview]
Message-ID: <YVPaPf/rREkvZkok@piliu.users.ipa.redhat.com> (raw)
In-Reply-To: <20210928085500.GC1924@C02TD0UTHF1T.local>

On Tue, Sep 28, 2021 at 09:55:00AM +0100, Mark Rutland wrote:
> On Fri, Sep 24, 2021 at 09:28:35PM +0800, Pingfan Liu wrote:
> > handle_domain_irq() contains two major parts:
> >   -1. irq_enter()/irq_exit(), which serves as hooks for rcu and trace etc.
> >   -2. irq mapping and dispatching
> > 
> > After the introduction of irqentry_enter()/irqentry_exit() and arch
> > specific counterpart (e.g. arm64), roughly speaking, rcu_irq_enter() has
> > already been called.  Hence here comes requirement to move
> > irq_enter/irq_exit out of handle_domain_irq(). And arches should handle
> > about irq_enter_rcu()/irq_exit_rcu() by themself.
> > 
> > Since there is still arches, which does not adopt irqentry_enter(), let
> > irq_enter()/irq_exit() arch optional in handle_domain_irq().
> 
> The patch below looks good to me, but the commit message is a little
> hard to follow. How about:
> 
>   When an IRQ is taken, some accounting needs to be performed to enter
>   and exit IRQ context around the IRQ handler. Historically arch code
>   would leave this to the irqchip or core IRQ code, but these days we
>   want this to happen in exception entry code, and architectures such as
>   arm64 do this.
> 
>   Currently handle_domain_irq() performs this entry/exit accounting, and
>   if used on an architecture where the entry code also does this, the
>   entry/exit accounting will be performed twice per IRQ. This is
>   problematic as core RCU code such as rcu_is_cpu_rrupt_from_idle()
>   depends on this happening once per IRQ, and will not detect quescent
>   periods correctly, leading to stall warnings.
> 
>   As irqchip drivers which use handle_domain_irq() need to work on
>   architectures with or without their own entry/exit accounting, this
>   patch makes handle_domain_irq() conditionally perform the entry
>   accounting depending on a new HAVE_ARCH_IRQENTRY Kconfig symbol that
>   architectures can select if they perform this entry accounting
>   themselves.
> 
>   For architectures which do not select the symbol. there should be no
>   functional change as a result of this patch.
> 
Thanks for your precious time to improve the log. It looks great, and I
will use it in V2. I will keep learning to improve my level of log.

> With that commit message:
> 
>   Reviewed-by: Mark Rutland <mark.rutland@arm.com>
> 
Thanks,

	Pingfan