2010-11-01 16:44:08

by Stephen Caudle

[permalink] [raw]
Subject: [PATCH] [ARM] gic: Unmask private interrupts on all cores during IRQ enable

Requesting/freeing private peripheral interrupts on multi-core chips that
use only one IRQ number for all cores currently unmasks/masks the interrupt
for only the executing core.

This change prevents the need for a separate call to enable_irq on other
cores after request_irq. Also, shutdown is implemented instead of disable
to allow for lazy IRQ disabling.

Signed-off-by: Stephen Caudle <[email protected]>
---
arch/arm/Kconfig | 5 +++
arch/arm/common/gic.c | 83 +++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 88 insertions(+), 0 deletions(-)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index a19a526..fbf5236 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1298,6 +1298,11 @@ config LOCAL_TIMERS
accounting to be spread across the timer interval, preventing a
"thundering herd" at every timer tick.

+config IRQ_PER_CPU
+ bool
+ depends on SMP
+ default n
+
source kernel/Kconfig.preempt

config HZ
diff --git a/arch/arm/common/gic.c b/arch/arm/common/gic.c
index ada6359..372cfb3 100644
--- a/arch/arm/common/gic.c
+++ b/arch/arm/common/gic.c
@@ -39,12 +39,25 @@ struct gic_chip_data {
unsigned int irq_offset;
void __iomem *dist_base;
void __iomem *cpu_base;
+#ifdef CONFIG_IRQ_PER_CPU
+ struct call_single_data ppi_data;
+#endif
};

#ifndef MAX_GIC_NR
#define MAX_GIC_NR 1
#endif

+#ifdef CONFIG_IRQ_PER_CPU
+#ifndef GIC_PPI_FIRST
+#define GIC_PPI_FIRST 16
+#endif
+
+#ifndef GIC_PPI_LAST
+#define GIC_PPI_LAST 31
+#endif
+#endif
+
static struct gic_chip_data gic_data[MAX_GIC_NR];

static inline void __iomem *gic_dist_base(unsigned int irq)
@@ -158,6 +171,72 @@ static int gic_set_cpu(unsigned int irq, const struct cpumask *mask_val)
}
#endif

+#ifdef CONFIG_IRQ_PER_CPU
+static inline void gic_smp_call_function(struct call_single_data *data)
+{
+ int cpu;
+ int this_cpu = smp_processor_id();
+
+ /*
+ * Since this function is called with interrupts disabled,
+ * smp_call_function can't be used here because it warns (even
+ * if wait = 0) when interrupts are disabled.
+ *
+ * __smp_call_function_single doesn't warn when interrupts are
+ * disabled and not waiting, so use it instead.
+ */
+ for_each_online_cpu(cpu)
+ if (cpu != this_cpu)
+ __smp_call_function_single(cpu, data, 0);
+}
+
+static void gic_mask_ppi(void *info)
+{
+ gic_mask_irq(*(unsigned int *)info);
+}
+
+static void gic_unmask_ppi(void *info)
+{
+ gic_unmask_irq(*(unsigned int *)info);
+}
+
+static void gic_enable_irq(unsigned int irq)
+{
+ struct irq_desc *desc = irq_to_desc(irq);
+ struct gic_chip_data *gic_data = get_irq_chip_data(irq);
+
+ if (irq >= GIC_PPI_FIRST && irq <= GIC_PPI_LAST) {
+ gic_data->ppi_data.func = gic_unmask_ppi;
+ gic_data->ppi_data.info = &desc->irq;
+ gic_data->ppi_data.flags = 0;
+
+ /* Unmask PPIs on all cores during enable. */
+ gic_smp_call_function(&gic_data->ppi_data);
+ }
+
+ desc->chip->unmask(irq);
+ desc->status &= ~IRQ_MASKED;
+}
+
+static void gic_shutdown_irq(unsigned int irq)
+{
+ struct irq_desc *desc = irq_to_desc(irq);
+ struct gic_chip_data *gic_data = get_irq_chip_data(irq);
+
+ if (irq >= GIC_PPI_FIRST && irq <= GIC_PPI_LAST) {
+ gic_data->ppi_data.func = gic_mask_ppi;
+ gic_data->ppi_data.info = &desc->irq;
+ gic_data->ppi_data.flags = 0;
+
+ /* Mask PPIs on all cores during disable. */
+ gic_smp_call_function(&gic_data->ppi_data);
+ }
+
+ desc->chip->mask(irq);
+ desc->status |= IRQ_MASKED;
+}
+#endif
+
static void gic_handle_cascade_irq(unsigned int irq, struct irq_desc *desc)
{
struct gic_chip_data *chip_data = get_irq_data(irq);
@@ -196,6 +275,10 @@ static struct irq_chip gic_chip = {
#ifdef CONFIG_SMP
.set_affinity = gic_set_cpu,
#endif
+#ifdef CONFIG_IRQ_PER_CPU
+ .enable = gic_enable_irq,
+ .shutdown = gic_shutdown_irq,
+#endif
};

void __init gic_cascade_irq(unsigned int gic_nr, unsigned int irq)
--
1.7.3.2

--
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum


2010-11-02 07:51:15

by Milton Miller

[permalink] [raw]
Subject: Re: [PATCH] [ARM] gic: Unmask private interrupts on all cores during IRQ enable

On Mon, 1 Nov 2010 about 12:39:55 -0400, Stephen Caudle wrote:
> +#ifdef CONFIG_IRQ_PER_CPU
> +static inline void gic_smp_call_function(struct call_single_data *data)
> +{
> + int cpu;
> + int this_cpu = smp_processor_id();
> +
> + /*
> + * Since this function is called with interrupts disabled,
> + * smp_call_function can't be used here because it warns (even
> + * if wait = 0) when interrupts are disabled.
> + *
> + * __smp_call_function_single doesn't warn when interrupts are
> + * disabled and not waiting, so use it instead.
> + */
> + for_each_online_cpu(cpu)
> + if (cpu != this_cpu)
> + __smp_call_function_single(cpu, data, 0);
> +}
> +

So you think that calling __smp_call_function_single with irqs disabled
with a data that is reused is not deadlock prone ?

If you look, __smp_call_function_single will wait in csd_lock until
the previous requested cpu has consumed the request (as it must, because
the data contains both the arguments and the list pointers to link
the element).

> +static void gic_enable_irq(unsigned int irq)
> +{
> + struct irq_desc *desc = irq_to_desc(irq);
> + struct gic_chip_data *gic_data = get_irq_chip_data(irq);
> +
> + if (irq >= GIC_PPI_FIRST && irq <= GIC_PPI_LAST) {
> + gic_data->ppi_data.func = gic_unmask_ppi;
> + gic_data->ppi_data.info = &desc->irq;
> + gic_data->ppi_data.flags = 0;

Oh, now the flags (and therefore the lock) are cleared, and the function
is overwritten before it is known that the last cpu has processed this
call function request.

So you could have anything from the wrong function executed to
the last cpu's ipi function call list is crossed with another cpus,
probably leading to other call function data never being processed. In
the best case other callers to call_function_single will hang forever
waiting for their own (per-cpu by originator) request to be consumed.

If you don't want to wait you must to allocate csd data per cpu, and
never clear flags except at the initial allocation. You can not
overwrite function or info unless you know its safe if the previous
ofunction(odata) is issued and the new nfunction(ndata) is also safe
as nfunction(odata) or ofunction(ndata) (or you put in additional
barriers so only one has to be safe).

And then someone has to decide delayed mask or unmask is safe.

[note: it may be difficult to impossible to observe these races
with nr_cpus <= 2]

btw, the generic code is moving to passing desc not irq numbers around.

milton