Re: [patch v2 06/14] genirq: Cache the condition for /proc/interrupts exposure

[Radu Rendec <radu@rendec.net>]

On Fri, 2026-03-20 at 14:21 +0100, Thomas Gleixner wrote:
> show_interrupts() evaluates a boatload of conditions to establish whether
> exposing an interrupt in /proc/interrupts or not.
> 
> That can be simplified by caching the condition in an internal status flag,
> which is updated when one of the relevant inputs changes.
> 
> As a result the number of instructions and branches for reading
> /proc/interrupts is reduced significantly.
> 
> Signed-off-by: Thomas Gleixner <tglx@kernel.org>
> Reviewed-by: Dmitry Ilvokhin <d@ilvokhin.com>
> ---
> V2: s/IRQF_/IRQ/ and fixup the enum treatment - Dmitry
> ---
>  include/linux/irq.h    |    1 +
>  kernel/irq/chip.c      |    2 ++
>  kernel/irq/internals.h |    2 ++
>  kernel/irq/manage.c    |    2 ++
>  kernel/irq/proc.c      |   16 ++++++++++++----
>  kernel/irq/settings.h  |   13 +++++++++++++
>  6 files changed, 32 insertions(+), 4 deletions(-)
> 
> --- a/include/linux/irq.h
> +++ b/include/linux/irq.h
> @@ -99,6 +99,7 @@ enum {
>  	IRQ_DISABLE_UNLAZY	= (1 << 19),
>  	IRQ_HIDDEN		= (1 << 20),
>  	IRQ_NO_DEBUG		= (1 << 21),
> +	IRQ_RESERVED		= (1 << 22),
>  };
>  
>  #define IRQF_MODIFY_MASK	\
> --- a/kernel/irq/chip.c
> +++ b/kernel/irq/chip.c
> @@ -1004,6 +1004,7 @@ static void
>  		WARN_ON(irq_chip_pm_get(irq_desc_get_irq_data(desc)));
>  		irq_activate_and_startup(desc, IRQ_RESEND);
>  	}
> +	irq_proc_update_valid(desc);
>  }
>  
>  void __irq_set_handler(unsigned int irq, irq_flow_handler_t handle, int is_chained,
> @@ -1064,6 +1065,7 @@ void irq_modify_status(unsigned int irq,
>  			trigger = tmp;
>  
>  		irqd_set(&desc->irq_data, trigger);
> +		irq_proc_update_valid(desc);
>  	}
>  }
>  EXPORT_SYMBOL_GPL(irq_modify_status);
> --- a/kernel/irq/internals.h
> +++ b/kernel/irq/internals.h
> @@ -123,6 +123,7 @@ extern void register_irq_proc(unsigned i
>  extern void unregister_irq_proc(unsigned int irq, struct irq_desc *desc);
>  extern void register_handler_proc(unsigned int irq, struct irqaction *action);
>  extern void unregister_handler_proc(unsigned int irq, struct irqaction *action);
> +void irq_proc_update_valid(struct irq_desc *desc);
>  #else
>  static inline void register_irq_proc(unsigned int irq, struct irq_desc *desc) { }
>  static inline void unregister_irq_proc(unsigned int irq, struct irq_desc *desc) { }
> @@ -130,6 +131,7 @@ static inline void register_handler_proc
>  					 struct irqaction *action) { }
>  static inline void unregister_handler_proc(unsigned int irq,
>  					   struct irqaction *action) { }
> +static inline void irq_proc_update_valid(struct irq_desc *desc) { }
>  #endif
>  
>  extern bool irq_can_set_affinity_usr(unsigned int irq);
> --- a/kernel/irq/manage.c
> +++ b/kernel/irq/manage.c
> @@ -1802,6 +1802,7 @@ static int
>  		__enable_irq(desc);
>  	}
>  
> +	irq_proc_update_valid(desc);
>  	raw_spin_unlock_irqrestore(&desc->lock, flags);
>  	chip_bus_sync_unlock(desc);
>  	mutex_unlock(&desc->request_mutex);
> @@ -1906,6 +1907,7 @@ static struct irqaction *__free_irq(stru
>  		desc->affinity_hint = NULL;
>  #endif
>  
> +	irq_proc_update_valid(desc);
>  	raw_spin_unlock_irqrestore(&desc->lock, flags);
>  	/*
>  	 * Drop bus_lock here so the changes which were done in the chip
> --- a/kernel/irq/proc.c
> +++ b/kernel/irq/proc.c
> @@ -439,6 +439,17 @@ void init_irq_proc(void)
>  		register_irq_proc(irq, desc);
>  }
>  
> +void irq_proc_update_valid(struct irq_desc *desc)
> +{
> +	u32 set = _IRQ_PROC_VALID;
> +
> +	if (irq_settings_is_hidden(desc) || !desc->action ||
> +	    irq_desc_is_chained(desc) || !desc->kstat_irqs)

Can desc->kstat_irqs ever be NULL? Looking at kernel/irq/irqdesc.c, it
seems to me that it's allocated always and very early (before every
other field in struct irq_desc), and with an explicit check for NULL on
allocation. It's also deallocated late, right before struct irq_desc
itself is deallocated (in irq_kobj_release()).

Other than that, LGTM.

> +		set = 0;
> +
> +	irq_settings_update_proc_valid(desc, set);
> +}
> +
>  #ifdef CONFIG_GENERIC_IRQ_SHOW
>  
>  int __weak arch_show_interrupts(struct seq_file *p, int prec)
> @@ -514,10 +525,7 @@ int show_interrupts(struct seq_file *p,
>  
>  	guard(rcu)();
>  	desc = irq_to_desc(i);
> -	if (!desc || irq_settings_is_hidden(desc))
> -		return 0;
> -
> -	if (!desc->action || irq_desc_is_chained(desc) || !desc->kstat_irqs)
> +	if (!desc || !irq_settings_proc_valid(desc))
>  		return 0;
>  
>  	seq_printf(p, "%*d:", prec, i);
> --- a/kernel/irq/settings.h
> +++ b/kernel/irq/settings.h
> @@ -18,6 +18,7 @@ enum {
>  	_IRQ_DISABLE_UNLAZY	= IRQ_DISABLE_UNLAZY,
>  	_IRQ_HIDDEN		= IRQ_HIDDEN,
>  	_IRQ_NO_DEBUG		= IRQ_NO_DEBUG,
> +	_IRQ_PROC_VALID		= IRQ_RESERVED,
>  	_IRQF_MODIFY_MASK	= IRQF_MODIFY_MASK,
>  };
>  
> @@ -34,6 +35,7 @@ enum {
>  #define IRQ_DISABLE_UNLAZY	GOT_YOU_MORON
>  #define IRQ_HIDDEN		GOT_YOU_MORON
>  #define IRQ_NO_DEBUG		GOT_YOU_MORON
> +#define IRQ_RESERVED		GOT_YOU_MORON
>  #undef IRQF_MODIFY_MASK
>  #define IRQF_MODIFY_MASK	GOT_YOU_MORON
>  
> @@ -180,3 +182,14 @@ static inline bool irq_settings_no_debug
>  {
>  	return desc->status_use_accessors & _IRQ_NO_DEBUG;
>  }
> +
> +static inline bool irq_settings_proc_valid(struct irq_desc *desc)
> +{
> +	return desc->status_use_accessors & _IRQ_PROC_VALID;
> +}
> +
> +static inline void irq_settings_update_proc_valid(struct irq_desc *desc, u32 set)
> +{
> +	desc->status_use_accessors &= ~_IRQ_PROC_VALID;
> +	desc->status_use_accessors |= (set & _IRQ_PROC_VALID);
> +}