Hi Thomas,
On Wednesday, 31 May 2017 02:58:33 PDT Thomas Gleixner wrote:
> Shared interrupts do not go well with disabling auto enable:
>
> 1) The sharing interrupt might request it while it's still disabled and
> then wait for interrupts forever.
>
> 2) The interrupt might have been requested by the driver sharing the line
> before IRQ_NOAUTOEN has been set. So the driver which expects that
> disabled state after calling request_irq() will not get what it wants.
> Even worse, when it calls enable_irq() later, it will trigger the
> unbalanced enable_irq() warning.
>
> Reported-by: Brian Norris <[email protected]>
> Signed-off-by: Thomas Gleixner <[email protected]>
> ---
> kernel/irq/chip.c | 7 +++++++
> kernel/irq/manage.c | 12 ++++++++++--
> 2 files changed, 17 insertions(+), 2 deletions(-)
>
> --- a/kernel/irq/manage.c
> +++ b/kernel/irq/manage.c
> @@ -1328,11 +1328,19 @@ static int
> if (new->flags & IRQF_ONESHOT)
> desc->istate |= IRQS_ONESHOT;
>
> - if (irq_settings_can_autoenable(desc))
> + if (irq_settings_can_autoenable(desc)) {
> irq_startup(desc, true);
> - else
> + } else {
> + /*
> + * Shared interrupts do not go well with disabling
> + * auto enable. The sharing interrupt might request
> + * it while it's still disabled and then wait for
> + * interrupts forever.
> + */
> + WARN_ON_ONCE(new->flags & IRQF_SHARED);
> /* Undo nested disables: */
> desc->depth = 1;
> + }
>
> /* Exclude IRQ from balancing if requested */
> if (new->flags & IRQF_NOBALANCING) {
I'm currently attempting to clean up a hack that we have in the MIPS GIC
irqchip driver - we have some interrupts which are really per-CPU, but are
currently used with the regular non-per-CPU IRQ APIs. Please search for usage
of gic_all_vpes_local_irq_controller (or for the string "HACK") in drivers/
irqchip/irq-mips-gic.c if you wish to find what I'm talking about. The
important details are that the interrupts in question are both per-CPU and on
many systems are shared (between the CPU timer, performance counters & fast
debug channel).
I have been attempting to move towards using the per-CPU APIs instead in order
to remove this hack - ie. using setup_percpu_irq() & enable_percpu_irq() in
place of plain old setup_irq(). Unfortunately what I've run into is this:
- Per-CPU interrupts get the IRQ_NOAUTOEN flag set by default, in
irq_set_percpu_devid_flags(). I can see why this makes sense in the
general case, since the alternative is setup_percpu_irq() enabling the
interrupt on the CPU that calls it & leaving it disabled on others, which
feels a little unclean.
- Your warning above triggers when a shared interrupt has the IRQ_NOAUTOEN
flag set. I can see why your warning makes sense if another driver has
already enabled the shared interrupt, which would make IRQ_NOAUTOEN
ineffective. I'm not sure I follow your comment above the warning though -
it sounds like you're trying to describe something else?
For my interrupts which are both per-CPU & shared the combination of these 2
facts mean I end up triggering your warning. My current ideas include:
- I could clear the IRQ_NOAUTOEN flag before calling setup_percpu_irq(). In
my cases that should be fine - we call enable_percpu_irq() anyway, and
would just enable the IRQ slightly earlier on the CPU which calls
setup_percpu_irq() which wouldn't be a problem. It feels a bit yucky
though.
- I could adjust your warning such that it only triggers if the shared
interrupt is indeed already enabled. This relies on my understanding of
the issue described above being correct though, and it doesn't match your
comment so I'm unsure I'm imagining the same issue you were warning about.
Do you have any thoughts or suggestions?
Thanks,
Paul
On Tue, 5 Sep 2017, Paul Burton wrote:
> I'm currently attempting to clean up a hack that we have in the MIPS GIC
> irqchip driver - we have some interrupts which are really per-CPU, but are
> currently used with the regular non-per-CPU IRQ APIs. Please search for usage
> of gic_all_vpes_local_irq_controller (or for the string "HACK") in drivers/
> irqchip/irq-mips-gic.c if you wish to find what I'm talking about. The
> important details are that the interrupts in question are both per-CPU and on
> many systems are shared (between the CPU timer, performance counters & fast
> debug channel).
>
> I have been attempting to move towards using the per-CPU APIs instead in order
> to remove this hack - ie. using setup_percpu_irq() & enable_percpu_irq() in
> place of plain old setup_irq(). Unfortunately what I've run into is this:
>
> - Per-CPU interrupts get the IRQ_NOAUTOEN flag set by default, in
> irq_set_percpu_devid_flags(). I can see why this makes sense in the
> general case, since the alternative is setup_percpu_irq() enabling the
> interrupt on the CPU that calls it & leaving it disabled on others, which
> feels a little unclean.
>
> - Your warning above triggers when a shared interrupt has the IRQ_NOAUTOEN
> flag set. I can see why your warning makes sense if another driver has
> already enabled the shared interrupt, which would make IRQ_NOAUTOEN
> ineffective. I'm not sure I follow your comment above the warning though -
> it sounds like you're trying to describe something else?
> > + /*
> > + * Shared interrupts do not go well with disabling
> > + * auto enable. The sharing interrupt might request
> > + * it while it's still disabled and then wait for
> > + * interrupts forever.
> > + */
Assume the following:
request_irq(X, handler1, NOAUTOEN|SHARED, dev1);
now the second device does:
request_irq(X, handler2, SHARED, dev2):
which will see the first handler installed, so it wont run into the code
path which starts up the interrupt. That means as long as dev1 does not
explicitely enable the interrupt dev2 will wait for it forever.
> For my interrupts which are both per-CPU & shared the combination of these 2
> facts mean I end up triggering your warning. My current ideas include:
>
> - I could clear the IRQ_NOAUTOEN flag before calling setup_percpu_irq(). In
> my cases that should be fine - we call enable_percpu_irq() anyway, and
> would just enable the IRQ slightly earlier on the CPU which calls
> setup_percpu_irq() which wouldn't be a problem. It feels a bit yucky
> though.
What's the problem with IRQ_NOAUTOEN and do
setup_percpu_irq();
enable_percpu_irq();
on the boot CPU and then later call it when the secondary CPUs come up in
cpu bringup code or a hotplug state callback?
Thanks,
tglx
Hi Thomas,
On Wednesday, 6 September 2017 01:16:48 PDT Thomas Gleixner wrote:
> On Tue, 5 Sep 2017, Paul Burton wrote:
> > I'm currently attempting to clean up a hack that we have in the MIPS GIC
> > irqchip driver - we have some interrupts which are really per-CPU, but are
> > currently used with the regular non-per-CPU IRQ APIs. Please search for
> > usage of gic_all_vpes_local_irq_controller (or for the string "HACK") in
> > drivers/ irqchip/irq-mips-gic.c if you wish to find what I'm talking
> > about. The important details are that the interrupts in question are both
> > per-CPU and on many systems are shared (between the CPU timer,
> > performance counters & fast debug channel).
> >
> > I have been attempting to move towards using the per-CPU APIs instead in
> > order to remove this hack - ie. using setup_percpu_irq() &
> > enable_percpu_irq() in>
> > place of plain old setup_irq(). Unfortunately what I've run into is this:
> > - Per-CPU interrupts get the IRQ_NOAUTOEN flag set by default, in
> >
> > irq_set_percpu_devid_flags(). I can see why this makes sense in the
> > general case, since the alternative is setup_percpu_irq() enabling the
> > interrupt on the CPU that calls it & leaving it disabled on others,
> > which
> > feels a little unclean.
> >
> > - Your warning above triggers when a shared interrupt has the
> > IRQ_NOAUTOEN
> >
> > flag set. I can see why your warning makes sense if another driver has
> > already enabled the shared interrupt, which would make IRQ_NOAUTOEN
> > ineffective. I'm not sure I follow your comment above the warning
> > though -
> > it sounds like you're trying to describe something else?
> > >
> > > + /*
> > > + * Shared interrupts do not go well with disabling
> > > + * auto enable. The sharing interrupt might request
> > > + * it while it's still disabled and then wait for
> > > + * interrupts forever.
> > > + */
>
> Assume the following:
>
> request_irq(X, handler1, NOAUTOEN|SHARED, dev1);
>
> now the second device does:
>
> request_irq(X, handler2, SHARED, dev2):
>
> which will see the first handler installed, so it wont run into the code
> path which starts up the interrupt. That means as long as dev1 does not
> explicitely enable the interrupt dev2 will wait for it forever.
Ok, makes sense.
> > For my interrupts which are both per-CPU & shared the combination of these
> > 2>
> > facts mean I end up triggering your warning. My current ideas include:
> > - I could clear the IRQ_NOAUTOEN flag before calling setup_percpu_irq().
> > In
> >
> > my cases that should be fine - we call enable_percpu_irq() anyway, and
> > would just enable the IRQ slightly earlier on the CPU which calls
> > setup_percpu_irq() which wouldn't be a problem. It feels a bit yucky
> > though.
>
> What's the problem with IRQ_NOAUTOEN and do
>
> setup_percpu_irq();
> enable_percpu_irq();
>
> on the boot CPU and then later call it when the secondary CPUs come up in
> cpu bringup code or a hotplug state callback?
There's no problem with that at all, apart from that it triggers your warning
when the boot CPU calls setup_percpu_irq().
Thanks,
Paul
On Wed, 6 Sep 2017, Paul Burton wrote:
> On Wednesday, 6 September 2017 01:16:48 PDT Thomas Gleixner wrote:
> > What's the problem with IRQ_NOAUTOEN and do
> >
> > setup_percpu_irq();
> > enable_percpu_irq();
> >
> > on the boot CPU and then later call it when the secondary CPUs come up in
> > cpu bringup code or a hotplug state callback?
>
> There's no problem with that at all, apart from that it triggers your warning
> when the boot CPU calls setup_percpu_irq().
Ok, I misread the whole thing somehow. What you are saying is that you need
per cpu interrupts which can be shared ....
Hrmpf, ok. I'll have a look from that side then
Hi Thomas,
On Wednesday, 6 September 2017 07:14:50 PDT Thomas Gleixner wrote:
> On Wed, 6 Sep 2017, Paul Burton wrote:
> > On Wednesday, 6 September 2017 01:16:48 PDT Thomas Gleixner wrote:
> > > What's the problem with IRQ_NOAUTOEN and do
> > >
> > > setup_percpu_irq();
> > > enable_percpu_irq();
> > >
> > > on the boot CPU and then later call it when the secondary CPUs come up
> > > in
> > > cpu bringup code or a hotplug state callback?
> >
> > There's no problem with that at all, apart from that it triggers your
> > warning when the boot CPU calls setup_percpu_irq().
>
> Ok, I misread the whole thing somehow. What you are saying is that you need
> per cpu interrupts which can be shared ....
>
> Hrmpf, ok. I'll have a look from that side then
So actually it turns out that getting this to work involves more than just
avoiding your warning. I have some code which is mostly working; I'll clean up
& send it as an RFC hopefully tomorrow.
Thanks,
Paul
This series introduces support for percpu shared interrupts and makes
use of this support to clean up some hacks that have been used to
support such interrupts on MIPS.
- Patch 1 allows users of shared interrupts to opt into IRQ_NOAUTOEN
behaviour & avoid warnings from doing so.
- Patch 2 introduces support for shared percpu_devid interrupts.
- Patch 3 introduces a helper allowing users to detect whether an
interrupt is a percpu_devid interrupt or not, which is useful
during the transition phase where interrupts may be either.
- Patches 4 & 5 removes an ugly custom implementation of shared
interrupts between the MIPS cevt-r4k timer driver & users of
performance counters, in favor of using standard IRQF_SHARED &
multiple handlers.
- Patches 6 & 7 add percpu interrupt support to the MIPS perf &
cevt-r4k timer drivers respectively.
- Patch 8 configures the MIPS cop 0 count/compare, fast debug channel &
performance counter overflow interrupts as percpu_devid when they are
mapped by the irqchip-mips-cpu driver.
- Patch 9 removes a hack from the irqchip-mips-gic driver that was
used to enable & disable an interrupt across all CPUs, which is no
longer necessary with users of those interrupts using the percpu
interrupt APIs correctly. This mirrors patch 8 for systems where we
map the CPU local interrupts via the GIC.
There's a little more work necessary before this could go in - the
MIPS oprofile code needs adjusting to use the percpu interrupt APIs, as
does the fast debug channel driver.
Applies atop next-20170905.
Paul Burton (9):
genirq: Allow shared interrupt users to opt into IRQ_NOAUTOEN
genirq: Support shared per_cpu_devid interrupts
genirq: Introduce irq_is_percpu_devid()
MIPS: Remove perf_irq interrupt sharing fallback
MIPS: Remove perf_irq
MIPS: perf: percpu_devid interrupt support
MIPS: cevt-r4k: percpu_devid interrupt support
irqchip: mips-cpu: Set timer, FDC & perf interrupts percpu_devid
irqchip: mips-gic: Remove gic_all_vpes_local_irq_controller
arch/mips/include/asm/time.h | 1 -
arch/mips/kernel/cevt-r4k.c | 77 ++++++++++++-----------------
arch/mips/kernel/perf_event_mipsxx.c | 71 +++++++++++----------------
arch/mips/kernel/time.c | 9 ----
arch/mips/kernel/traps.c | 2 +-
arch/mips/oprofile/op_impl.h | 2 -
arch/mips/oprofile/op_model_loongson3.c | 39 +++++++--------
arch/mips/oprofile/op_model_mipsxx.c | 10 +---
drivers/irqchip/irq-mips-cpu.c | 9 +++-
drivers/irqchip/irq-mips-gic.c | 69 +++-----------------------
include/linux/interrupt.h | 2 +
include/linux/irqdesc.h | 8 +++
kernel/irq/chip.c | 8 +--
kernel/irq/handle.c | 8 ++-
kernel/irq/manage.c | 86 +++++++++++++++++++++++++--------
kernel/irq/settings.h | 5 ++
16 files changed, 189 insertions(+), 217 deletions(-)
--
2.14.1
Shared interrupts which aren't automatically enabled during setup (ie.
which have the IRQ_NOAUTOEN flag set) can be problematic if one or more
users of the shared interrupt aren't expecting the IRQ_NOAUTOEN
behaviour. This led to a warning being added when a combination of
IRQ_NOAUTOEN & IRQF_SHARED are used by commit 04c848d39879 ("genirq:
Warn when IRQ_NOAUTOEN is used with shared interrupts").
There are however legitimate cases where a shared interrupt which isn't
automatically enabled may make sense. One such case is shared percpu
interrupts, which we don't currently support but will with subsequent
patches. For percpu interrupts the IRQ_NOAUTOEN flag is automatically
set by irq_set_percpu_devid_flags() because it would be inconsistent to
automatically enable the interrupt on the CPU that calls
setup_percpu_irq() but not on others, and anything else would at best be
an expensive operation with few legitimate uses. The use of IRQ_NOAUTOEN
means that percpu interrupts cannot currently be shared without running
into the warning that commit 04c848d39879 ("genirq: Warn when
IRQ_NOAUTOEN is used with shared interrupts") introduced.
This patch allows for the future possibility of shared interrupts that
are not automatically enabled, by introducing an IRQF_NOAUTOEN flag
which users of a shared interrupt can set to state that they are
prepared for the IRQ_NOAUTOEN behaviour. We then require that all
actions associated with the shared interrupt share the same value for
the IRQF_NOAUTOEN flag, and that if IRQ_NOAUTOEN is set before any
actions with IRQF_SHARED are associated with the interrupt then any
actions without the IRQF_NOAUTOEN flag set are rejected, with
__setup_irq() returning -EINVAL.
In some ways this means that we go further than commit 04c848d39879
("genirq: Warn when IRQ_NOAUTOEN is used with shared interrupts") did,
in that any existing users of shared interrupts with IRQ_NOAUTOEN won't
only trigger a warning but will fail to setup their interrupt handler
entirely. In others this leaves us with an out for legitimate users
which will be able to opt-in to the IRQ_NOAUTOEN behaviour by setting
IRQF_NOAUTOEN.
Signed-off-by: Paul Burton <[email protected]>
Cc: James Hogan <[email protected]>
Cc: Jason Cooper <[email protected]>
Cc: Marc Zyngier <[email protected]>
Cc: Ralf Baechle <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: [email protected]
Cc: [email protected]
---
include/linux/interrupt.h | 2 ++
kernel/irq/manage.c | 58 ++++++++++++++++++++++++++++++++++++-----------
kernel/irq/settings.h | 5 ++++
3 files changed, 52 insertions(+), 13 deletions(-)
diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
index 59ba11661b6e..a27e22275ca7 100644
--- a/include/linux/interrupt.h
+++ b/include/linux/interrupt.h
@@ -62,6 +62,7 @@
* interrupt handler after suspending interrupts. For system
* wakeup devices users need to implement wakeup detection in
* their interrupt handlers.
+ * IRQF_NOAUTOEN - Don't automatically enable the interrupt during setup.
*/
#define IRQF_SHARED 0x00000080
#define IRQF_PROBE_SHARED 0x00000100
@@ -75,6 +76,7 @@
#define IRQF_NO_THREAD 0x00010000
#define IRQF_EARLY_RESUME 0x00020000
#define IRQF_COND_SUSPEND 0x00040000
+#define IRQF_NOAUTOEN 0x00080000
#define IRQF_TIMER (__IRQF_TIMER | IRQF_NO_SUSPEND | IRQF_NO_THREAD)
diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
index 573dc52b0806..fb5445a4a359 100644
--- a/kernel/irq/manage.c
+++ b/kernel/irq/manage.c
@@ -1229,15 +1229,19 @@ __setup_irq(unsigned int irq, struct irq_desc *desc, struct irqaction *new)
* agree on ONESHOT.
*/
unsigned int oldtype = irqd_get_trigger_type(&desc->irq_data);
+ unsigned int must_match;
+
+ /*
+ * These flags must have the same value for all actions
+ * registered for a shared interrupt.
+ */
+ must_match = IRQF_ONESHOT |
+ IRQF_PERCPU |
+ IRQF_NOAUTOEN;
if (!((old->flags & new->flags) & IRQF_SHARED) ||
(oldtype != (new->flags & IRQF_TRIGGER_MASK)) ||
- ((old->flags ^ new->flags) & IRQF_ONESHOT))
- goto mismatch;
-
- /* All handlers must agree on per-cpuness */
- if ((old->flags & IRQF_PERCPU) !=
- (new->flags & IRQF_PERCPU))
+ ((old->flags ^ new->flags) & must_match))
goto mismatch;
/* add new interrupt at end of irq queue */
@@ -1313,6 +1317,38 @@ __setup_irq(unsigned int irq, struct irq_desc *desc, struct irqaction *new)
goto out_unlock;
}
+ /*
+ * Using shared interrupts with the IRQ_NOAUTOEN flag can be risky if
+ * the flag is set when one or more of the drivers sharing the IRQ
+ * don't expect it. For example if driver A does:
+ *
+ * irq_set_status_flags(shared_irq, IRQ_NOAUTOEN);
+ * request_irq(shared_irq, handler_a, IRQF_SHARED, "a", dev_a);
+ *
+ * Then driver B does:
+ *
+ * request_irq(shared_irq, handler_b, IRQF_SHARED, "b", dev_b);
+ *
+ * Driver B has no idea that driver A set the IRQ_NOAUTOEN flag, and
+ * thus may expect that the shared_irq is enabled after its call to
+ * request_irq(). It may then miss interrupts that it was expecting to
+ * receive.
+ *
+ * We therefore require that if a shared IRQ is used with IRQ_NOAUTOEN
+ * then all drivers sharing it explicitly declare that they are aware
+ * of the fact that the interrupt won't be automatically enabled, by
+ * setting the IRQF_NOAUTOEN flag in their struct irqaction or
+ * providing it to request_irq().
+ */
+ if (WARN((new->flags && IRQF_SHARED) &&
+ !irq_settings_can_autoenable(desc) &&
+ !(new->flags & IRQF_NOAUTOEN),
+ "shared irq %d isn't automatically enabled, but the caller doesn't set IRQF_NOAUTOEN",
+ irq)) {
+ ret = -EINVAL;
+ goto out_unlock;
+ }
+
if (!shared) {
init_waitqueue_head(&desc->wait_for_threads);
@@ -1343,16 +1379,12 @@ __setup_irq(unsigned int irq, struct irq_desc *desc, struct irqaction *new)
irqd_set(&desc->irq_data, IRQD_NO_BALANCING);
}
+ if (new->flags & IRQF_NOAUTOEN)
+ irq_settings_set_noautoenable(desc);
+
if (irq_settings_can_autoenable(desc)) {
irq_startup(desc, IRQ_RESEND, IRQ_START_COND);
} else {
- /*
- * Shared interrupts do not go well with disabling
- * auto enable. The sharing interrupt might request
- * it while it's still disabled and then wait for
- * interrupts forever.
- */
- WARN_ON_ONCE(new->flags & IRQF_SHARED);
/* Undo nested disables: */
desc->depth = 1;
}
diff --git a/kernel/irq/settings.h b/kernel/irq/settings.h
index 320579d89091..97edef1bd781 100644
--- a/kernel/irq/settings.h
+++ b/kernel/irq/settings.h
@@ -147,6 +147,11 @@ static inline bool irq_settings_can_autoenable(struct irq_desc *desc)
return !(desc->status_use_accessors & _IRQ_NOAUTOEN);
}
+static inline void irq_settings_set_noautoenable(struct irq_desc *desc)
+{
+ desc->status_use_accessors |= _IRQ_NOAUTOEN;
+}
+
static inline bool irq_settings_is_nested_thread(struct irq_desc *desc)
{
return desc->status_use_accessors & _IRQ_NESTED_THREAD;
--
2.14.1
Up until now per_cpu_devid interrupts have not supported sharing. On
MIPS we have some percpu interrupts which are shared in many systems -
a single CPU interrupt line may be used to indicate a timer interrupt,
performance counter interrupt or fast debug channel interrupt. We have
up until now supported this with a series of hacks, wherein drivers call
each other's interrupt handlers & our MIPS GIC irqchip driver includes a
hack which configures the interrupt(s) for all CPUs. In order to allow
this mess to be cleaned up, this patch introduces support for shared
per_cpu_devid interrupts.
The major portion of this is supporting per_cpu_devid interrupts in
__handle_irq_event_percpu() and then making use of this, via
handle_irq_event_percpu(), from handler_percpu_devif_irq() to invoke the
handler for all actions associated with the shared interrupt. This does
have a few side effects worth noting:
- per_cpu_devid interrupts will now add to the entropy pool via
add_interrupt_randomness(), where they previously did not.
- per_cpu_devid interrupts will record timings when IRQS_TIMINGS is
set, via record_irq_time(), where they previously did not.
- per_cpu_devid interrupts will handle an IRQ_WAKE_THREAD return from
their handlers to wake a thread, where they previously did not.
I'm not aware of any reason the above should be bad side effects, so
sharing __handle_irq_event_percpu() for per_cpu_devid interrupts seems
like a positive.
The other area that requires work for shared per_cpu_devid interrupts is
__free_percpu_irq() which is adjusted to support removing the correct
struct irqaction from the list pointed to by the action field of struct
irqdesc, where it previously presumed only one action is present. The
new behaviour mirrors that of __free_irq().
Signed-off-by: Paul Burton <[email protected]>
Cc: James Hogan <[email protected]>
Cc: Jason Cooper <[email protected]>
Cc: Marc Zyngier <[email protected]>
Cc: Ralf Baechle <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: [email protected]
Cc: [email protected]
---
kernel/irq/chip.c | 8 ++------
kernel/irq/handle.c | 8 +++++++-
kernel/irq/manage.c | 28 +++++++++++++++++++++-------
3 files changed, 30 insertions(+), 14 deletions(-)
diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
index f51b7b6d2451..063a125059b5 100644
--- a/kernel/irq/chip.c
+++ b/kernel/irq/chip.c
@@ -859,7 +859,6 @@ void handle_percpu_irq(struct irq_desc *desc)
void handle_percpu_devid_irq(struct irq_desc *desc)
{
struct irq_chip *chip = irq_desc_get_chip(desc);
- struct irqaction *action = desc->action;
unsigned int irq = irq_desc_get_irq(desc);
irqreturn_t res;
@@ -868,11 +867,8 @@ void handle_percpu_devid_irq(struct irq_desc *desc)
if (chip->irq_ack)
chip->irq_ack(&desc->irq_data);
- if (likely(action)) {
- trace_irq_handler_entry(irq, action);
- res = action->handler(irq, raw_cpu_ptr(action->percpu_dev_id));
- trace_irq_handler_exit(irq, action, res);
- } else {
+ res = handle_irq_event_percpu(desc);
+ if (unlikely(res == IRQ_NONE)) {
unsigned int cpu = smp_processor_id();
bool enabled = cpumask_test_cpu(cpu, desc->percpu_enabled);
diff --git a/kernel/irq/handle.c b/kernel/irq/handle.c
index 79f987b942b8..f0309679f2c8 100644
--- a/kernel/irq/handle.c
+++ b/kernel/irq/handle.c
@@ -142,9 +142,15 @@ irqreturn_t __handle_irq_event_percpu(struct irq_desc *desc, unsigned int *flags
for_each_action_of_desc(desc, action) {
irqreturn_t res;
+ void *dev_id;
+
+ if (irq_settings_is_per_cpu_devid(desc))
+ dev_id = raw_cpu_ptr(action->percpu_dev_id);
+ else
+ dev_id = action->dev_id;
trace_irq_handler_entry(irq, action);
- res = action->handler(irq, action->dev_id);
+ res = action->handler(irq, dev_id);
trace_irq_handler_exit(irq, action, res);
if (WARN_ONCE(!irqs_disabled(),"irq %u handler %pF enabled interrupts\n",
diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
index fb5445a4a359..6b8a34971a0f 100644
--- a/kernel/irq/manage.c
+++ b/kernel/irq/manage.c
@@ -1929,7 +1929,7 @@ EXPORT_SYMBOL_GPL(disable_percpu_irq);
static struct irqaction *__free_percpu_irq(unsigned int irq, void __percpu *dev_id)
{
struct irq_desc *desc = irq_to_desc(irq);
- struct irqaction *action;
+ struct irqaction *action, **action_ptr;
unsigned long flags;
WARN(in_interrupt(), "Trying to free IRQ %d from IRQ context!\n", irq);
@@ -1939,20 +1939,34 @@ static struct irqaction *__free_percpu_irq(unsigned int irq, void __percpu *dev_
raw_spin_lock_irqsave(&desc->lock, flags);
- action = desc->action;
- if (!action || action->percpu_dev_id != dev_id) {
- WARN(1, "Trying to free already-free IRQ %d\n", irq);
- goto bad;
+ /*
+ * There can be multiple actions per IRQ descriptor, find the right
+ * one based on the dev_id:
+ */
+ action_ptr = &desc->action;
+ for (;;) {
+ action = *action_ptr;
+
+ if (!action) {
+ WARN(1, "Trying to free already-free IRQ %d\n", irq);
+ goto bad;
+ }
+
+ if (action->percpu_dev_id == dev_id)
+ break;
+ action_ptr = &action->next;
}
- if (!cpumask_empty(desc->percpu_enabled)) {
+ if ((action_ptr == &desc->action) &&
+ !action->next &&
+ !cpumask_empty(desc->percpu_enabled)) {
WARN(1, "percpu IRQ %d still enabled on CPU%d!\n",
irq, cpumask_first(desc->percpu_enabled));
goto bad;
}
/* Found it - now remove it from the list of entries: */
- desc->action = NULL;
+ *action_ptr = action->next;
raw_spin_unlock_irqrestore(&desc->lock, flags);
--
2.14.1
In preparation for allowing code to handle both percpu_devid interrupts
using the percpu interrupt APIs, and non-percpu_devid but still percpu
interrupts with the regular interrupt APIs, introduce a new
irq_is_percpu_devid() helper function to allow callers to check whether
an interrupt has the IRQ_PER_CPU_DEVID flag set.
Signed-off-by: Paul Burton <[email protected]>
Cc: James Hogan <[email protected]>
Cc: Jason Cooper <[email protected]>
Cc: Marc Zyngier <[email protected]>
Cc: Ralf Baechle <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: [email protected]
Cc: [email protected]
---
include/linux/irqdesc.h | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/include/linux/irqdesc.h b/include/linux/irqdesc.h
index 3e90a094798d..93960cf36e23 100644
--- a/include/linux/irqdesc.h
+++ b/include/linux/irqdesc.h
@@ -244,6 +244,14 @@ static inline int irq_is_percpu(unsigned int irq)
return desc->status_use_accessors & IRQ_PER_CPU;
}
+static inline int irq_is_percpu_devid(unsigned int irq)
+{
+ struct irq_desc *desc;
+
+ desc = irq_to_desc(irq);
+ return desc->status_use_accessors & IRQ_PER_CPU_DEVID;
+}
+
static inline void
irq_set_lockdep_class(unsigned int irq, struct lock_class_key *class)
{
--
2.14.1
Since commit a1ec0e188330 ("MIPS: perf: Allow sharing IRQ with timer")
we have supported registering our performance counter overflow IRQ
handler using the IRQF_SHARED flag when cp0_perfcount_irq >= 0 or
get_c0_perfcount_int() is implemented & returns a valid interrupt. This
was made unconditional for MIPSr2 & beyond by commit 4a91d8fb61e2
("MIPS: Allow shared IRQ for timer & perf counter") which removed a
special case that set cp0_perfcount_irq to -1 if the performance counter
overflow & timer interrupts share a CPU interrupt pin, however for
pre-r2 systems we retained a fallback wherein we don't register the perf
IRQ handler & instead set a perf_irq function pointer which is called by
the timer driver.
Commit 4a91d8fb61e2 ("MIPS: Allow shared IRQ for timer & perf counter")
seems to suggest that this is necessary because the we can't decode
which interrupt happened on pre-r2 systems, but this is not true - in
this case __handle_irq_event_percpu() will simply invoke both the timer
driver & perf interrupt handlers which is exactly what we want, and
they'll simply both perform their work as they do now.
As such we can set cp0_perfcount_irq = cp0_compare_irq for pre-r2
systems and remove the perf_irq fallback in favor of always relying on
more standard interrupt sharing using IRQF_SHARED & multiple handlers.
A natural cleanup that ties in with no longer using perf_irq is that we
can remove mipsxx_pmu_handle_shared_irq() which we previously pointed
perf_irq at, and effectively inline it in mipsxx_pmu_handle_irq().
In the the loongson3 oprofile case the driver had exclusively relied
upon perf_irq, and we switch instead to calling request_irq() to
register the shared handler just like the mipsxx perf & oprofile code.
Signed-off-by: Paul Burton <[email protected]>
Cc: James Hogan <[email protected]>
Cc: Jason Cooper <[email protected]>
Cc: Marc Zyngier <[email protected]>
Cc: Ralf Baechle <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: [email protected]
Cc: [email protected]
---
arch/mips/kernel/perf_event_mipsxx.c | 59 +++++++++++----------------------
arch/mips/kernel/traps.c | 2 +-
arch/mips/oprofile/op_model_loongson3.c | 39 +++++++++++-----------
arch/mips/oprofile/op_model_mipsxx.c | 10 ++----
4 files changed, 42 insertions(+), 68 deletions(-)
diff --git a/arch/mips/kernel/perf_event_mipsxx.c b/arch/mips/kernel/perf_event_mipsxx.c
index 9e6c74bf66c4..cae36ca400e9 100644
--- a/arch/mips/kernel/perf_event_mipsxx.c
+++ b/arch/mips/kernel/perf_event_mipsxx.c
@@ -25,7 +25,7 @@
#include <asm/irq.h>
#include <asm/irq_regs.h>
#include <asm/stacktrace.h>
-#include <asm/time.h> /* For perf_irq */
+#include <asm/time.h>
#define MIPS_MAX_HWEVENTS 4
#define MIPS_TCS_PER_COUNTER 2
@@ -167,7 +167,6 @@ static unsigned int counters_total_to_per_cpu(unsigned int counters)
static void resume_local_counters(void);
static void pause_local_counters(void);
static irqreturn_t mipsxx_pmu_handle_irq(int, void *);
-static int mipsxx_pmu_handle_shared_irq(void);
static unsigned int mipsxx_pmu_swizzle_perf_idx(unsigned int idx)
{
@@ -538,44 +537,25 @@ static void mipspmu_disable(struct pmu *pmu)
static atomic_t active_events = ATOMIC_INIT(0);
static DEFINE_MUTEX(pmu_reserve_mutex);
-static int (*save_perf_irq)(void);
static int mipspmu_get_irq(void)
{
int err;
- if (mipspmu.irq >= 0) {
- /* Request my own irq handler. */
- err = request_irq(mipspmu.irq, mipsxx_pmu_handle_irq,
- IRQF_PERCPU | IRQF_NOBALANCING |
- IRQF_NO_THREAD | IRQF_NO_SUSPEND |
- IRQF_SHARED,
- "mips_perf_pmu", &mipspmu);
- if (err) {
- pr_warn("Unable to request IRQ%d for MIPS performance counters!\n",
- mipspmu.irq);
- }
- } else if (cp0_perfcount_irq < 0) {
- /*
- * We are sharing the irq number with the timer interrupt.
- */
- save_perf_irq = perf_irq;
- perf_irq = mipsxx_pmu_handle_shared_irq;
- err = 0;
- } else {
- pr_warn("The platform hasn't properly defined its interrupt controller\n");
- err = -ENOENT;
- }
-
+ err = request_irq(mipspmu.irq, mipsxx_pmu_handle_irq,
+ IRQF_PERCPU | IRQF_NOBALANCING |
+ IRQF_NO_THREAD | IRQF_NO_SUSPEND |
+ IRQF_SHARED,
+ "mips_perf_pmu", &mipspmu);
+ if (err)
+ pr_warn("Unable to request IRQ%d for MIPS performance counters!\n",
+ mipspmu.irq);
return err;
}
static void mipspmu_free_irq(void)
{
- if (mipspmu.irq >= 0)
- free_irq(mipspmu.irq, &mipspmu);
- else if (cp0_perfcount_irq < 0)
- perf_irq = save_perf_irq;
+ free_irq(mipspmu.irq, &mipspmu);
}
/*
@@ -1403,13 +1383,13 @@ static void resume_local_counters(void)
} while (ctr > 0);
}
-static int mipsxx_pmu_handle_shared_irq(void)
+static irqreturn_t mipsxx_pmu_handle_irq(int irq, void *dev)
{
struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
struct perf_sample_data data;
unsigned int counters = mipspmu.num_counters;
u64 counter;
- int handled = IRQ_NONE;
+ irqreturn_t handled = IRQ_NONE;
struct pt_regs *regs;
if (cpu_has_perf_cntr_intr_bit && !(read_c0_cause() & CAUSEF_PCI))
@@ -1462,11 +1442,6 @@ static int mipsxx_pmu_handle_shared_irq(void)
return handled;
}
-static irqreturn_t mipsxx_pmu_handle_irq(int irq, void *dev)
-{
- return mipsxx_pmu_handle_shared_irq();
-}
-
/* 24K */
#define IS_BOTH_COUNTERS_24K_EVENT(b) \
((b) == 0 || (b) == 1 || (b) == 11)
@@ -1736,6 +1711,11 @@ init_hw_perf_events(void)
else
irq = -1;
+ if (irq < 0) {
+ pr_warn("The platform hasn't properly defined its interrupt controller\n");
+ return -ENOENT;
+ }
+
mipspmu.map_raw_event = mipsxx_pmu_map_raw_event;
switch (current_cpu_type()) {
@@ -1850,9 +1830,8 @@ init_hw_perf_events(void)
on_each_cpu(reset_counters, (void *)(long)counters, 1);
- pr_cont("%s PMU enabled, %d %d-bit counters available to each "
- "CPU, irq %d%s\n", mipspmu.name, counters, counter_bits, irq,
- irq < 0 ? " (share with timer interrupt)" : "");
+ pr_cont("%s PMU enabled, %d %d-bit counters available to each CPU, irq %d",
+ mipspmu.name, counters, counter_bits, irq);
perf_pmu_register(&pmu, "cpu", PERF_TYPE_RAW);
diff --git a/arch/mips/kernel/traps.c b/arch/mips/kernel/traps.c
index 5669d3b8bd38..0fe19103e882 100644
--- a/arch/mips/kernel/traps.c
+++ b/arch/mips/kernel/traps.c
@@ -2227,7 +2227,7 @@ void per_cpu_trap_init(bool is_boot_cpu)
} else {
cp0_compare_irq = CP0_LEGACY_COMPARE_IRQ;
cp0_compare_irq_shift = CP0_LEGACY_PERFCNT_IRQ;
- cp0_perfcount_irq = -1;
+ cp0_perfcount_irq = cp0_compare_irq;
cp0_fdc_irq = -1;
}
diff --git a/arch/mips/oprofile/op_model_loongson3.c b/arch/mips/oprofile/op_model_loongson3.c
index 436b1fc99f2c..e6954ebff4a1 100644
--- a/arch/mips/oprofile/op_model_loongson3.c
+++ b/arch/mips/oprofile/op_model_loongson3.c
@@ -40,8 +40,6 @@
#define read_c0_perfhi2() __read_64bit_c0_register($25, 3)
#define write_c0_perfhi2(val) __write_64bit_c0_register($25, 3, val)
-static int (*save_perf_irq)(void);
-
static struct loongson3_register_config {
unsigned int control1;
unsigned int control2;
@@ -130,12 +128,13 @@ static void loongson3_cpu_stop(void *args)
memset(®, 0, sizeof(reg));
}
-static int loongson3_perfcount_handler(void)
+static irqreturn_t loongson3_perfcount_handler(int irq, void *dev_id)
{
unsigned long flags;
uint64_t counter1, counter2;
- uint32_t cause, handled = IRQ_NONE;
+ uint32_t cause;
struct pt_regs *regs = get_irq_regs();
+ irqreturn_t handled = IRQ_NONE;
cause = read_c0_cause();
if (!(cause & CAUSEF_PCI))
@@ -182,32 +181,34 @@ static int loongson3_dying_cpu(unsigned int cpu)
return 0;
}
+struct op_mips_model op_model_loongson3_ops = {
+ .reg_setup = loongson3_reg_setup,
+ .cpu_setup = loongson3_cpu_setup,
+ .init = loongson3_init,
+ .exit = loongson3_exit,
+ .cpu_start = loongson3_cpu_start,
+ .cpu_stop = loongson3_cpu_stop,
+ .cpu_type = "mips/loongson3",
+ .num_counters = 2
+};
+
static int __init loongson3_init(void)
{
on_each_cpu(reset_counters, NULL, 1);
cpuhp_setup_state_nocalls(CPUHP_AP_MIPS_OP_LOONGSON3_STARTING,
"mips/oprofile/loongson3:starting",
loongson3_starting_cpu, loongson3_dying_cpu);
- save_perf_irq = perf_irq;
- perf_irq = loongson3_perfcount_handler;
- return 0;
+ return request_irq(cp0_compare_irq, loongson3_perfcount_handler,
+ IRQF_PERCPU | IRQF_NOBALANCING |
+ IRQF_NO_THREAD | IRQF_NO_SUSPEND |
+ IRQF_SHARED,
+ "Perfcounter", &op_model_loongson3_ops);
}
static void loongson3_exit(void)
{
+ free_irq(cp0_compare_irq, &op_model_loongson3_ops);
on_each_cpu(reset_counters, NULL, 1);
cpuhp_remove_state_nocalls(CPUHP_AP_MIPS_OP_LOONGSON3_STARTING);
- perf_irq = save_perf_irq;
}
-
-struct op_mips_model op_model_loongson3_ops = {
- .reg_setup = loongson3_reg_setup,
- .cpu_setup = loongson3_cpu_setup,
- .init = loongson3_init,
- .exit = loongson3_exit,
- .cpu_start = loongson3_cpu_start,
- .cpu_stop = loongson3_cpu_stop,
- .cpu_type = "mips/loongson3",
- .num_counters = 2
-};
diff --git a/arch/mips/oprofile/op_model_mipsxx.c b/arch/mips/oprofile/op_model_mipsxx.c
index c3e4c18ef8d4..09cbb226c7da 100644
--- a/arch/mips/oprofile/op_model_mipsxx.c
+++ b/arch/mips/oprofile/op_model_mipsxx.c
@@ -21,7 +21,6 @@
#define M_COUNTER_OVERFLOW (1UL << 31)
-static int (*save_perf_irq)(void);
static int perfcount_irq;
/*
@@ -423,9 +422,6 @@ static int __init mipsxx_init(void)
return -ENODEV;
}
- save_perf_irq = perf_irq;
- perf_irq = mipsxx_perfcount_handler;
-
if (get_c0_perfcount_int)
perfcount_irq = get_c0_perfcount_int();
else if (cp0_perfcount_irq >= 0)
@@ -438,7 +434,7 @@ static int __init mipsxx_init(void)
IRQF_PERCPU | IRQF_NOBALANCING |
IRQF_NO_THREAD | IRQF_NO_SUSPEND |
IRQF_SHARED,
- "Perfcounter", save_perf_irq);
+ "Perfcounter", &op_model_mipsxx_ops);
return 0;
}
@@ -448,12 +444,10 @@ static void mipsxx_exit(void)
int counters = op_model_mipsxx_ops.num_counters;
if (perfcount_irq >= 0)
- free_irq(perfcount_irq, save_perf_irq);
+ free_irq(perfcount_irq, &op_model_mipsxx_ops);
counters = counters_per_cpu_to_total(counters);
on_each_cpu(reset_counters, (void *)(long)counters, 1);
-
- perf_irq = save_perf_irq;
}
struct op_mips_model op_model_mipsxx_ops = {
--
2.14.1
Remove the perf_irq function pointer which we no longer use. The
cevt-r4k clock event driver no longer needs to call it, which simplifies
c0_compare_interrupt(), and we drop its definition & declarations.
Signed-off-by: Paul Burton <[email protected]>
Cc: James Hogan <[email protected]>
Cc: Jason Cooper <[email protected]>
Cc: Marc Zyngier <[email protected]>
Cc: Ralf Baechle <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: [email protected]
Cc: [email protected]
---
arch/mips/include/asm/time.h | 1 -
arch/mips/kernel/cevt-r4k.c | 48 +++++++++-----------------------------------
arch/mips/kernel/time.c | 9 ---------
arch/mips/oprofile/op_impl.h | 2 --
4 files changed, 10 insertions(+), 50 deletions(-)
diff --git a/arch/mips/include/asm/time.h b/arch/mips/include/asm/time.h
index 17d4cd20f18c..7a21792826a6 100644
--- a/arch/mips/include/asm/time.h
+++ b/arch/mips/include/asm/time.h
@@ -45,7 +45,6 @@ extern unsigned int mips_hpt_frequency;
* The performance counter IRQ on MIPS is a close relative to the timer IRQ
* so it lives here.
*/
-extern int (*perf_irq)(void);
extern int __weak get_c0_perfcount_int(void);
/*
diff --git a/arch/mips/kernel/cevt-r4k.c b/arch/mips/kernel/cevt-r4k.c
index dd6a18bc10ab..893aa32759d9 100644
--- a/arch/mips/kernel/cevt-r4k.c
+++ b/arch/mips/kernel/cevt-r4k.c
@@ -108,54 +108,26 @@ static unsigned int calculate_min_delta(void)
DEFINE_PER_CPU(struct clock_event_device, mips_clockevent_device);
int cp0_timer_irq_installed;
-/*
- * Possibly handle a performance counter interrupt.
- * Return true if the timer interrupt should not be checked
- */
-static inline int handle_perf_irq(int r2)
-{
- /*
- * The performance counter overflow interrupt may be shared with the
- * timer interrupt (cp0_perfcount_irq < 0). If it is and a
- * performance counter has overflowed (perf_irq() == IRQ_HANDLED)
- * and we can't reliably determine if a counter interrupt has also
- * happened (!r2) then don't check for a timer interrupt.
- */
- return (cp0_perfcount_irq < 0) &&
- perf_irq() == IRQ_HANDLED &&
- !r2;
-}
-
irqreturn_t c0_compare_interrupt(int irq, void *dev_id)
{
- const int r2 = cpu_has_mips_r2_r6;
struct clock_event_device *cd;
int cpu = smp_processor_id();
/*
- * Suckage alert:
- * Before R2 of the architecture there was no way to see if a
- * performance counter interrupt was pending, so we have to run
- * the performance counter interrupt handler anyway.
+ * If we have the Cause.TI bit with which to decode whether this was in
+ * fact a timer interrupt, rather than another which shares the CPU
+ * pin, then check that & return if no timer interrupt is pending.
*/
- if (handle_perf_irq(r2))
- return IRQ_HANDLED;
+ if (cpu_has_mips_r2_r6 && !(read_c0_cause() & CAUSEF_TI))
+ return IRQ_NONE;
- /*
- * The same applies to performance counter interrupts. But with the
- * above we now know that the reason we got here must be a timer
- * interrupt. Being the paranoiacs we are we check anyway.
- */
- if (!r2 || (read_c0_cause() & CAUSEF_TI)) {
- /* Clear Count/Compare Interrupt */
- write_c0_compare(read_c0_compare());
- cd = &per_cpu(mips_clockevent_device, cpu);
- cd->event_handler(cd);
+ /* Clear Count/Compare Interrupt */
+ write_c0_compare(read_c0_compare());
- return IRQ_HANDLED;
- }
+ cd = &per_cpu(mips_clockevent_device, cpu);
+ cd->event_handler(cd);
- return IRQ_NONE;
+ return IRQ_HANDLED;
}
struct irqaction c0_compare_irqaction = {
diff --git a/arch/mips/kernel/time.c b/arch/mips/kernel/time.c
index a6ebc8135112..1090d1c11afa 100644
--- a/arch/mips/kernel/time.c
+++ b/arch/mips/kernel/time.c
@@ -49,15 +49,6 @@ int update_persistent_clock(struct timespec now)
return rtc_mips_set_mmss(now.tv_sec);
}
-static int null_perf_irq(void)
-{
- return 0;
-}
-
-int (*perf_irq)(void) = null_perf_irq;
-
-EXPORT_SYMBOL(perf_irq);
-
/*
* time_init() - it does the following things.
*
diff --git a/arch/mips/oprofile/op_impl.h b/arch/mips/oprofile/op_impl.h
index a4e758a39af4..9b0b295bdaf1 100644
--- a/arch/mips/oprofile/op_impl.h
+++ b/arch/mips/oprofile/op_impl.h
@@ -10,8 +10,6 @@
#ifndef OP_IMPL_H
#define OP_IMPL_H 1
-extern int (*perf_irq)(void);
-
/* Per-counter configuration as set via oprofilefs. */
struct op_counter_config {
unsigned long enabled;
--
2.14.1
The MIPS CPU performance counter overflow interrupt is really a percpu
interrupt, but up until now we have not used the percpu interrupt APIs
to configure & control it. In preparation for doing so, introduce
support for percpu_devid interrupts in the MIPS perf implementation.
We switch from using request_irq() to using either setup_irq() or
setup_percpu_irq() with an explicit struct irqaction such that we can
set the flags, handler & name for that struct irqaction once rather than
needing to duplicate them in calls to request_irq() and
request_percpu_irq().
The IRQF_NOAUTOEN flag is passed because percpu interrupts
automatically get IRQ_NOAUTOEN set by irq_set_percpu_devid_flags(). We
opt into accepting this behaviour & explicitly enable the interrupt in
mipspmu_enable() right after configuring the local performance counters.
Signed-off-by: Paul Burton <[email protected]>
Cc: James Hogan <[email protected]>
Cc: Jason Cooper <[email protected]>
Cc: Marc Zyngier <[email protected]>
Cc: Ralf Baechle <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: [email protected]
Cc: [email protected]
---
arch/mips/kernel/perf_event_mipsxx.c | 30 +++++++++++++++++++-----------
1 file changed, 19 insertions(+), 11 deletions(-)
diff --git a/arch/mips/kernel/perf_event_mipsxx.c b/arch/mips/kernel/perf_event_mipsxx.c
index cae36ca400e9..af7bae79dc51 100644
--- a/arch/mips/kernel/perf_event_mipsxx.c
+++ b/arch/mips/kernel/perf_event_mipsxx.c
@@ -514,6 +514,11 @@ static void mipspmu_enable(struct pmu *pmu)
write_unlock(&pmuint_rwlock);
#endif
resume_local_counters();
+
+ if (irq_is_percpu_devid(mipspmu.irq))
+ enable_percpu_irq(mipspmu.irq, IRQ_TYPE_NONE);
+ else
+ enable_irq(mipspmu.irq);
}
/*
@@ -538,24 +543,27 @@ static void mipspmu_disable(struct pmu *pmu)
static atomic_t active_events = ATOMIC_INIT(0);
static DEFINE_MUTEX(pmu_reserve_mutex);
+static struct irqaction c0_perf_irqaction = {
+ .handler = mipsxx_pmu_handle_irq,
+ .flags = IRQF_PERCPU | IRQF_TIMER | IRQF_SHARED | IRQF_NOAUTOEN,
+ .name = "mips_perf_pmu",
+ .percpu_dev_id = &mipspmu,
+};
+
static int mipspmu_get_irq(void)
{
- int err;
+ if (irq_is_percpu_devid(mipspmu.irq))
+ return setup_percpu_irq(mipspmu.irq, &c0_perf_irqaction);
- err = request_irq(mipspmu.irq, mipsxx_pmu_handle_irq,
- IRQF_PERCPU | IRQF_NOBALANCING |
- IRQF_NO_THREAD | IRQF_NO_SUSPEND |
- IRQF_SHARED,
- "mips_perf_pmu", &mipspmu);
- if (err)
- pr_warn("Unable to request IRQ%d for MIPS performance counters!\n",
- mipspmu.irq);
- return err;
+ return setup_irq(mipspmu.irq, &c0_perf_irqaction);
}
static void mipspmu_free_irq(void)
{
- free_irq(mipspmu.irq, &mipspmu);
+ if (irq_is_percpu_devid(mipspmu.irq))
+ remove_percpu_irq(mipspmu.irq, &c0_perf_irqaction);
+ else
+ remove_irq(mipspmu.irq, &c0_perf_irqaction);
}
/*
--
2.14.1
The MIPS coprocessor 0 count/compare interrupt, used by the cevt-r4k
driver, is really a percpu interrupt but up until now we have not used
the percpu interrupt APIs to configure & control it. In preparation for
doing so, introduce support for percpu_devid interrupts in cevt-r4k.
We switch from using request_irq() to using either setup_irq() or
setup_percpu_irq() with an explicit struct irqaction such that we can
set the flags, handler & name for that struct irqaction once rather than
needing to duplicate them in calls to request_irq() and
request_percpu_irq().
The IRQF_NOAUTOEN flag is passed because percpu interrupts
automatically get IRQ_NOAUTOEN set by irq_set_percpu_devid_flags(). We
opt into accepting this behaviour & explicitly enable the interrupt in
r4k_clockevent_init() which is called on all CPUs during bringup.
Signed-off-by: Paul Burton <[email protected]>
Cc: James Hogan <[email protected]>
Cc: Jason Cooper <[email protected]>
Cc: Marc Zyngier <[email protected]>
Cc: Ralf Baechle <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: [email protected]
Cc: [email protected]
---
arch/mips/kernel/cevt-r4k.c | 29 ++++++++++++++++++++++-------
1 file changed, 22 insertions(+), 7 deletions(-)
diff --git a/arch/mips/kernel/cevt-r4k.c b/arch/mips/kernel/cevt-r4k.c
index 893aa32759d9..0021fc1226f3 100644
--- a/arch/mips/kernel/cevt-r4k.c
+++ b/arch/mips/kernel/cevt-r4k.c
@@ -8,6 +8,7 @@
*/
#include <linux/clockchips.h>
#include <linux/interrupt.h>
+#include <linux/once.h>
#include <linux/percpu.h>
#include <linux/smp.h>
#include <linux/irq.h>
@@ -106,7 +107,6 @@ static unsigned int calculate_min_delta(void)
}
DEFINE_PER_CPU(struct clock_event_device, mips_clockevent_device);
-int cp0_timer_irq_installed;
irqreturn_t c0_compare_interrupt(int irq, void *dev_id)
{
@@ -136,8 +136,9 @@ struct irqaction c0_compare_irqaction = {
* IRQF_SHARED: The timer interrupt may be shared with other interrupts
* such as perf counter and FDC interrupts.
*/
- .flags = IRQF_PERCPU | IRQF_TIMER | IRQF_SHARED,
+ .flags = IRQF_PERCPU | IRQF_TIMER | IRQF_SHARED | IRQF_NOAUTOEN,
.name = "timer",
+ .percpu_dev_id = &mips_clockevent_device,
};
@@ -222,11 +223,20 @@ unsigned int __weak get_c0_compare_int(void)
return MIPS_CPU_IRQ_BASE + cp0_compare_irq;
}
+static void setup_c0_compare_int(int irq, int *err)
+{
+ if (irq_is_percpu_devid(irq))
+ *err = setup_percpu_irq(irq, &c0_compare_irqaction);
+ else
+ *err = setup_irq(irq, &c0_compare_irqaction);
+}
+
int r4k_clockevent_init(void)
{
unsigned int cpu = smp_processor_id();
struct clock_event_device *cd;
unsigned int irq, min_delta;
+ int err;
if (!cpu_has_counter || !mips_hpt_frequency)
return -ENXIO;
@@ -258,12 +268,17 @@ int r4k_clockevent_init(void)
clockevents_config_and_register(cd, mips_hpt_frequency, min_delta, 0x7fffffff);
- if (cp0_timer_irq_installed)
- return 0;
-
- cp0_timer_irq_installed = 1;
+ err = 0;
+ DO_ONCE(setup_c0_compare_int, irq, &err);
+ if (err) {
+ pr_err("Unable to setup timer IRQ %d: %d\n", irq, err);
+ return err;
+ }
- setup_irq(irq, &c0_compare_irqaction);
+ if (irq_is_percpu_devid(irq))
+ enable_percpu_irq(irq, IRQ_TYPE_NONE);
+ else
+ enable_irq(irq);
return 0;
}
--
2.14.1
The MIPS timer, fast debug channel (FDC) & performance counter overflow
interrupts are all really percpu interrupts. However up until now the
users of these interrupt haven't used the percpu interrupt APIs to
configure & control them; instead using the regular non-percpu APIs such
as request_irq(), enable_irq() etc. This has required hacks elsewhere,
and generally does not fit well with the fact that the interrupts are
actually percpu.
The users of these interrupts are now prepared for them to be used with
the percpu interrupt APIs, so set them up as percpu_devid interrupts in
order to allow these users to begin using the percpu interrupt APIs.
Signed-off-by: Paul Burton <[email protected]>
Cc: James Hogan <[email protected]>
Cc: Jason Cooper <[email protected]>
Cc: Marc Zyngier <[email protected]>
Cc: Ralf Baechle <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: [email protected]
Cc: [email protected]
---
drivers/irqchip/irq-mips-cpu.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/irqchip/irq-mips-cpu.c b/drivers/irqchip/irq-mips-cpu.c
index 66f97fde13d8..8f7de01f6f35 100644
--- a/drivers/irqchip/irq-mips-cpu.c
+++ b/drivers/irqchip/irq-mips-cpu.c
@@ -166,7 +166,14 @@ static int mips_cpu_intc_map(struct irq_domain *d, unsigned int irq,
if (cpu_has_vint)
set_vi_handler(hw, plat_irq_dispatch);
- irq_set_chip_and_handler(irq, chip, handle_percpu_irq);
+ if ((irq == cp0_compare_irq) ||
+ (irq == cp0_fdc_irq) ||
+ (irq == cp0_perfcount_irq)) {
+ irq_set_chip_and_handler(irq, chip, handle_percpu_devid_irq);
+ irq_set_percpu_devid(irq);
+ } else {
+ irq_set_chip_and_handler(irq, chip, handle_percpu_irq);
+ }
return 0;
}
--
2.14.1
The gic_all_vpes_local_irq_controller irq_chip in the MIPS GIC driver is
a hack which was necessary due to other drivers & MIPS arch code not
using the percpu interrupt APIs to configure & control interrupts which
are really percpu.
This is no longer a problem - other drivers & arch code support using
the percpu interrupt APIs so we can now remove the
gic_all_vpes_local_irq_controller hack.
Signed-off-by: Paul Burton <[email protected]>
Cc: James Hogan <[email protected]>
Cc: Jason Cooper <[email protected]>
Cc: Marc Zyngier <[email protected]>
Cc: Ralf Baechle <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: [email protected]
Cc: [email protected]
---
drivers/irqchip/irq-mips-gic.c | 69 +++++-------------------------------------
1 file changed, 7 insertions(+), 62 deletions(-)
diff --git a/drivers/irqchip/irq-mips-gic.c b/drivers/irqchip/irq-mips-gic.c
index 40159ac12ac8..99dda0618599 100644
--- a/drivers/irqchip/irq-mips-gic.c
+++ b/drivers/irqchip/irq-mips-gic.c
@@ -337,40 +337,6 @@ static struct irq_chip gic_local_irq_controller = {
.irq_unmask = gic_unmask_local_irq,
};
-static void gic_mask_local_irq_all_vpes(struct irq_data *d)
-{
- int intr = GIC_HWIRQ_TO_LOCAL(d->hwirq);
- int i;
- unsigned long flags;
-
- spin_lock_irqsave(&gic_lock, flags);
- for (i = 0; i < gic_vpes; i++) {
- write_gic_vl_other(mips_cm_vp_id(i));
- write_gic_vo_rmask(BIT(intr));
- }
- spin_unlock_irqrestore(&gic_lock, flags);
-}
-
-static void gic_unmask_local_irq_all_vpes(struct irq_data *d)
-{
- int intr = GIC_HWIRQ_TO_LOCAL(d->hwirq);
- int i;
- unsigned long flags;
-
- spin_lock_irqsave(&gic_lock, flags);
- for (i = 0; i < gic_vpes; i++) {
- write_gic_vl_other(mips_cm_vp_id(i));
- write_gic_vo_smask(BIT(intr));
- }
- spin_unlock_irqrestore(&gic_lock, flags);
-}
-
-static struct irq_chip gic_all_vpes_local_irq_controller = {
- .name = "MIPS GIC Local",
- .irq_mask = gic_mask_local_irq_all_vpes,
- .irq_unmask = gic_unmask_local_irq_all_vpes,
-};
-
static void __gic_irq_dispatch(void)
{
gic_handle_local_int(false);
@@ -471,35 +437,14 @@ static int gic_irq_domain_map(struct irq_domain *d, unsigned int virq,
return gic_shared_irq_domain_map(d, virq, hwirq, 0);
}
- switch (GIC_HWIRQ_TO_LOCAL(hwirq)) {
- case GIC_LOCAL_INT_TIMER:
- case GIC_LOCAL_INT_PERFCTR:
- case GIC_LOCAL_INT_FDC:
- /*
- * HACK: These are all really percpu interrupts, but
- * the rest of the MIPS kernel code does not use the
- * percpu IRQ API for them.
- */
- err = irq_domain_set_hwirq_and_chip(d, virq, hwirq,
- &gic_all_vpes_local_irq_controller,
- NULL);
- if (err)
- return err;
-
- irq_set_handler(virq, handle_percpu_irq);
- break;
+ err = irq_domain_set_hwirq_and_chip(d, virq, hwirq,
+ &gic_local_irq_controller,
+ NULL);
+ if (err)
+ return err;
- default:
- err = irq_domain_set_hwirq_and_chip(d, virq, hwirq,
- &gic_local_irq_controller,
- NULL);
- if (err)
- return err;
-
- irq_set_handler(virq, handle_percpu_devid_irq);
- irq_set_percpu_devid(virq);
- break;
- }
+ irq_set_handler(virq, handle_percpu_devid_irq);
+ irq_set_percpu_devid(virq);
return gic_local_irq_domain_map(d, virq, hwirq);
}
--
2.14.1
On Thu, 7 Sep 2017, Paul Burton wrote:
> Up until now per_cpu_devid interrupts have not supported sharing. On
> MIPS we have some percpu interrupts which are shared in many systems -
> a single CPU interrupt line may be used to indicate a timer interrupt,
> performance counter interrupt or fast debug channel interrupt. We have
> up until now supported this with a series of hacks, wherein drivers call
> each other's interrupt handlers & our MIPS GIC irqchip driver includes a
> hack which configures the interrupt(s) for all CPUs. In order to allow
> this mess to be cleaned up, this patch introduces support for shared
> per_cpu_devid interrupts.
>
> The major portion of this is supporting per_cpu_devid interrupts in
> __handle_irq_event_percpu() and then making use of this, via
> handle_irq_event_percpu(), from handler_percpu_devif_irq() to invoke the
> handler for all actions associated with the shared interrupt. This does
> have a few side effects worth noting:
>
> - per_cpu_devid interrupts will now add to the entropy pool via
> add_interrupt_randomness(), where they previously did not.
>
> - per_cpu_devid interrupts will record timings when IRQS_TIMINGS is
> set, via record_irq_time(), where they previously did not.
>
> - per_cpu_devid interrupts will handle an IRQ_WAKE_THREAD return from
> their handlers to wake a thread, where they previously did not.
That's broken because it lacks the magic synchronization which is described
in the comment in __irq_wake_thread().
> I'm not aware of any reason the above should be bad side effects, so
> sharing __handle_irq_event_percpu() for per_cpu_devid interrupts seems
> like a positive.
Now you are :)
The other side effect of this is the extra overhead. You add an extra
conditional into the main interrupt handling function
__handle_irq_event_percpu() and the extra loop and hoops overhead for
handle_percpu_devid_irq().
Thanks,
tglx
On Mon, 25 Sep 2017, Thomas Gleixner wrote:
> On Thu, 7 Sep 2017, Paul Burton wrote:
> > Up until now per_cpu_devid interrupts have not supported sharing. On
> > MIPS we have some percpu interrupts which are shared in many systems -
> > a single CPU interrupt line may be used to indicate a timer interrupt,
> > performance counter interrupt or fast debug channel interrupt. We have
> > up until now supported this with a series of hacks, wherein drivers call
> > each other's interrupt handlers & our MIPS GIC irqchip driver includes a
> > hack which configures the interrupt(s) for all CPUs. In order to allow
> > this mess to be cleaned up, this patch introduces support for shared
> > per_cpu_devid interrupts.
> >
> > The major portion of this is supporting per_cpu_devid interrupts in
> > __handle_irq_event_percpu() and then making use of this, via
> > handle_irq_event_percpu(), from handler_percpu_devif_irq() to invoke the
> > handler for all actions associated with the shared interrupt. This does
> > have a few side effects worth noting:
> >
> > - per_cpu_devid interrupts will now add to the entropy pool via
> > add_interrupt_randomness(), where they previously did not.
> >
> > - per_cpu_devid interrupts will record timings when IRQS_TIMINGS is
> > set, via record_irq_time(), where they previously did not.
> >
> > - per_cpu_devid interrupts will handle an IRQ_WAKE_THREAD return from
> > their handlers to wake a thread, where they previously did not.
>
> That's broken because it lacks the magic synchronization which is described
> in the comment in __irq_wake_thread().
Aside of that to make that work at all would require per cpu threads and
not a single systemwide thread.
Thanks,
tglx