From: will.deacon@arm.com (Will Deacon)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] RFC: ux500: add PMU resources
Date: Mon, 7 Feb 2011 11:39:18 -0000 [thread overview]
Message-ID: <000601cbc6bb$a7cf8450$f76e8cf0$@deacon@arm.com> (raw)
In-Reply-To: <AANLkTi=atjR8-gDU-9=3o5GLjj+S50-7k_myP199tp0G@mail.gmail.com>
> Here's the IPI version as well, for comparison with the other
> approach. The IRQ handling code will mask the interrupt before
> handling it, so it can't nest, can it?
It's not safe to cross-call from an IRQ handler with interrupts disabled
though as you're asking for deadlock. I think you'll need to schedule the
IPI for later, which is why you end up being nested.
> diff --git a/arch/arm/kernel/perf_event.c b/arch/arm/kernel/perf_event.c
> index 5efa264..a97e50f 100644
> --- a/arch/arm/kernel/perf_event.c
> +++ b/arch/arm/kernel/perf_event.c
> @@ -19,6 +19,7 @@
> #include <linux/platform_device.h>
> #include <linux/spinlock.h>
> #include <linux/uaccess.h>
> +#include <linux/smp.h>
[...]
> +static irqreturn_t armpmu_core2_irqret;
> +
> +/* Called via IPI on the second core */
> +static void armpmu_kick(void *data)
> +{
> + int irq = (int) data;
> +
> + armpmu_core2_irqret = armpmu->handle_irq(irq, NULL);
> + smp_wmb();
> +}
That barrier isn't going to do much. If you want to make the value
visible, you'll need a dsb().
> +static irqreturn_t armpmu_single_interrupt(int irq, void *dev)
> +{
> + irqreturn_t irqret = armpmu->handle_irq(irq, dev);
> + int err;
> +
> + if (irqret != IRQ_NONE)
> + return irqret;
> +
> + local_irq_enable();
> + err = smp_call_function_single(1, armpmu_kick, (void *) irq, true);
> + local_irq_disable();
Ah, I'd not considered re-enabling interrupts from the handler. I think that will
work, but this code doesn't belong in the main perf stuff as it's too platform
dependent.
> + if (err)
> + return irqret;
> +
> + smp_rmb();
> + return armpmu_core2_irqret;
> +}
This read barrier won't help either because there's only one shared variable.
The visibility of the return value should be guaranteed by the cross-call code
(see csd_{un}lock) so you can probably lose the barriers altogether.
Will
prev parent reply other threads:[~2011-02-07 11:39 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-01-18 22:59 [PATCH] RFC: ux500: add PMU resources Linus Walleij
2011-01-19 11:39 ` Will Deacon
2011-01-19 11:57 ` Russell King - ARM Linux
2011-01-19 13:12 ` Will Deacon
2011-02-02 9:52 ` Lee Jones
[not found] ` <-2131964397930844736@unknownmsgid>
2011-02-07 5:48 ` Rabin Vincent
2011-02-07 10:08 ` Will Deacon
2011-02-07 10:14 ` Russell King - ARM Linux
2011-02-07 5:56 ` Rabin Vincent
2011-02-07 7:55 ` Lee Jones
2011-02-07 10:04 ` Linus Walleij
2011-02-07 10:25 ` Lee Jones
2011-02-07 11:39 ` Will Deacon [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='000601cbc6bb$a7cf8450$f76e8cf0$@deacon@arm.com' \
--to=will.deacon@arm.com \
--cc=linux-arm-kernel@lists.infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.