linux-kernel - Re: [PATCH 1/1] genirq/proc: Try to jump over the unallocated irq hole whenever possible

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Wed, 27 Mar 2024 00:36:57 +0100
From: Thomas Gleixner <tglx@...utronix.de>
To: Huang Adrian <adrianhuang0701@...il.com>
Cc: linux-kernel@...r.kernel.org, Jiwei Sun <sunjw10@...ovo.com>, Adrian
 Huang <ahuang12@...ovo.com>
Subject: Re: [PATCH 1/1] genirq/proc: Try to jump over the unallocated irq
 hole whenever possible

On Tue, Mar 26 2024 at 21:35, Huang Adrian wrote:
> On Tue, Mar 26, 2024 at 6:32 AM Thomas Gleixner <tglx@...utronix.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