Messages in this thread | | | From | Thomas Gleixner <> | Subject | Re: [PATCH 1/1] genirq/proc: Try to jump over the unallocated irq hole whenever possible | Date | Wed, 27 Mar 2024 00:36:57 +0100 |
| |
On Tue, Mar 26 2024 at 21:35, Huang Adrian wrote: > On Tue, Mar 26, 2024 at 6:32 AM Thomas Gleixner <tglx@linutronix.de> wrote: > The reason I made the changes in show_interrupts() is to minimize the > traversal times of the 'sparse_irqs' maple tree since irq_to_desc() is > invoked in show_interrupts().
They are not really relevant. The cache line is hot after the irq_get_next_irq() lookup and maple tree is a highly optimized data structure.
I'm not saying that it is free, but if you want to avoid that in the first place then you need to do a lookup of the next descriptor and hand it into show_interrupts() right away and not just hack some completely obscure optimization into show_interrupts().
> @@ -19,6 +19,10 @@ static void *int_seq_next(struct seq_file *f, void > *v, loff_t *pos) [ 3 more citation lines. Click/Enter to show. ] > (*pos)++; > if (*pos > nr_irqs) > return NULL; > + > + if (!irq_to_desc(*pos)) > + *pos = irq_get_next_irq(*pos);
How is irq_get_next_irq() valid without either holding the sparse irq lock or RCU read lock?
Testing this with full debug enabled might give you the answer to that question.
But that aside you are missing a massive performance problem in the generic show_interrupts() code:
if (desc->kstat_irqs) { for_each_online_cpu(j) any_count |= data_race(*per_cpu_ptr(desc->kstat_irqs, j)); }
if ((!desc->action || irq_desc_is_chained(desc)) && !any_count) goto outsparse;
There are two obvious problems with that, no?
Thanks,
tglx
| |