| Message ID | 20231026101957.320572-1-bjorn@kernel.org (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | [RFC] genirq/matrix: Dynamic bitmap allocation | expand |
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 :)
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; }