Sorry, hit the wrong key. 'T' and 'Y' are too close together. (mutt)
----- Forwarded message from Russell King <[email protected]> -----
Date: Fri, 28 Dec 2007 19:57:24 +0000
From: Russell King <[email protected]>
To: Adrian Bunk <[email protected]>
Cc: Mathieu Desnoyers <[email protected]>,
Randy Dunlap <[email protected]>, [email protected],
[email protected], [email protected]
Subject: Re: [2.6.24 patch] restore ARMv6 OProfile support
On Fri, Dec 28, 2007 at 08:56:36PM +0200, Adrian Bunk wrote:
> This patch restores the ARMv6 OProfile support that was killed by
> commit 09cadedbdc01f1a4bea1f427d4fb4642eaa19da9.
>
> Signed-off-by: Adrian Bunk <[email protected]>
Given where we are in the release cycle, I think this "cleanup" patch
should be reverted. Once the issues with it are resolved (why was it
never even CC'd to the arch maintainers for review?) then it can go
into mainline.
Note that it also looks like (from the commitdiff) that the above
mentioned commit also removes:
CONFIG_HARDWARE_PM (blackfin)
CONFIG_OPROFILE_CELL (powerpc)
So they're probably subtly broken as well.
Linus, what do you think? Should 09cadedbdc01f1a4bea1f427d4fb4642eaa19da9
be reverted?
>
> ---
>
> kernel/Kconfig.instrumentation | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
>
> 7fc221ef169610b5eac98e2ddd641811c0d53e4a
> diff --git a/kernel/Kconfig.instrumentation b/kernel/Kconfig.instrumentation
> index 468f47a..4453187 100644
> --- a/kernel/Kconfig.instrumentation
> +++ b/kernel/Kconfig.instrumentation
> @@ -29,2 +29,17 @@ config OPROFILE
>
> +config OPROFILE_ARMV6
> + bool
> + depends on OPROFILE && ARM && CPU_V6 && !SMP
> + default y
> + select OPROFILE_ARM11_CORE
> +
> +config OPROFILE_MPCORE
> + bool
> + depends on OPROFILE && ARM && CPU_V6 && SMP
> + default y
> + select OPROFILE_ARM11_CORE
> +
> +config OPROFILE_ARM11_CORE
> + bool
> +
> config KPROBES
--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of:
----- End forwarded message -----
--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of:
I never got a response on my message, but I have just receieved:
| Date: Tue, 15 Jan 2008 11:05:00 +0100
| From: Joerg Wagner <[email protected]>
| To: ARM Linux Mailing List <[email protected]>
| Subject: 2.6.24-rc7 : oprofile on MPCore broken
|
| Hello,
|
| just tried to use oprofile on 2.6.24-rc7.
| It does not detect the right processor
| (/dev/oprofile/cpu_type contains "timer").
|
| As I don't know exactly, how the string
| "arm/mpcore" from arch/arm/oprofile/op_model_mpcore.c
| gets feeded into that file, maybe someone else can help ?
So people are hitting the resulting mess created by 09cadedbdc01f1a4bea1f427d4fb4642eaa19da9.
Can we please fix this regression one way or another please?
I don't particularly like stuffing the options into some random place
in the architectures Kconfig file when they should stay along side the
instrumentation configuration entries.
On Fri, Dec 28, 2007 at 07:58:41PM +0000, Russell King wrote:
> Sorry, hit the wrong key. 'T' and 'Y' are too close together. (mutt)
>
> ----- Forwarded message from Russell King <[email protected]> -----
>
> Date: Fri, 28 Dec 2007 19:57:24 +0000
> From: Russell King <[email protected]>
> To: Adrian Bunk <[email protected]>
> Cc: Mathieu Desnoyers <[email protected]>,
> Randy Dunlap <[email protected]>, [email protected],
> [email protected], [email protected]
> Subject: Re: [2.6.24 patch] restore ARMv6 OProfile support
>
> On Fri, Dec 28, 2007 at 08:56:36PM +0200, Adrian Bunk wrote:
> > This patch restores the ARMv6 OProfile support that was killed by
> > commit 09cadedbdc01f1a4bea1f427d4fb4642eaa19da9.
> >
> > Signed-off-by: Adrian Bunk <[email protected]>
>
> Given where we are in the release cycle, I think this "cleanup" patch
> should be reverted. Once the issues with it are resolved (why was it
> never even CC'd to the arch maintainers for review?) then it can go
> into mainline.
>
> Note that it also looks like (from the commitdiff) that the above
> mentioned commit also removes:
>
> CONFIG_HARDWARE_PM (blackfin)
> CONFIG_OPROFILE_CELL (powerpc)
>
> So they're probably subtly broken as well.
>
> Linus, what do you think? Should 09cadedbdc01f1a4bea1f427d4fb4642eaa19da9
> be reverted?
>
> >
> > ---
> >
> > kernel/Kconfig.instrumentation | 19 +++++++++++++++++++
> > 1 file changed, 19 insertions(+)
> >
> > 7fc221ef169610b5eac98e2ddd641811c0d53e4a
> > diff --git a/kernel/Kconfig.instrumentation b/kernel/Kconfig.instrumentation
> > index 468f47a..4453187 100644
> > --- a/kernel/Kconfig.instrumentation
> > +++ b/kernel/Kconfig.instrumentation
> > @@ -29,2 +29,17 @@ config OPROFILE
> >
> > +config OPROFILE_ARMV6
> > + bool
> > + depends on OPROFILE && ARM && CPU_V6 && !SMP
> > + default y
> > + select OPROFILE_ARM11_CORE
> > +
> > +config OPROFILE_MPCORE
> > + bool
> > + depends on OPROFILE && ARM && CPU_V6 && SMP
> > + default y
> > + select OPROFILE_ARM11_CORE
> > +
> > +config OPROFILE_ARM11_CORE
> > + bool
> > +
> > config KPROBES
>
> --
> Russell King
> Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
> maintainer of:
>
> ----- End forwarded message -----
--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of:
On Tue, Jan 15, 2008 at 10:45:26AM +0000, Russell King wrote:
> I never got a response on my message, but I have just receieved:
>
> | Date: Tue, 15 Jan 2008 11:05:00 +0100
> | From: Joerg Wagner <[email protected]>
> | To: ARM Linux Mailing List <[email protected]>
> | Subject: 2.6.24-rc7 : oprofile on MPCore broken
> |
> | Hello,
> |
> | just tried to use oprofile on 2.6.24-rc7.
> | It does not detect the right processor
> | (/dev/oprofile/cpu_type contains "timer").
> |
> | As I don't know exactly, how the string
> | "arm/mpcore" from arch/arm/oprofile/op_model_mpcore.c
> | gets feeded into that file, maybe someone else can help ?
>
> So people are hitting the resulting mess created by 09cadedbdc01f1a4bea1f427d4fb4642eaa19da9.
> Can we please fix this regression one way or another please?
>
> I don't particularly like stuffing the options into some random place
> in the architectures Kconfig file when they should stay along side the
> instrumentation configuration entries.
Below is the patch I already sent on 28 Dec 2007 that stuffs it into
Kconfig.instrumentation.
Technically it shouldn't make any difference whether this patch or
Mathieu's patch that stuffs it into arch/arm/Kconfig gets applied, but
one of them should be applied for 2.6.24 (plus either mine or Mathieu's
fix for the blackfin HARDWARE_PM support broken by the same commit).
I found the bugs in Mathieu's commit 09cadedbdc01f1a4bea1f427d4fb4642eaa19da9,
I wrote patches that restore the status quo, Cc'ed all people even
remotely related to this issue, and I opened the Bugzilla bugs required
for getting them on the regression lists. Mathieu wants the regressions
he introduced fixed different from what my patches did and that's not a
problem for me (his patches are also OK).
What went wrong that his regression fixes did not land in Linus' tree?
cu
Adrian
<-- snip -->
This patch restores the ARMv6 OProfile support that was killed by
commit 09cadedbdc01f1a4bea1f427d4fb4642eaa19da9.
Signed-off-by: Adrian Bunk <[email protected]>
---
kernel/Kconfig.instrumentation | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
7fc221ef169610b5eac98e2ddd641811c0d53e4a
diff --git a/kernel/Kconfig.instrumentation b/kernel/Kconfig.instrumentation
index 468f47a..4453187 100644
--- a/kernel/Kconfig.instrumentation
+++ b/kernel/Kconfig.instrumentation
@@ -29,2 +29,17 @@ config OPROFILE
+config OPROFILE_ARMV6
+ bool
+ depends on OPROFILE && ARM && CPU_V6 && !SMP
+ default y
+ select OPROFILE_ARM11_CORE
+
+config OPROFILE_MPCORE
+ bool
+ depends on OPROFILE && ARM && CPU_V6 && SMP
+ default y
+ select OPROFILE_ARM11_CORE
+
+config OPROFILE_ARM11_CORE
+ bool
+
config KPROBES
* Adrian Bunk ([email protected]) wrote:
> On Tue, Jan 15, 2008 at 10:45:26AM +0000, Russell King wrote:
> > I never got a response on my message, but I have just receieved:
> >
> > | Date: Tue, 15 Jan 2008 11:05:00 +0100
> > | From: Joerg Wagner <[email protected]>
> > | To: ARM Linux Mailing List <[email protected]>
> > | Subject: 2.6.24-rc7 : oprofile on MPCore broken
> > |
> > | Hello,
> > |
> > | just tried to use oprofile on 2.6.24-rc7.
> > | It does not detect the right processor
> > | (/dev/oprofile/cpu_type contains "timer").
> > |
> > | As I don't know exactly, how the string
> > | "arm/mpcore" from arch/arm/oprofile/op_model_mpcore.c
> > | gets feeded into that file, maybe someone else can help ?
> >
> > So people are hitting the resulting mess created by 09cadedbdc01f1a4bea1f427d4fb4642eaa19da9.
> > Can we please fix this regression one way or another please?
> >
> > I don't particularly like stuffing the options into some random place
> > in the architectures Kconfig file when they should stay along side the
> > instrumentation configuration entries.
>
> Below is the patch I already sent on 28 Dec 2007 that stuffs it into
> Kconfig.instrumentation.
>
> Technically it shouldn't make any difference whether this patch or
> Mathieu's patch that stuffs it into arch/arm/Kconfig gets applied, but
> one of them should be applied for 2.6.24 (plus either mine or Mathieu's
> fix for the blackfin HARDWARE_PM support broken by the same commit).
>
> I found the bugs in Mathieu's commit 09cadedbdc01f1a4bea1f427d4fb4642eaa19da9,
> I wrote patches that restore the status quo, Cc'ed all people even
> remotely related to this issue, and I opened the Bugzilla bugs required
> for getting them on the regression lists. Mathieu wants the regressions
> he introduced fixed different from what my patches did and that's not a
> problem for me (his patches are also OK).
>
> What went wrong that his regression fixes did not land in Linus' tree?
>
Yes Adrian, and I thank you very much for all this work. I only
suggested the alternative patches so it would remove menu options nobody
really needs (historical special-cases) and make it easier to apply the
following set of patches already in -mm.
One way or another, either your or my arm and blackfin patches should
get into 2.6.24 final.
Mathieu
> cu
> Adrian
>
>
> <-- snip -->
>
>
> This patch restores the ARMv6 OProfile support that was killed by
> commit 09cadedbdc01f1a4bea1f427d4fb4642eaa19da9.
>
> Signed-off-by: Adrian Bunk <[email protected]>
>
> ---
>
> kernel/Kconfig.instrumentation | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
>
> 7fc221ef169610b5eac98e2ddd641811c0d53e4a
> diff --git a/kernel/Kconfig.instrumentation b/kernel/Kconfig.instrumentation
> index 468f47a..4453187 100644
> --- a/kernel/Kconfig.instrumentation
> +++ b/kernel/Kconfig.instrumentation
> @@ -29,2 +29,17 @@ config OPROFILE
>
> +config OPROFILE_ARMV6
> + bool
> + depends on OPROFILE && ARM && CPU_V6 && !SMP
> + default y
> + select OPROFILE_ARM11_CORE
> +
> +config OPROFILE_MPCORE
> + bool
> + depends on OPROFILE && ARM && CPU_V6 && SMP
> + default y
> + select OPROFILE_ARM11_CORE
> +
> +config OPROFILE_ARM11_CORE
> + bool
> +
> config KPROBES
--
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
On Tue, 15 Jan 2008, Russell King wrote:
>
> I don't particularly like stuffing the options into some random place
> in the architectures Kconfig file when they should stay along side the
> instrumentation configuration entries.
Well, I have to say that I don't particularly like obviously
architecture-specific stuff in an obviously non-architecture file..
I'd almost prefer to revert the thing that caused the problem, because
with Adrian's patch, I think the end result may *work*, but it's uglier
than what we started out with.
However, I think the *cleanest* solution right now may be something like
the appended. Totally untested, of course. It basically just copies the
generic kernel/Kconfig.instrumentation file into the arm directory, makes
arm use its own instead of the generic one, and removes the dependencies
on ARM in there (including all of the KPROBES entry that apparently isn't
an issue on ARM anyway). It then adds back the ARM-specific ones.
This follows the sacred rules of good code:
- generic code is either generic or not. If it's not generic, don't claim
it is.
- don't *force* people to use generic code if it doesn't suit them. Make
it available for the cases it makes sense for, but don't shoe-horn it
into cases where it doesn't work well.
So it allows the sharing of the common case and *many* architectures end
up using the generic Kconfig file, but hey, if it doesn't make sense for
ARM, it doesn't make sense for ARM. It's that simple.
But as mentioned, it's totally untested and I don't have (or really want
to have) a cross-compiling environment. And I don't care *that* much. I
just want something we can all live with.
So does something like this work for people?
Linus
---
arch/arm/Kconfig | 2 +-
arch/arm/Kconfig.instrumentation | 52 ++++++++++++++++++++++++++++++++++++++
2 files changed, 53 insertions(+), 1 deletions(-)
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index c4de2d4..3a75a0b 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1076,7 +1076,7 @@ endmenu
source "fs/Kconfig"
-source "kernel/Kconfig.instrumentation"
+source "arch/arm/Kconfig.instrumentation"
source "arch/arm/Kconfig.debug"
diff --git a/arch/arm/Kconfig.instrumentation b/arch/arm/Kconfig.instrumentation
new file mode 100644
index 0000000..63b8c6d
--- /dev/null
+++ b/arch/arm/Kconfig.instrumentation
@@ -0,0 +1,52 @@
+menuconfig INSTRUMENTATION
+ bool "Instrumentation Support"
+ default y
+ ---help---
+ Say Y here to get to see options related to performance measurement,
+ system-wide debugging, and testing. This option alone does not add any
+ kernel code.
+
+ If you say N, all options in this submenu will be skipped and
+ disabled. If you're trying to debug the kernel itself, go see the
+ Kernel Hacking menu.
+
+if INSTRUMENTATION
+
+config PROFILING
+ bool "Profiling support (EXPERIMENTAL)"
+ help
+ Say Y here to enable the extended profiling support mechanisms used
+ by profilers such as OProfile.
+
+config OPROFILE
+ tristate "OProfile system profiling (EXPERIMENTAL)"
+ depends on PROFILING && !UML
+ help
+ OProfile is a profiling system capable of profiling the
+ whole system, include the kernel, kernel modules, libraries,
+ and applications.
+
+ If unsure, say N.
+
+config OPROFILE_ARMV6
+ bool
+ depends on OPROFILE && CPU_V6 && !SMP
+ default y
+ select OPROFILE_ARM11_CORE
+
+config OPROFILE_MPCORE
+ bool
+ depends on OPROFILE && CPU_V6 && SMP
+ default y
+ select OPROFILE_ARM11_CORE
+
+config OPROFILE_ARM11_CORE
+ bool
+
+config MARKERS
+ bool "Activate markers"
+ help
+ Place an empty function call at each marker site. Can be
+ dynamically changed for a probe function.
+
+endif # INSTRUMENTATION
* Linus Torvalds <[email protected]> wrote:
> But as mentioned, it's totally untested and I don't have (or really
> want to have) a cross-compiling environment. And I don't care *that*
> much. I just want something we can all live with.
>
> So does something like this work for people?
> arch/arm/Kconfig | 2 +-
> arch/arm/Kconfig.instrumentation | 52 ++++++++++++++++++++++++++++++++++++++
> 2 files changed, 53 insertions(+), 1 deletions(-)
i like this approach better, not the least because it affects only one
architecture so late in the .24-rc cycle. Instrumentation bugs tend to
be found with a few weeks/months of delays. (because historically
instrumentation has been typically used by folks who cannot change their
kernel easily to debug it, i.e. by extension they dont run bleeding edge
kernels either.)
Acked-by: Ingo Molnar <[email protected]>
Ingo
On Tue, Jan 15, 2008 at 08:24:34AM -0800, Linus Torvalds wrote:
> So does something like this work for people?
Just tested it and the right Kconfig symbols get set again - thanks.
--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of:
* Linus Torvalds ([email protected]) wrote:
>
>
> On Tue, 15 Jan 2008, Russell King wrote:
> >
> > I don't particularly like stuffing the options into some random place
> > in the architectures Kconfig file when they should stay along side the
> > instrumentation configuration entries.
>
> Well, I have to say that I don't particularly like obviously
> architecture-specific stuff in an obviously non-architecture file..
>
> I'd almost prefer to revert the thing that caused the problem, because
> with Adrian's patch, I think the end result may *work*, but it's uglier
> than what we started out with.
>
> However, I think the *cleanest* solution right now may be something like
> the appended. Totally untested, of course. It basically just copies the
> generic kernel/Kconfig.instrumentation file into the arm directory, makes
> arm use its own instead of the generic one, and removes the dependencies
> on ARM in there (including all of the KPROBES entry that apparently isn't
> an issue on ARM anyway). It then adds back the ARM-specific ones.
>
> This follows the sacred rules of good code:
>
> - generic code is either generic or not. If it's not generic, don't claim
> it is.
>
> - don't *force* people to use generic code if it doesn't suit them. Make
> it available for the cases it makes sense for, but don't shoe-horn it
> into cases where it doesn't work well.
>
> So it allows the sharing of the common case and *many* architectures end
> up using the generic Kconfig file, but hey, if it doesn't make sense for
> ARM, it doesn't make sense for ARM. It's that simple.
>
> But as mentioned, it's totally untested and I don't have (or really want
> to have) a cross-compiling environment. And I don't care *that* much. I
> just want something we can all live with.
>
> So does something like this work for people?
>
Hi,
Well, it goes along the lines of the patch I suggested as a reply to
Adrian, with these differences :
- I still source the kernel/Kconfig.instrumentation file.
- I put back the missing OPROFILE options directly in arch/arm/Kconfig
Then end result is the same as your patch, but without the code
duplication.
Here is the patch :
Fix ARMv6 oprofile support
This patch restores the ARMv6 OProfile support that was killed by
commit 09cadedbdc01f1a4bea1f427d4fb4642eaa19da9.
It puts the config options in arch/arm/Kconfig.
Thanks to Adrian Bunk for finding this bug and providing an initial
patch.
Changelog :
Use def_bool.
Signed-off-by: Mathieu Desnoyers <[email protected]>
CC: Adrian Bunk <[email protected]>
CC: Randy Dunlap <[email protected]>
CC: [email protected]
CC: [email protected]
CC: [email protected]
---
arch/arm/Kconfig | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
Index: linux-2.6-lttng/arch/arm/Kconfig
===================================================================
--- linux-2.6-lttng.orig/arch/arm/Kconfig 2007-12-29 16:58:32.000000000 -0500
+++ linux-2.6-lttng/arch/arm/Kconfig 2007-12-29 16:59:25.000000000 -0500
@@ -130,6 +130,23 @@ config FIQ
config ARCH_MTD_XIP
bool
+if OPROFILE
+
+config OPROFILE_ARMV6
+ def_bool y
+ depends on CPU_V6 && !SMP
+ select OPROFILE_ARM11_CORE
+
+config OPROFILE_MPCORE
+ def_bool y
+ depends on CPU_V6 && SMP
+ select OPROFILE_ARM11_CORE
+
+config OPROFILE_ARM11_CORE
+ bool
+
+endif
+
config VECTORS_BASE
hex
default 0xffff0000 if MMU || CPU_HIGH_VECTOR
-
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
On Tue, Jan 15, 2008 at 08:24:34AM -0800, Linus Torvalds wrote:
>
>
> On Tue, 15 Jan 2008, Russell King wrote:
> >
> > I don't particularly like stuffing the options into some random place
> > in the architectures Kconfig file when they should stay along side the
> > instrumentation configuration entries.
>
> Well, I have to say that I don't particularly like obviously
> architecture-specific stuff in an obviously non-architecture file..
>
> I'd almost prefer to revert the thing that caused the problem, because
> with Adrian's patch, I think the end result may *work*, but it's uglier
> than what we started out with.
>
> However, I think the *cleanest* solution right now may be something like
> the appended. Totally untested, of course. It basically just copies the
> generic kernel/Kconfig.instrumentation file into the arm directory, makes
> arm use its own instead of the generic one, and removes the dependencies
> on ARM in there (including all of the KPROBES entry that apparently isn't
> an issue on ARM anyway). It then adds back the ARM-specific ones.
>
> This follows the sacred rules of good code:
>
> - generic code is either generic or not. If it's not generic, don't claim
> it is.
>
> - don't *force* people to use generic code if it doesn't suit them. Make
> it available for the cases it makes sense for, but don't shoe-horn it
> into cases where it doesn't work well.
>
> So it allows the sharing of the common case and *many* architectures end
> up using the generic Kconfig file, but hey, if it doesn't make sense for
> ARM, it doesn't make sense for ARM. It's that simple.
>
> But as mentioned, it's totally untested and I don't have (or really want
> to have) a cross-compiling environment. And I don't care *that* much. I
> just want something we can all live with.
>
> So does something like this work for people?
BTW, your patch may fix ARM, but the original commit broke blackfin as
well - it removed the Kconfig entry for HARDWARE_PM, which is still used:
$ grep HARDWARE_PM arch/blackfin/ -r
arch/blackfin/mach-common/interrupt.S:#ifdef CONFIG_HARDWARE_PM
arch/blackfin/mach-common/interrupt.S:#ifdef CONFIG_HARDWARE_PM
arch/blackfin/mach-common/irqpanic.c:#ifdef CONFIG_HARDWARE_PM
arch/blackfin/oprofile/Makefile:oprofile-$(CONFIG_HARDWARE_PM) += op_model_bf533.o
arch/blackfin/oprofile/common.c:#ifdef CONFIG_HARDWARE_PM
So blackfin also needs fixing. I don't know if blackfin people are
aware of this.
--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of:
On Tue, Jan 15, 2008 at 05:49:48PM +0000, Russell King wrote:
>...
> So blackfin also needs fixing. I don't know if blackfin people are
> aware of this.
They are:
http://lkml.org/lkml/2007/12/28/100
> Russell King
cu
Adrian
--
"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed
On Tue, 15 Jan 2008, Mathieu Desnoyers wrote:
>
> Well, it goes along the lines of the patch I suggested as a reply to
> Adrian, with these differences :
>
> - I still source the kernel/Kconfig.instrumentation file.
> - I put back the missing OPROFILE options directly in arch/arm/Kconfig
>
> Then end result is the same as your patch, but without the code
> duplication.
No it's not.
Now the config variables may all be there, but the UI for the *menu*
system is broken (ie all the ARM profiling config options are now outside
the profiling menu).
Is that menu really needed? I dunno. But since it exists, it should be
correct.
Linus
* Linus Torvalds ([email protected]) wrote:
>
>
> On Tue, 15 Jan 2008, Mathieu Desnoyers wrote:
> >
> > Well, it goes along the lines of the patch I suggested as a reply to
> > Adrian, with these differences :
> >
> > - I still source the kernel/Kconfig.instrumentation file.
> > - I put back the missing OPROFILE options directly in arch/arm/Kconfig
> >
> > Then end result is the same as your patch, but without the code
> > duplication.
>
> No it's not.
>
> Now the config variables may all be there, but the UI for the *menu*
> system is broken (ie all the ARM profiling config options are now outside
> the profiling menu).
>
> Is that menu really needed? I dunno. But since it exists, it should be
> correct.
>
> Linus
There is an "instrumentation menu removal" patchset I've submitted to
Andrew for the next release cycle that moves the instrumentation menu
content into General setup (I did this following your advice).
Furthermore, on ARM, the OPROFILE_ARMV6, OPROFILE_MPCORE and
OPROFILE_ARM11_CORE are all "bool , default y" (equivalent to the
preferred def_bool y). Unless I am grossly mistaken, this is not
supposed to show up in the menus; it's just selected when the
dependencies are met.
Mathieu
--
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
On Tue, Jan 15, 2008 at 02:07:20PM -0500, Mathieu Desnoyers wrote:
> * Linus Torvalds ([email protected]) wrote:
> >
> >
> > On Tue, 15 Jan 2008, Mathieu Desnoyers wrote:
> > >
> > > Well, it goes along the lines of the patch I suggested as a reply to
> > > Adrian, with these differences :
> > >
> > > - I still source the kernel/Kconfig.instrumentation file.
> > > - I put back the missing OPROFILE options directly in arch/arm/Kconfig
> > >
> > > Then end result is the same as your patch, but without the code
> > > duplication.
> >
> > No it's not.
> >
> > Now the config variables may all be there, but the UI for the *menu*
> > system is broken (ie all the ARM profiling config options are now outside
> > the profiling menu).
> >
> > Is that menu really needed? I dunno. But since it exists, it should be
> > correct.
> >
> > Linus
>
> There is an "instrumentation menu removal" patchset I've submitted to
> Andrew for the next release cycle that moves the instrumentation menu
> content into General setup (I did this following your advice).
>
> Furthermore, on ARM, the OPROFILE_ARMV6, OPROFILE_MPCORE and
> OPROFILE_ARM11_CORE are all "bool , default y" (equivalent to the
> preferred def_bool y). Unless I am grossly mistaken, this is not
> supposed to show up in the menus; it's just selected when the
> dependencies are met.
This was the patch:
+if OPROFILE
+
+config OPROFILE_ARMV6
+ def_bool y
+ depends on CPU_V6 && !SMP
+ select OPROFILE_ARM11_CORE
+
+config OPROFILE_MPCORE
+ def_bool y
+ depends on CPU_V6 && SMP
+ select OPROFILE_ARM11_CORE
+
+config OPROFILE_ARM11_CORE
+ bool
+
+endif
And none of these has a prompt defined so they do not show up
as a menu.
It is very easy to test if you have the patch applied.
No cross toolchin is needed - just do:
make ARCH=arm menuconfig
Sam
* Sam Ravnborg ([email protected]) wrote:
> On Tue, Jan 15, 2008 at 02:07:20PM -0500, Mathieu Desnoyers wrote:
> > * Linus Torvalds ([email protected]) wrote:
> > >
> > >
> > > On Tue, 15 Jan 2008, Mathieu Desnoyers wrote:
> > > >
> > > > Well, it goes along the lines of the patch I suggested as a reply to
> > > > Adrian, with these differences :
> > > >
> > > > - I still source the kernel/Kconfig.instrumentation file.
> > > > - I put back the missing OPROFILE options directly in arch/arm/Kconfig
> > > >
> > > > Then end result is the same as your patch, but without the code
> > > > duplication.
> > >
> > > No it's not.
> > >
> > > Now the config variables may all be there, but the UI for the *menu*
> > > system is broken (ie all the ARM profiling config options are now outside
> > > the profiling menu).
> > >
> > > Is that menu really needed? I dunno. But since it exists, it should be
> > > correct.
> > >
> > > Linus
> >
> > There is an "instrumentation menu removal" patchset I've submitted to
> > Andrew for the next release cycle that moves the instrumentation menu
> > content into General setup (I did this following your advice).
> >
> > Furthermore, on ARM, the OPROFILE_ARMV6, OPROFILE_MPCORE and
> > OPROFILE_ARM11_CORE are all "bool , default y" (equivalent to the
> > preferred def_bool y). Unless I am grossly mistaken, this is not
> > supposed to show up in the menus; it's just selected when the
> > dependencies are met.
>
> This was the patch:
> +if OPROFILE
> +
> +config OPROFILE_ARMV6
> + def_bool y
> + depends on CPU_V6 && !SMP
> + select OPROFILE_ARM11_CORE
> +
> +config OPROFILE_MPCORE
> + def_bool y
> + depends on CPU_V6 && SMP
> + select OPROFILE_ARM11_CORE
> +
> +config OPROFILE_ARM11_CORE
> + bool
> +
> +endif
>
> And none of these has a prompt defined so they do not show up
> as a menu.
> It is very easy to test if you have the patch applied.
> No cross toolchin is needed - just do:
> make ARCH=arm menuconfig
>
> Sam
Just tested with and without oprofile/MPCORE/CPU_V6 combinations and I
confirm that :
- no menu is showing up (as expected)
- the OPROFILE_* config options are selected (or not) as expected
Mathieu
--
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
On Tue, 15 Jan 2008, Mathieu Desnoyers wrote:
>
> Furthermore, on ARM, the OPROFILE_ARMV6, OPROFILE_MPCORE and
> OPROFILE_ARM11_CORE are all "bool , default y" (equivalent to the
> preferred def_bool y).
Ahh, I didn't notice that. If so, yes, it doesn't matter at all whether
it's inside the menu or not, of course.
Linus