| Message ID | 20210930131708.35328-2-kernelfans@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm64/irqentry: remove duplicate housekeeping of rcu | expand |
On Thu, 30 Sep 2021 14:17:06 +0100, Pingfan Liu <kernelfans@gmail.com> wrote: > > 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. > > Signed-off-by: Pingfan Liu <kernelfans@gmail.com> > Reviewed-by: Mark Rutland <mark.rutland@arm.com> > Cc: "Paul E. McKenney" <paulmck@kernel.org> > Cc: Catalin Marinas <catalin.marinas@arm.com> > Cc: Will Deacon <will@kernel.org> > Cc: Marc Zyngier <maz@kernel.org> > Cc: Joey Gouly <joey.gouly@arm.com> > Cc: Sami Tolvanen <samitolvanen@google.com> > Cc: Julien Thierry <julien.thierry@arm.com> > Cc: Thomas Gleixner <tglx@linutronix.de> > Cc: Yuichi Ito <ito-yuichi@fujitsu.com> > Cc: linux-kernel@vger.kernel.org > To: linux-arm-kernel@lists.infradead.org > --- > kernel/irq/Kconfig | 3 +++ > kernel/irq/irqdesc.c | 4 ++++ > 2 files changed, 7 insertions(+) > > diff --git a/kernel/irq/Kconfig b/kernel/irq/Kconfig > index fbc54c2a7f23..defa1db2d664 100644 > --- a/kernel/irq/Kconfig > +++ b/kernel/irq/Kconfig > @@ -100,6 +100,9 @@ config IRQ_MSI_IOMMU > config HANDLE_DOMAIN_IRQ > bool > > +config HAVE_ARCH_IRQENTRY > + bool > + > config IRQ_TIMINGS > bool > > diff --git a/kernel/irq/irqdesc.c b/kernel/irq/irqdesc.c > index 4e3c29bb603c..fd5dd9d278b5 100644 > --- a/kernel/irq/irqdesc.c > +++ b/kernel/irq/irqdesc.c > @@ -693,7 +693,9 @@ int handle_domain_irq(struct irq_domain *domain, > struct irq_desc *desc; > int ret = 0; > > +#ifndef CONFIG_HAVE_ARCH_IRQENTRY > irq_enter(); > +#endif nit: I tend to prefer the 'if (!IS_ENABLED(CONFIG_*))' approach. > > /* The irqdomain code provides boundary checks */ > desc = irq_resolve_mapping(domain, hwirq); > @@ -702,7 +704,9 @@ int handle_domain_irq(struct irq_domain *domain, > else > ret = -EINVAL; > > +#ifndef CONFIG_HAVE_ARCH_IRQENTRY > irq_exit(); > +#endif > set_irq_regs(old_regs); > return ret; > } Apart from that: Reviewed-by: Marc Zyngier <maz@kernel.org> Thanks, M.
On Fri, Oct 01, 2021 at 10:15:16AM +0100, Marc Zyngier wrote: > On Thu, 30 Sep 2021 14:17:06 +0100, > Pingfan Liu <kernelfans@gmail.com> wrote: > > > > 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. > > > > Signed-off-by: Pingfan Liu <kernelfans@gmail.com> > > Reviewed-by: Mark Rutland <mark.rutland@arm.com> > > Cc: "Paul E. McKenney" <paulmck@kernel.org> > > Cc: Catalin Marinas <catalin.marinas@arm.com> > > Cc: Will Deacon <will@kernel.org> > > Cc: Marc Zyngier <maz@kernel.org> > > Cc: Joey Gouly <joey.gouly@arm.com> > > Cc: Sami Tolvanen <samitolvanen@google.com> > > Cc: Julien Thierry <julien.thierry@arm.com> > > Cc: Thomas Gleixner <tglx@linutronix.de> > > Cc: Yuichi Ito <ito-yuichi@fujitsu.com> > > Cc: linux-kernel@vger.kernel.org > > To: linux-arm-kernel@lists.infradead.org > > --- > > kernel/irq/Kconfig | 3 +++ > > kernel/irq/irqdesc.c | 4 ++++ > > 2 files changed, 7 insertions(+) > > > > diff --git a/kernel/irq/Kconfig b/kernel/irq/Kconfig > > index fbc54c2a7f23..defa1db2d664 100644 > > --- a/kernel/irq/Kconfig > > +++ b/kernel/irq/Kconfig > > @@ -100,6 +100,9 @@ config IRQ_MSI_IOMMU > > config HANDLE_DOMAIN_IRQ > > bool > > > > +config HAVE_ARCH_IRQENTRY > > + bool > > + > > config IRQ_TIMINGS > > bool > > > > diff --git a/kernel/irq/irqdesc.c b/kernel/irq/irqdesc.c > > index 4e3c29bb603c..fd5dd9d278b5 100644 > > --- a/kernel/irq/irqdesc.c > > +++ b/kernel/irq/irqdesc.c > > @@ -693,7 +693,9 @@ int handle_domain_irq(struct irq_domain *domain, > > struct irq_desc *desc; > > int ret = 0; > > > > +#ifndef CONFIG_HAVE_ARCH_IRQENTRY > > irq_enter(); > > +#endif > > nit: I tend to prefer the 'if (!IS_ENABLED(CONFIG_*))' approach. > I check the irqdesc.c, the other macro conditional statements have the style "#ifdef". So if using 'if (!IS_ENABLED(CONFIG_*))', the file looks a little disharmony. > > > > /* The irqdomain code provides boundary checks */ > > desc = irq_resolve_mapping(domain, hwirq); > > @@ -702,7 +704,9 @@ int handle_domain_irq(struct irq_domain *domain, > > else > > ret = -EINVAL; > > > > +#ifndef CONFIG_HAVE_ARCH_IRQENTRY > > irq_exit(); > > +#endif > > set_irq_regs(old_regs); > > return ret; > > } > > Apart from that: > > Reviewed-by: Marc Zyngier <maz@kernel.org> > I am not sure whether you agree with my opinion about the style of macro and hesitate to include your Reviewed-by. Could you kindly give your Reviewed-by to my V4, if it is OK for you? Thanks Pingfan
diff --git a/kernel/irq/Kconfig b/kernel/irq/Kconfig index fbc54c2a7f23..defa1db2d664 100644 --- a/kernel/irq/Kconfig +++ b/kernel/irq/Kconfig @@ -100,6 +100,9 @@ config IRQ_MSI_IOMMU config HANDLE_DOMAIN_IRQ bool +config HAVE_ARCH_IRQENTRY + bool + config IRQ_TIMINGS bool diff --git a/kernel/irq/irqdesc.c b/kernel/irq/irqdesc.c index 4e3c29bb603c..fd5dd9d278b5 100644 --- a/kernel/irq/irqdesc.c +++ b/kernel/irq/irqdesc.c @@ -693,7 +693,9 @@ int handle_domain_irq(struct irq_domain *domain, struct irq_desc *desc; int ret = 0; +#ifndef CONFIG_HAVE_ARCH_IRQENTRY irq_enter(); +#endif /* The irqdomain code provides boundary checks */ desc = irq_resolve_mapping(domain, hwirq); @@ -702,7 +704,9 @@ int handle_domain_irq(struct irq_domain *domain, else ret = -EINVAL; +#ifndef CONFIG_HAVE_ARCH_IRQENTRY irq_exit(); +#endif set_irq_regs(old_regs); return ret; }