From: Mark Brown broonie@linaro.org
Commit 2353c1f800 (arm: ipi raise/start/end tracing) added tracepoints for IPIs in the generic GIC driver but only added definitions for them on ARM, causing build failures on ARM64. Fix this by adding equivalent definitions for arm64.
Signed-off-by: Mark Brown broonie@linaro.org --- arch/arm64/kernel/smp.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c index 5d54e37..a28d285 100644 --- a/arch/arm64/kernel/smp.c +++ b/arch/arm64/kernel/smp.c @@ -48,6 +48,9 @@ #include <asm/tlbflush.h> #include <asm/ptrace.h>
+#define CREATE_TRACE_POINTS +#include <trace/events/arm-ipi.h> + /* * as from 2.5, kernels no longer have an init_tasks structure * so we need some other way of telling a new secondary core
Hi Mark,
I'm not really comfortable with this. Given the fact that we haven't tested on AArch64 at all, I'm a bit reluctant to ACK this.
What about this for a change?
-8<--------------------------------------------------------------------------
GIC: enable ipi raise/start/end tracing only for ARM
Commit 2353c1f800 (arm: ipi raise/start/end tracing) added tracepoints for IPIs in the generic GIC driver but only added definitions for them on ARM, causing build failures on ARM64. Fix this by restricting its use only on ARM architecture.
Signed-off-by: Liviu Dudau Liviu.Dudau@arm.com -- drivers/irqchip/irq-gic.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c index 65bc837..2171198 100644 --- a/drivers/irqchip/irq-gic.c +++ b/drivers/irqchip/irq-gic.c @@ -41,7 +41,9 @@ #include <linux/slab.h> #include <linux/irqchip/chained_irq.h> #include <linux/irqchip/arm-gic.h> +#ifdef CONFIG_ARM #include <trace/events/arm-ipi.h> +#endif
#include <asm/irq.h> #include <asm/exception.h> @@ -658,7 +660,9 @@ void gic_raise_softirq(const struct cpumask *mask, unsigned int irq)
/* Convert our logical CPU mask into a physical one. */ for_each_cpu(cpu, mask) { +#ifdef CONFIG_ARM trace_arm_ipi_send(irq, cpu); +#endif map |= gic_cpu_map[cpu]; }
-- 1.8.4
-8<---------------------------------------------------------------------------
Best regards, Liviu
On Mon, Oct 14, 2013 at 01:38:52PM +0100, Mark Brown wrote:
From: Mark Brown broonie@linaro.org
Commit 2353c1f800 (arm: ipi raise/start/end tracing) added tracepoints for IPIs in the generic GIC driver but only added definitions for them on ARM, causing build failures on ARM64. Fix this by adding equivalent definitions for arm64.
Signed-off-by: Mark Brown broonie@linaro.org
arch/arm64/kernel/smp.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c index 5d54e37..a28d285 100644 --- a/arch/arm64/kernel/smp.c +++ b/arch/arm64/kernel/smp.c @@ -48,6 +48,9 @@ #include <asm/tlbflush.h> #include <asm/ptrace.h> +#define CREATE_TRACE_POINTS +#include <trace/events/arm-ipi.h>
/*
- as from 2.5, kernels no longer have an init_tasks structure
- so we need some other way of telling a new secondary core
-- 1.8.4.rc3
On Mon, Oct 14, 2013 at 02:21:22PM +0100, Liviu.Dudau@arm.com wrote:
I'm not really comfortable with this. Given the fact that we haven't tested on AArch64 at all, I'm a bit reluctant to ACK this.
We've got test coverage on ARMv8 so if there's a substantial problem we should catch it though adding tracepoints should be very low risk given how widespread their use in the kernel is - is there some particular reason to be concerned about fagility?
What about this for a change?
I'd rather avoid the ifdefs if we can - it'd make the change harder to upstream (though of course that'll need the other tracepoints adding for aarch64 too).
On Mon, Oct 14, 2013 at 02:53:07PM +0100, Mark Brown wrote:
On Mon, Oct 14, 2013 at 02:21:22PM +0100, Liviu.Dudau@arm.com wrote:
I'm not really comfortable with this. Given the fact that we haven't tested on AArch64 at all, I'm a bit reluctant to ACK this.
We've got test coverage on ARMv8 so if there's a substantial problem we should catch it though adding tracepoints should be very low risk given how widespread their use in the kernel is - is there some particular reason to be concerned about fagility?
What about this for a change?
I'd rather avoid the ifdefs if we can - it'd make the change harder to upstream (though of course that'll need the other tracepoints adding for aarch64 too).
Hi Mark,
I'll leave it in your capable hands to decide which path you take.
If you need it for your patch:
Acked-by: Liviu Dudau Liviu.Dudau@arm.com
Regards, Liviu
Arm mailing list Arm@lists.linaro.org http://lists.linaro.org/mailman/listinfo/arm
On 14 October 2013 14:57, Liviu.Dudau@arm.com wrote:
I'll leave it in your capable hands to decide which path you take.
If you need it for your patch:
Thanks - I'll go with my patch if only because I already have it in git and it's less effort to just push it out.
On 10/14/2013 04:23 PM, Mark Brown wrote:
On 14 October 2013 14:57, <Liviu.Dudau@arm.com mailto:Liviu.Dudau@arm.com> wrote:
I'll leave it in your capable hands to decide which path you take. If you need it for your patch:
Thanks - I'll go with my patch if only because I already have it in git and it's less effort to just push it out.
Hi Mark,
I noticed your mail and I sent a very similar patch adding the tracepoints for IPI a couple of hour ago.
Regards -- Daniel
On Mon, Oct 14, 2013 at 04:32:19PM +0200, Daniel Lezcano wrote:
I noticed your mail and I sent a very similar patch adding the tracepoints for IPI a couple of hour ago.
Ah, cool - Liviu, Chris, I guess it should be OK to switch over to the upstream version once that's merged?
On Mon, Oct 14, 2013 at 05:11:11PM +0100, Mark Brown wrote:
On Mon, Oct 14, 2013 at 04:32:19PM +0200, Daniel Lezcano wrote:
I noticed your mail and I sent a very similar patch adding the tracepoints for IPI a couple of hour ago.
Ah, cool - Liviu, Chris, I guess it should be OK to switch over to the upstream version once that's merged?
Yes, that should be fine.
Best regards, Liviu
Arm mailing list Arm@lists.linaro.org http://lists.linaro.org/mailman/listinfo/arm
On Mon, 2013-10-14 at 17:11 +0100, Mark Brown wrote:
On Mon, Oct 14, 2013 at 04:32:19PM +0200, Daniel Lezcano wrote:
I noticed your mail and I sent a very similar patch adding the tracepoints for IPI a couple of hour ago.
Ah, cool - Liviu, Chris, I guess it should be OK to switch over to the upstream version once that's merged?
I'm wondering how I'm going to maintain the definitive bit.LITTLE MP branch [1] we point members/customers to.
I'm going to have to put all these fixups going into LSK into that branch too, and then that is going to have to get merged back into LSK, duplicating the commits already there. I guess that's OK, but seems like it would be cleaner if the patches went into the MP branch first.
[1] https://git.linaro.org/gitweb?p=arm/big.LITTLE/mp.git%3Ba=shortlog%3Bh=refs/...
On 10/14/2013 06:22 PM, Jon Medhurst (Tixy) wrote:
On Mon, 2013-10-14 at 17:11 +0100, Mark Brown wrote:
On Mon, Oct 14, 2013 at 04:32:19PM +0200, Daniel Lezcano wrote:
I noticed your mail and I sent a very similar patch adding the tracepoints for IPI a couple of hour ago.
Ah, cool - Liviu, Chris, I guess it should be OK to switch over to the upstream version once that's merged?
I'm wondering how I'm going to maintain the definitive bit.LITTLE MP branch [1] we point members/customers to.
I'm going to have to put all these fixups going into LSK into that branch too, and then that is going to have to get merged back into LSK, duplicating the commits already there. I guess that's OK, but seems like it would be cleaner if the patches went into the MP branch first.
[1] https://git.linaro.org/gitweb?p=arm/big.LITTLE/mp.git%3Ba=shortlog%3Bh=refs/...
Yes, sorry for that. When trying to identify the wake up sources in the idlestat tool, I noticed it was missing the IPI traces, so I went straight forward and send the patch to add these traces. Then I saw Mark's email...
-- Daniel
On Mon, 2013-10-14 at 18:35 +0200, Daniel Lezcano wrote:
On 10/14/2013 06:22 PM, Jon Medhurst (Tixy) wrote:
On Mon, 2013-10-14 at 17:11 +0100, Mark Brown wrote:
On Mon, Oct 14, 2013 at 04:32:19PM +0200, Daniel Lezcano wrote:
I noticed your mail and I sent a very similar patch adding the tracepoints for IPI a couple of hour ago.
Ah, cool - Liviu, Chris, I guess it should be OK to switch over to the upstream version once that's merged?
I'm wondering how I'm going to maintain the definitive bit.LITTLE MP branch [1] we point members/customers to.
I'm going to have to put all these fixups going into LSK into that branch too, and then that is going to have to get merged back into LSK, duplicating the commits already there. I guess that's OK, but seems like it would be cleaner if the patches went into the MP branch first.
[1] https://git.linaro.org/gitweb?p=arm/big.LITTLE/mp.git%3Ba=shortlog%3Bh=refs/...
Yes, sorry for that. When trying to identify the wake up sources in the idlestat tool, I noticed it was missing the IPI traces, so I went straight forward and send the patch to add these traces. Then I saw Mark's email...
Sorry Daniel, I wasn't suggesting you should have done anything different, you identified a missing kernel feature, implemented, and sent it upstream, all fine and as it should be :-)
My comments were aimed at people trying to manage LSK topics, and how we sort this sort of thing out. Complicated by the fact that two teams are maintaining two trees which are meant to have the same content.
On Mon, Oct 14, 2013 at 05:22:47PM +0100, Jon Medhurst (Tixy) wrote:
I'm wondering how I'm going to maintain the definitive bit.LITTLE MP branch [1] we point members/customers to.
I hadn't been aware of this until you mentioned it just now... what's the difference to the LSK topic branch?
I'm going to have to put all these fixups going into LSK into that branch too, and then that is going to have to get merged back into LSK, duplicating the commits already there. I guess that's OK, but seems like it would be cleaner if the patches went into the MP branch first.
Well, if the branch were what was synced with the LSK topic branch these merges would end up being noops unless we ended up with concurrent development (which seems relatively unlikely) - any backmerges would just be fast forwards.
On Mon, 2013-10-14 at 17:59 +0100, Mark Brown wrote:
On Mon, Oct 14, 2013 at 05:22:47PM +0100, Jon Medhurst (Tixy) wrote:
I'm wondering how I'm going to maintain the definitive bit.LITTLE MP branch [1] we point members/customers to.
I hadn't been aware of this until you mentioned it just now... what's the difference to the LSK topic branch?
It only consists of the patches that are considered part of the 'big.LITTLE MP' patchset. The LSK topic ended up with some other stuff as when LSK was first put together things were a bit hurried and vague.
I'm going to have to put all these fixups going into LSK into that branch too, and then that is going to have to get merged back into LSK, duplicating the commits already there. I guess that's OK, but seems like it would be cleaner if the patches went into the MP branch first.
Well, if the branch were what was synced with the LSK topic branch these merges would end up being noops unless we ended up with concurrent development (which seems relatively unlikely) - any backmerges would just be fast forwards.
But if the topics have different patches in?
Perhaps we'll have to reconsider again how all this is done.
On Mon, Oct 14, 2013 at 06:07:42PM +0100, Jon Medhurst (Tixy) wrote:
On Mon, 2013-10-14 at 17:59 +0100, Mark Brown wrote:
I hadn't been aware of this until you mentioned it just now... what's the difference to the LSK topic branch?
It only consists of the patches that are considered part of the 'big.LITTLE MP' patchset. The LSK topic ended up with some other stuff as when LSK was first put together things were a bit hurried and vague.
I'm not seeing much extra there except possibly the Linaro configs which shouldn't cause much issue if thy get pulled in by mistake?
linaro-kernel@lists.linaro.org