Received-SPF: pass (google.com: domain of linux-kernel+bounces-120140-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
From: Thomas Gleixner <tglx@linutronix.de>
To: Huang Adrian <adrianhuang0701@gmail.com>
Cc: linux-kernel@vger.kernel.org, Jiwei Sun <sunjw10@lenovo.com>, Adrian
 Huang <ahuang12@lenovo.com>
Subject: Re: [PATCH 1/1] genirq/proc: Try to jump over the unallocated irq
 hole whenever possible
In-Reply-To: <CAHKZfL1xq-pemEMo7m8DctEVxSG_ASQWxCC_HsN=-92UAjP_Mw@mail.gmail.com>
References: <20240325035102.15245-1-adrianhuang0701@gmail.com>
 <874jcueydh.ffs@tglx>
 <CAHKZfL1xq-pemEMo7m8DctEVxSG_ASQWxCC_HsN=-92UAjP_Mw@mail.gmail.com>
Date: Wed, 27 Mar 2024 00:36:57 +0100
Message-ID: <87h6gsefae.ffs@tglx>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable

On Tue, Mar 26 2024 at 21:35, Huang Adrian wrote:
> On Tue, Mar 26, 2024 at 6:32=E2=80=AFAM 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 =3D 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 |=3D 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