Re: [patch v2 07/14] genirq: Calculate precision only when required

[Radu Rendec]

On Fri, 2026-03-20 at 14:21 +0100, Thomas Gleixner wrote:
> Calculating the precision of the interrupt number column on every initial
> show_interrupt() invocation is a pointless exercise as the underlying
> maximum number of interrupts rarely changes.
> 
> Calculate it only when that number is modified and let show_interrupts()
> use the cached value.
> 
> Signed-off-by: Thomas Gleixner <tglx@xxxxxxxxxx>
> Reviewed-by: Dmitry Ilvokhin <d@xxxxxxxxxxxx>
> ---
>  kernel/irq/internals.h |    6 ++++++
>  kernel/irq/irqdesc.c   |   10 ++++++----
>  kernel/irq/proc.c      |   28 +++++++++++++++++++---------
>  3 files changed, 31 insertions(+), 13 deletions(-)
> 
> --- a/kernel/irq/internals.h
> +++ b/kernel/irq/internals.h
> @@ -134,6 +134,12 @@ static inline void unregister_handler_pr
>  static inline void irq_proc_update_valid(struct irq_desc *desc) { }
>  #endif
>  
> +#if defined(CONFIG_PROC_FS) && defined(CONFIG_GENERIC_IRQ_SHOW)
> +void irq_proc_calc_prec(void);
> +#else
> +static inline void irq_proc_calc_prec(void) { }
> +#endif
> +
>  extern bool irq_can_set_affinity_usr(unsigned int irq);
>  
>  extern int irq_do_set_affinity(struct irq_data *data,
> --- a/kernel/irq/irqdesc.c
> +++ b/kernel/irq/irqdesc.c
> @@ -157,13 +157,12 @@ EXPORT_SYMBOL_GPL(irq_get_nr_irqs);
>   *
>   * Return: @nr.
>   */
> -unsigned int irq_set_nr_irqs(unsigned int nr)
> +unsigned int __init irq_set_nr_irqs(unsigned int nr)
>  {
>  	total_nr_irqs = nr;
> -
> +	irq_proc_calc_prec();
>  	return nr;
>  }
> -EXPORT_SYMBOL_GPL(irq_set_nr_irqs);
>  
>  static DEFINE_MUTEX(sparse_irq_lock);
>  static struct maple_tree sparse_irqs = MTREE_INIT_EXT(sparse_irqs,
> @@ -544,6 +543,7 @@ static bool irq_expand_nr_irqs(unsigned
>  	if (nr > MAX_SPARSE_IRQS)
>  		return false;
>  	total_nr_irqs = nr;
> +	irq_proc_calc_prec();
>  	return true;
>  }
>  
> @@ -572,6 +572,7 @@ int __init early_irq_init(void)
>  		desc = alloc_desc(i, node, 0, NULL, NULL);
>  		irq_insert_desc(i, desc);
>  	}
> +	irq_proc_calc_prec();
>  	return arch_early_irq_init();
>  }
>  
> @@ -592,7 +593,7 @@ int __init early_irq_init(void)
>  
>  	init_irq_default_affinity();
>  
> -	printk(KERN_INFO "NR_IRQS: %d\n", NR_IRQS);
> +	pr_info("NR_IRQS: %d\n", NR_IRQS);
>  
>  	count = ARRAY_SIZE(irq_desc);
>  
> @@ -602,6 +603,7 @@ int __init early_irq_init(void)
>  			goto __free_desc_res;
>  	}
>  
> +	irq_proc_calc_prec();
>  	return arch_early_irq_init();
>  
>  __free_desc_res:
> --- a/kernel/irq/proc.c
> +++ b/kernel/irq/proc.c
> @@ -457,10 +457,21 @@ int __weak arch_show_interrupts(struct s
>  	return 0;
>  }
>  
> +static int irq_num_prec __read_mostly = 3;
> +
>  #ifndef ACTUAL_NR_IRQS
>  # define ACTUAL_NR_IRQS total_nr_irqs
>  #endif
>  
> +void irq_proc_calc_prec(void)
> +{
> +	unsigned int prec, n;
> +
> +	for (prec = 3, n = 1000; prec < 10 && n <= total_nr_irqs; ++prec)
> +		n *= 10;
> +	WRITE_ONCE(irq_num_prec, prec);
> +}
> +
>  #define ZSTR1 "          0"
>  #define ZSTR1_LEN	(sizeof(ZSTR1) - 1)
>  #define ZSTR16		ZSTR1 ZSTR1 ZSTR1 ZSTR1 ZSTR1 ZSTR1 ZSTR1 ZSTR1 \
> @@ -499,8 +510,7 @@ void irq_proc_emit_counts(struct seq_fil
>  
>  int show_interrupts(struct seq_file *p, void *v)
>  {
> -	const unsigned int nr_irqs = irq_get_nr_irqs();
> -	static int prec;
> +	int prec = READ_ONCE(irq_num_prec);
>  
>  	int i = *(loff_t *) v, j;
>  	struct irqaction *action;
> @@ -514,9 +524,6 @@ int show_interrupts(struct seq_file *p,
>  
>  	/* print header and calculate the width of the first column */
>  	if (i == 0) {
> -		for (prec = 3, j = 1000; prec < 10 && j <= nr_irqs; ++prec)
> -			j *= 10;
> -
>  		seq_printf(p, "%*s", prec + 8, "");
>  		for_each_online_cpu(j)
>  			seq_printf(p, "CPU%-8d", j);
> @@ -552,13 +559,16 @@ int show_interrupts(struct seq_file *p,
>  	} else {
>  		seq_printf(p, "%8s", "None");
>  	}
> +
> +	seq_putc(p, ' ');
>  	if (desc->irq_data.domain)
> -		seq_printf(p, " %*lu", prec, desc->irq_data.hwirq);
> +		seq_put_decimal_ull_width(p, "", desc->irq_data.hwirq, prec);
>  	else
>  		seq_printf(p, " %*s", prec, "");
> -#ifdef CONFIG_GENERIC_IRQ_SHOW_LEVEL
> -	seq_printf(p, " %-8s", irqd_is_level_type(&desc->irq_data) ? "Level" : "Edge");
> -#endif
> +
> +	if (IS_ENABLED(CONFIG_GENERIC_IRQ_SHOW_LEVEL))
> +		seq_printf(p, " %-8s", irqd_is_level_type(&desc->irq_data) ? "Level" : "Edge");
> +
>  	if (desc->name)
>  		seq_printf(p, "-%-8s", desc->name);
>  

Reviewed-by: Radu Rendec <radu@xxxxxxxxxx>