[RFC] genirq/matrix: Dynamic bitmap allocation

  From: Björn Töpel <bjorn@rivosinc.com>

Some (future) users of the irq matrix allocator, do not know the size
of the matrix bitmaps at compile time.

To avoid wasting memory on unnecessary large bitmaps, size the bitmap
at matrix allocation time.

Signed-off-by: Björn Töpel <bjorn@rivosinc.com>
---
Here's a cleaned up, boot tested, proper patch.

Thomas, this is just FYI/RFC. This change would only make sense, if
the RISC-V AIA series starts using the IRQ matrix allocator.


Björn
---
 arch/x86/include/asm/hw_irq.h |  2 --
 kernel/irq/matrix.c           | 28 +++++++++++++++++-----------
 2 files changed, 17 insertions(+), 13 deletions(-)


base-commit: 611da07b89fdd53f140d7b33013f255bf0ed8f34
  

Björn!

On Thu, Oct 26 2023 at 12:19, Björn Töpel wrote:
> Thomas, this is just FYI/RFC. This change would only make sense, if
> the RISC-V AIA series starts using the IRQ matrix allocator.

I'm not seeing anything horrible with that.

Thanks,

        tglx
  

On Thu, Oct 26, 2023 at 3:50 PM Björn Töpel <bjorn@kernel.org> wrote:
>
> From: Björn Töpel <bjorn@rivosinc.com>
>
> Some (future) users of the irq matrix allocator, do not know the size
> of the matrix bitmaps at compile time.
>
> To avoid wasting memory on unnecessary large bitmaps, size the bitmap
> at matrix allocation time.
>
> Signed-off-by: Björn Töpel <bjorn@rivosinc.com>
> ---
> Here's a cleaned up, boot tested, proper patch.
>
> Thomas, this is just FYI/RFC. This change would only make sense, if
> the RISC-V AIA series starts using the IRQ matrix allocator.

I have dropped "multi-MSI" support from the AIA v12 series
and also integrated the IRQ matrix allocator. This patch is
included in the AIA v12 series.

For reference, look at riscv_aia_v12 branch at:
https://github.com/avpatel/linux.git

I still need to rebase this upon device MSI domain support
from Thomas.

Regards,
Anup


>
>
> Björn
> ---
>  arch/x86/include/asm/hw_irq.h |  2 --
>  kernel/irq/matrix.c           | 28 +++++++++++++++++-----------
>  2 files changed, 17 insertions(+), 13 deletions(-)
>
> diff --git a/arch/x86/include/asm/hw_irq.h b/arch/x86/include/asm/hw_irq.h
> index 551829884734..dcfaa3812306 100644
> --- a/arch/x86/include/asm/hw_irq.h
> +++ b/arch/x86/include/asm/hw_irq.h
> @@ -16,8 +16,6 @@
>
>  #include <asm/irq_vectors.h>
>
> -#define IRQ_MATRIX_BITS                NR_VECTORS
> -
>  #ifndef __ASSEMBLY__
>
>  #include <linux/percpu.h>
> diff --git a/kernel/irq/matrix.c b/kernel/irq/matrix.c
> index 1698e77645ac..996cbb46d654 100644
> --- a/kernel/irq/matrix.c
> +++ b/kernel/irq/matrix.c
> @@ -8,8 +8,6 @@
>  #include <linux/cpu.h>
>  #include <linux/irq.h>
>
> -#define IRQ_MATRIX_SIZE        (BITS_TO_LONGS(IRQ_MATRIX_BITS))
> -
>  struct cpumap {
>         unsigned int            available;
>         unsigned int            allocated;
> @@ -17,8 +15,8 @@ struct cpumap {
>         unsigned int            managed_allocated;
>         bool                    initialized;
>         bool                    online;
> -       unsigned long           alloc_map[IRQ_MATRIX_SIZE];
> -       unsigned long           managed_map[IRQ_MATRIX_SIZE];
> +       unsigned long           *managed_map;
> +       unsigned long           alloc_map[];
>  };
>
>  struct irq_matrix {
> @@ -32,8 +30,8 @@ struct irq_matrix {
>         unsigned int            total_allocated;
>         unsigned int            online_maps;
>         struct cpumap __percpu  *maps;
> -       unsigned long           scratch_map[IRQ_MATRIX_SIZE];
> -       unsigned long           system_map[IRQ_MATRIX_SIZE];
> +       unsigned long           *system_map;
> +       unsigned long           scratch_map[];
>  };
>
>  #define CREATE_TRACE_POINTS
> @@ -50,24 +48,32 @@ __init struct irq_matrix *irq_alloc_matrix(unsigned int matrix_bits,
>                                            unsigned int alloc_start,
>                                            unsigned int alloc_end)
>  {
> +       unsigned int cpu, matrix_size = BITS_TO_LONGS(matrix_bits);
>         struct irq_matrix *m;
>
> -       if (matrix_bits > IRQ_MATRIX_BITS)
> -               return NULL;
> -
> -       m = kzalloc(sizeof(*m), GFP_KERNEL);
> +       m = kzalloc(struct_size(m, scratch_map, matrix_size * 2), GFP_KERNEL);
>         if (!m)
>                 return NULL;
>
> +       m->system_map = &m->scratch_map[matrix_size];
> +
>         m->matrix_bits = matrix_bits;
>         m->alloc_start = alloc_start;
>         m->alloc_end = alloc_end;
>         m->alloc_size = alloc_end - alloc_start;
> -       m->maps = alloc_percpu(*m->maps);
> +       m->maps = __alloc_percpu(struct_size(m->maps, alloc_map, matrix_size * 2),
> +                                __alignof__(*m->maps));
>         if (!m->maps) {
>                 kfree(m);
>                 return NULL;
>         }
> +
> +       for_each_possible_cpu(cpu) {
> +               struct cpumap *cm = per_cpu_ptr(m->maps, cpu);
> +
> +               cm->managed_map = &cm->alloc_map[matrix_size];
> +       }
> +
>         return m;
>  }
>
>
> base-commit: 611da07b89fdd53f140d7b33013f255bf0ed8f34
> --
> 2.40.1
>
  

Anup Patel <apatel@ventanamicro.com> writes:

> On Thu, Oct 26, 2023 at 3:50 PM Björn Töpel <bjorn@kernel.org> wrote:
>>
>> From: Björn Töpel <bjorn@rivosinc.com>
>>
>> Some (future) users of the irq matrix allocator, do not know the size
>> of the matrix bitmaps at compile time.
>>
>> To avoid wasting memory on unnecessary large bitmaps, size the bitmap
>> at matrix allocation time.
>>
>> Signed-off-by: Björn Töpel <bjorn@rivosinc.com>
>> ---
>> Here's a cleaned up, boot tested, proper patch.
>>
>> Thomas, this is just FYI/RFC. This change would only make sense, if
>> the RISC-V AIA series starts using the IRQ matrix allocator.
>
> I have dropped "multi-MSI" support from the AIA v12 series
> and also integrated the IRQ matrix allocator. This patch is
> included in the AIA v12 series.

Ok!

> For reference, look at riscv_aia_v12 branch at:
> https://github.com/avpatel/linux.git
>
> I still need to rebase this upon device MSI domain support
> from Thomas.

Note that the per-device domain support is already upstream, it's only
the ARM cleanups that are not.

IOW, there's no need to wait for the ARM cleanups. :-)


Björn
  

On Thu, Oct 26 2023 at 19:26, Björn Töpel wrote:
> Note that the per-device domain support is already upstream, it's only
> the ARM cleanups that are not.
>
> IOW, there's no need to wait for the ARM cleanups. :-)

Correct. The &*@^@#%$ delayed ARM link was only meant as a reference on
how this is implemented and obviously as a reminder to the ARM folks to
get this finally done...

The main point is not to introduce new users of this failed programming
model and instead make them use the more future proof implementation
right away - which of course might to turn out to be completely wrong
5-10 years from now :)

Thanks,

        tglx
  

Hi Thomas,

On Fri, Oct 27, 2023 at 4:47 AM Thomas Gleixner <tglx@linutronix.de> wrote:
>
> On Thu, Oct 26 2023 at 19:26, Björn Töpel wrote:
> > Note that the per-device domain support is already upstream, it's only
> > the ARM cleanups that are not.
> >
> > IOW, there's no need to wait for the ARM cleanups. :-)
>
> Correct. The &*@^@#%$ delayed ARM link was only meant as a reference on
> how this is implemented and obviously as a reminder to the ARM folks to
> get this finally done...
>
> The main point is not to introduce new users of this failed programming
> model and instead make them use the more future proof implementation
> right away - which of course might to turn out to be completely wrong
> 5-10 years from now :)
>

We have three types of MSIs on RISC-V platforms:
1) PCI MSIs (handled by the IMSIC driver of the RISC-V AIA series)
2) Platform MSIs (handled by the IMSIC driver of the RISC-V AIA series)
3) Wired IRQs converted to platform MSIs (aka wired-to-MSI bridge, which
    is handled by APLIC driver of the RISC-V AIA series)

The RISC-V AIA series needs the generic IRQ framework changes
related to #2 and #3 (above) from your series hence my suggestion
to rebase on your series.
(https://lore.kernel.org/all/20221121135653.208611233@linutronix.de/)

Is there a way to have your genirq changes merged before all ARM
drivers have been moved to the new programming model ?
OR
Any other way to deal with this dependency ?

Regards,
Anup
  

On Fri, Oct 27 2023 at 10:01, Anup Patel wrote:
> On Fri, Oct 27, 2023 at 4:47 AM Thomas Gleixner <tglx@linutronix.de> wrote:
> We have three types of MSIs on RISC-V platforms:
> 1) PCI MSIs (handled by the IMSIC driver of the RISC-V AIA series)
> 2) Platform MSIs (handled by the IMSIC driver of the RISC-V AIA series)
> 3) Wired IRQs converted to platform MSIs (aka wired-to-MSI bridge, which
>     is handled by APLIC driver of the RISC-V AIA series)
>
> The RISC-V AIA series needs the generic IRQ framework changes
> related to #2 and #3 (above) from your series hence my suggestion
> to rebase on your series.
> (https://lore.kernel.org/all/20221121135653.208611233@linutronix.de/)
>
> Is there a way to have your genirq changes merged before all ARM
> drivers have been moved to the new programming model ?
> OR
> Any other way to deal with this dependency ?

I'll have a look at this maze again and see how that can be separated
from the ARM pile, unless you beat me to it :)