Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
Date:   Thu, 28 Jul 2022 09:52:18 +0100
From:   Marc Zyngier <maz@kernel.org>
To:     Daniel Walker <danielwa@cisco.com>
Cc:     Thomas Gleixner <tglx@linutronix.de>,
        George Cherian <george.cherian@marvell.com>,
        sgoutham@marvell.com, "BOBBY Liu (bobbliu)" <bobbliu@cisco.com>,
        xe-linux-external@cisco.com, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] genirq: allow selection of number of sparse irqs
In-Reply-To: <20220728030420.2279713-1-danielwa@cisco.com>
References: <20220728030420.2279713-1-danielwa@cisco.com>
User-Agent: Roundcube Webmail/1.4.13
Message-ID: <980a561ed87c5530aab2e2b067074862@kernel.org>
Content-Type: text/plain; charset=US-ASCII;
 format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 2022-07-28 04:04, Daniel Walker wrote:
> Currently the maximum number of interrupters is capped at 8260 (64 +
> 8196) in most of the architectures were CONFIG_SPARSE_IRQ is selected.
> This upper limit is not sufficient for couple of existing SoC's from
> Marvell.
> For eg: Octeon TX2 series of processors support a maximum of 32K
> interrupters.
> 
> Allow configuration of the upper limit of the number of interrupts.
> 
> Cc: George Cherian <george.cherian@marvell.com>
> Cc: sgoutham@marvell.com
> Cc: "BOBBY Liu (bobbliu)" <bobbliu@cisco.com>
> Cc: xe-linux-external@cisco.com
> Signed-off-by: Daniel Walker <danielwa@cisco.com>
> ---
>  kernel/irq/Kconfig     | 23 +++++++++++++++++++++++
>  kernel/irq/internals.h | 10 +++++++++-
>  2 files changed, 32 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/irq/Kconfig b/kernel/irq/Kconfig
> index db3d174c53d4..b356217abcfe 100644
> --- a/kernel/irq/Kconfig
> +++ b/kernel/irq/Kconfig
> @@ -125,6 +125,29 @@ config SPARSE_IRQ
> 
>  	  If you don't know what to do here, say N.
> 
> +choice
> +	prompt "Select number of sparse irqs"
> +	depends on SPARSE_IRQ
> +	default SPARSE_IRQ_EXTEND_8K
> +	help
> +          Allows choosing the number of sparse irq's available on the
> +          system. For each 8k of additional irqs added there is 
> approximatly
> +          1kb of kernel size increase.
> +
> +config SPARSE_IRQ_EXTEND_8K
> +	bool "8k"
> +
> +config SPARSE_IRQ_EXTEND_16K
> +	bool "16K"
> +
> +config SPARSE_IRQ_EXTEND_32K
> +	bool "32K"
> +
> +config SPARSE_IRQ_EXTEND_64K
> +	bool "64K"
> +
> +endchoice
> +
>  config GENERIC_IRQ_DEBUGFS
>  	bool "Expose irq internals in debugfs"
>  	depends on DEBUG_FS
> diff --git a/kernel/irq/internals.h b/kernel/irq/internals.h
> index f09c60393e55..25fe5abf6c16 100644
> --- a/kernel/irq/internals.h
> +++ b/kernel/irq/internals.h
> @@ -12,7 +12,15 @@
>  #include <linux/sched/clock.h>
> 
>  #ifdef CONFIG_SPARSE_IRQ
> -# define IRQ_BITMAP_BITS	(NR_IRQS + 8196)
> +# if defined(CONFIG_SPARSE_IRQ_EXTEND_8K)
> +# define IRQ_BITMAP_BITS	(NR_IRQS + 8192 + 4)
> +# elif defined(CONFIG_SPARSE_IRQ_EXTEND_16K)
> +# define IRQ_BITMAP_BITS	(NR_IRQS + 16384 + 4)
> +# elif defined(CONFIG_SPARSE_IRQ_EXTEND_32K)
> +# define IRQ_BITMAP_BITS	(NR_IRQS + 32768 + 4)
> +# elif defined(CONFIG_SPARSE_IRQ_EXTEND_64K)
> +# define IRQ_BITMAP_BITS	(NR_IRQS + 65536 + 4)
> +# endif
>  #else
>  # define IRQ_BITMAP_BITS	NR_IRQS
>  #endif

It really feels like the wrong approach. If your system
supports a large number of interrupt (I guess it has
a GICv3 ITS), this shouldn't impact the lesser machines
(most people are using a distro kernel).

It also doesn't really scale: the GICv3 architecture gives
you up to 24 bits of interrupts. Are we going to allocate
2MB worth of bitmap? Future interrupt architectures may have
even larger interrupt spaces.

As it turns out, we already store the irqdesc in an rb-tree.
It doesn't take too much imagination to turn this into a
xarray and use it for both allocation and tracking.

It would also conveniently replace the irqs_resend bitmap
if using marks to flag the IRQs to be resent.

Thanks,

         M.
-- 
Jazz is not dead. It just smells funny...