From: Will C. <wc...@re...> - 2003-04-10 18:05:13
Attachments:
force_timer2.patch
|
There are cases where it is useful to force the oprofile to use timer mode even when there is performance monitoring hardware available on the processor: -testing and debugging timer int code on a machine that has supported perfmon hardware. -machines where the performance monitoring hardware does not provide a time-based measure in the performance monitoring hardware, e.g. Athlon -when doing comparisons between different processor architectures, e.g. using same software and trying to find out why one machine has worse performance. I have a small patch to the 2.5 oprofile driver which provides a oprofile_force_timer flag, that allows insmod to force the use of the timer for i386. What do people think of this patch? -Will |
From: Philippe E. <ph...@wa...> - 2003-04-10 21:47:27
|
Will Cohen wrote: > There are cases where it is useful to force the oprofile to use timer > mode even when there is performance monitoring hardware available on the > processor: > > -testing and debugging timer int code on a machine that has supported > perfmon hardware. > > -machines where the performance monitoring hardware does not provide a > time-based measure in the performance monitoring hardware, e.g. Athlon > > -when doing comparisons between different processor architectures, e.g. > using same software and trying to find out why one machine has worse > performance. - help us and user to check than on arch with cycle count we get the correct resultat > > I have a small patch to the 2.5 oprofile driver which provides a > oprofile_force_timer flag, that allows insmod to force the use of the > timer for i386. What do people think of this patch? I like it. John it's your area ? > --- linux-20030409/arch/i386/oprofile/init.c.timer 2003-04-10 10:43:30.000000000 -0400 > +++ linux-20030409/arch/i386/oprofile/init.c 2003-04-10 13:51:59.000000000 -0400 > @@ -7,6 +7,7 @@ > * @author John Levon <le...@mo...> > */ > > +#include <linux/sched.h> spurious diff ? > #include <linux/oprofile.h> > #include <linux/init.h> > > @@ -22,7 +23,7 @@ > int __init oprofile_arch_init(struct oprofile_operations ** ops) > { > #ifdef CONFIG_X86_LOCAL_APIC > - if (!nmi_init(ops)) > + if (oprofile_force_timer || (!nmi_init(ops))) > #endif > timer_init(ops); > return 0; regards, Phil |
From: William C. <wc...@nc...> - 2003-04-11 00:49:25
|
Philippe Elie wrote: > Will Cohen wrote: > >> There are cases where it is useful to force the oprofile to use timer >> mode even when there is performance monitoring hardware available on >> the processor: >> >> -testing and debugging timer int code on a machine that has supported >> perfmon hardware. >> >> -machines where the performance monitoring hardware does not provide a >> time-based measure in the performance monitoring hardware, e.g. Athlon >> >> -when doing comparisons between different processor architectures, >> e.g. using same software and trying to find out why one machine has >> worse performance. > > > - help us and user to check than on arch with cycle count we get > the correct resultat > >> >> I have a small patch to the 2.5 oprofile driver which provides a >> oprofile_force_timer flag, that allows insmod to force the use of the >> timer for i386. What do people think of this patch? > > > I like it. John it's your area ? I thought there might be some utility for it. >> --- linux-20030409/arch/i386/oprofile/init.c.timer 2003-04-10 >> 10:43:30.000000000 -0400 >> +++ linux-20030409/arch/i386/oprofile/init.c 2003-04-10 >> 13:51:59.000000000 -0400 >> @@ -7,6 +7,7 @@ >> * @author John Levon <le...@mo...> >> */ >> >> +#include <linux/sched.h> > > > spurious diff ? yes, the <linux/sched.h> was a leftover from some earlier attempts. -Will > >> #include <linux/oprofile.h> >> #include <linux/init.h> >> @@ -22,7 +23,7 @@ >> int __init oprofile_arch_init(struct oprofile_operations ** ops) >> { >> #ifdef CONFIG_X86_LOCAL_APIC >> - if (!nmi_init(ops)) >> + if (oprofile_force_timer || (!nmi_init(ops))) >> #endif >> timer_init(ops); >> return 0; > > > regards, > Phil > > > > ------------------------------------------------------- > This SF.net email is sponsored by: Etnus, makers of TotalView, The > debugger for complex code. Debugging C/C++ programs can leave you > feeling lost and disoriented. TotalView can help you find your way. > Available on major UNIX and Linux platforms. Try it free. www.etnus.com > _______________________________________________ > oprofile-list mailing list > opr...@li... > https://lists.sourceforge.net/lists/listinfo/oprofile-list > |
From: Will C. <wc...@re...> - 2003-04-17 13:52:35
Attachments:
force_timer3.patch
|
I have revised the patch based on the comments I got. I eliminated the spurious include and the I shortened the variable name to oprofile_timer. I kept the oprofile prefix because the variable is outside the module. I talked to Richard Henderson about adding the the same support to the Alpha. He is willing to add it to the oprofile support for the Alpha. I don't have an Alpha machine, so I can't really develop the code. Richard said that it would be easier for him to add the Alpha version if the patch was in the kernel. What needs to be done to get the patch into the 2.5 development kernel? -Will William Cohen wrote: > Philippe Elie wrote: > >> Will Cohen wrote: >> >>> There are cases where it is useful to force the oprofile to use timer >>> mode even when there is performance monitoring hardware available on >>> the processor: >>> >>> -testing and debugging timer int code on a machine that has supported >>> perfmon hardware. >>> >>> -machines where the performance monitoring hardware does not provide >>> a time-based measure in the performance monitoring hardware, e.g. Athlon >>> >>> -when doing comparisons between different processor architectures, >>> e.g. using same software and trying to find out why one machine has >>> worse performance. >> >> >> >> - help us and user to check than on arch with cycle count we get >> the correct resultat >> >>> >>> I have a small patch to the 2.5 oprofile driver which provides a >>> oprofile_force_timer flag, that allows insmod to force the use of the >>> timer for i386. What do people think of this patch? >> >> >> >> I like it. John it's your area ? > > > I thought there might be some utility for it. > >>> --- linux-20030409/arch/i386/oprofile/init.c.timer 2003-04-10 >>> 10:43:30.000000000 -0400 >>> +++ linux-20030409/arch/i386/oprofile/init.c 2003-04-10 >>> 13:51:59.000000000 -0400 >>> @@ -7,6 +7,7 @@ >>> * @author John Levon <le...@mo...> >>> */ >>> >>> +#include <linux/sched.h> >> >> >> >> spurious diff ? > > > yes, the <linux/sched.h> was a leftover from some earlier attempts. > > -Will > >> >>> #include <linux/oprofile.h> >>> #include <linux/init.h> >>> @@ -22,7 +23,7 @@ >>> int __init oprofile_arch_init(struct oprofile_operations ** ops) >>> { >>> #ifdef CONFIG_X86_LOCAL_APIC >>> - if (!nmi_init(ops)) >>> + if (oprofile_force_timer || (!nmi_init(ops))) >>> #endif >>> timer_init(ops); >>> return 0; >> >> >> >> regards, >> Phil >> >> >> >> ------------------------------------------------------- >> This SF.net email is sponsored by: Etnus, makers of TotalView, The >> debugger for complex code. Debugging C/C++ programs can leave you >> feeling lost and disoriented. TotalView can help you find your way. >> Available on major UNIX and Linux platforms. Try it free. www.etnus.com >> _______________________________________________ >> oprofile-list mailing list >> opr...@li... >> https://lists.sourceforge.net/lists/listinfo/oprofile-list >> > > |
From: Philippe E. <ph...@wa...> - 2003-04-17 16:03:33
|
Will Cohen wrote: > I have revised the patch based on the comments I got. I eliminated the > spurious include and the I shortened the variable name to > oprofile_timer. I kept the oprofile prefix because the variable is > outside the module. > > I talked to Richard Henderson about adding the the same support to the > Alpha. He is willing to add it to the oprofile support for the Alpha. I > don't have an Alpha machine, so I can't really develop the code. Richard > said that it would be easier for him to add the Alpha version if the > patch was in the kernel. What needs to be done to get the patch into the > 2.5 development kernel? John, diffing the four different timer_int.c in arch/ I see only spurious diff, and this diff are here because change to x86 timer_int.c are not propagated to other arch, isn't it the right way to provide this file in drivers/oprofile ? Each arch will provide only the oprofile_arch_init() entry point and the syscall wrapper, if neccessary, to cookie_lookup. And no, sorry, I only looked if it seems feasable but have no time to do it. The only problem is the submit process as you'll need acceptance from each arch maintainer $ find -name timer_int.c ./arch/i386/oprofile/timer_int.c ./arch/sparc64/oprofile/timer_int.c ./arch/ppc64/oprofile/timer_int.c ./arch/parisc/oprofile/timer_int.c needs also modification in x64 oprofile build process since it symlink needed file from i386 arch. Allowing easier port of oprofile through timer_int will be good and propagating change in timer_int.c is a pain currently. regards, Phil |
From: John L. <le...@mo...> - 2003-04-10 23:07:52
|
On Thu, Apr 10, 2003 at 02:05:01PM -0400, Will Cohen wrote: > I have a small patch to the 2.5 oprofile driver which provides a > oprofile_force_timer flag, that allows insmod to force the use of the > timer for i386. What do people think of this patch? You should add support to Alpha too. > +MODULE_PARM (oprofile_force_timer, "i"); > +MODULE_PARM_DESC(oprofile_force_timer, > + "Force use of OProfile timer mechanism"); Silly name modprobe oprofile_force_timer=1 why not just modprobe force_timer=1 Looks OK otherwise I suppose. regards john |
From: William C. <wc...@nc...> - 2003-04-11 00:40:07
|
John Levon wrote: > On Thu, Apr 10, 2003 at 02:05:01PM -0400, Will Cohen wrote: > > >>I have a small patch to the 2.5 oprofile driver which provides a >>oprofile_force_timer flag, that allows insmod to force the use of the >>timer for i386. What do people think of this patch? > > > You should add support to Alpha too. It should be relatively easy to add other architecture to the patch. I can add Alpha. >>+MODULE_PARM (oprofile_force_timer, "i"); >>+MODULE_PARM_DESC(oprofile_force_timer, >>+ "Force use of OProfile timer mechanism"); > > > Silly name > > modprobe oprofile_force_timer=1 > > why not just > > modprobe force_timer=1 I wasn't sure about the naming conventions with the symbol being visible across files and I wanted to make sure there wasn't a chance of name space conflict with some other driver that happen to have "force_timer" as a variable name. Hence the prefix "oprofile_". > > Looks OK otherwise I suppose. > > regards > john > > > ------------------------------------------------------- > This SF.net email is sponsored by: Etnus, makers of TotalView, The debugger > for complex code. Debugging C/C++ programs can leave you feeling lost and > disoriented. TotalView can help you find your way. Available on major UNIX > and Linux platforms. Try it free. www.etnus.com > _______________________________________________ > oprofile-list mailing list > opr...@li... > https://lists.sourceforge.net/lists/listinfo/oprofile-list > |
From: John L. <le...@mo...> - 2003-04-11 01:04:42
|
On Thu, Apr 10, 2003 at 08:37:52PM -0400, William Cohen wrote: > I wasn't sure about the naming conventions with the symbol being visible > across files and I wanted to make sure there wasn't a chance of name > space conflict with some other driver that happen to have "force_timer" > as a variable name. Hence the prefix "oprofile_". "some other driver" ? oprofile.h is not supposed to be included in general ... I'd be more worried about boot-time namespace though... so "oprofile_timer" perhaps ? regards john |
From: Philippe E. <ph...@wa...> - 2003-04-11 01:29:57
|
John Levon wrote: > On Thu, Apr 10, 2003 at 08:37:52PM -0400, William Cohen wrote: > > >>I wasn't sure about the naming conventions with the symbol being visible >>across files and I wanted to make sure there wasn't a chance of name >>space conflict with some other driver that happen to have "force_timer" >>as a variable name. Hence the prefix "oprofile_". > > > "some other driver" ? oprofile.h is not supposed to be included in > general ... > > I'd be more worried about boot-time namespace though... so > "oprofile_timer" perhaps ? boot-time ? it's a global var: no need to include oprofile.h to get problem, it will appear at link time, oprofile_timer is fine regards, Phil |
From: John L. <le...@mo...> - 2003-04-11 02:00:32
|
On Fri, Apr 11, 2003 at 03:34:10AM +0000, Philippe Elie wrote: > boot-time ? CONFIG_OPROFILE=y regards john |
From: John L. <le...@mo...> - 2003-05-05 22:42:22
|
On Thu, Apr 10, 2003 at 02:05:01PM -0400, Will Cohen wrote: > I have a small patch to the 2.5 oprofile driver which provides a > oprofile_force_timer flag, that allows insmod to force the use of the Can you redo the patch, and also test that it works with CONFIG_OPROFILE=y (i.e. boot-time flag). I vaguely remember that the flag is prefixed with the module name in fact. If this is true, then it can just be called "force_timer", as it's now local to oprof.c thanks john |
From: William C. <wc...@nc...> - 2003-05-06 00:16:06
|
Sure I can redo it. -Will John Levon wrote: > On Thu, Apr 10, 2003 at 02:05:01PM -0400, Will Cohen wrote: > > >>I have a small patch to the 2.5 oprofile driver which provides a >>oprofile_force_timer flag, that allows insmod to force the use of the > > > Can you redo the patch, and also test that it works with > CONFIG_OPROFILE=y (i.e. boot-time flag). I vaguely remember that the > flag is prefixed with the module name in fact. If this is true, then it > can just be called "force_timer", as it's now local to oprof.c > > thanks > john > > > ------------------------------------------------------- > This sf.net email is sponsored by:ThinkGeek > Welcome to geek heaven. > http://thinkgeek.com/sf > _______________________________________________ > oprofile-list mailing list > opr...@li... > https://lists.sourceforge.net/lists/listinfo/oprofile-list > |
From: William C. <wc...@nc...> - 2003-05-06 20:50:09
Attachments:
force_timer5.patch
|
I revised the patch to apply to the 2.5.69 kernel and added support so there is a boot line option "oprofile_timer" that will force oprofile to use the timer regardless of performance monitoring hardware. The "oprofile_timer=1" can still be used at module load to force use of the timer interrupt mechanism. -Will John Levon wrote: > On Thu, Apr 10, 2003 at 02:05:01PM -0400, Will Cohen wrote: > > >>I have a small patch to the 2.5 oprofile driver which provides a >>oprofile_force_timer flag, that allows insmod to force the use of the > > > Can you redo the patch, and also test that it works with > CONFIG_OPROFILE=y (i.e. boot-time flag). I vaguely remember that the > flag is prefixed with the module name in fact. If this is true, then it > can just be called "force_timer", as it's now local to oprof.c > > thanks > john > > > ------------------------------------------------------- > This sf.net email is sponsored by:ThinkGeek > Welcome to geek heaven. > http://thinkgeek.com/sf > _______________________________________________ > oprofile-list mailing list > opr...@li... > https://lists.sourceforge.net/lists/listinfo/oprofile-list > |
From: John L. <le...@mo...> - 2003-05-06 21:31:17
|
On Tue, May 06, 2003 at 04:45:27PM -0400, William Cohen wrote: > I revised the patch to apply to the 2.5.69 kernel and added support so > there is a boot line option "oprofile_timer" that will force oprofile to > use the timer regardless of performance monitoring hardware. The > "oprofile_timer=1" can still be used at module load to force use of the > timer interrupt mechanism. This is wrong, __setup() is obsolete, see the comment around __setup() definition. You should be using module_param() API. Note in particular, MODULE_PARAM_PREFIX in moduleparam.h. So it can simply be called "timer" or maybe "force_timer" Sorry I wasn't clearer about this. regards, john |
From: Will C. <wc...@re...> - 2003-05-09 21:33:28
Attachments:
force_timer7.patch
|
Here is a revised version using the newer macros in include/linux/modulesparam.h. I have checked that it works as a module in the 2.5.69 kernel. -Will John Levon wrote: > On Tue, May 06, 2003 at 04:45:27PM -0400, William Cohen wrote: > > >>I revised the patch to apply to the 2.5.69 kernel and added support so >>there is a boot line option "oprofile_timer" that will force oprofile to >>use the timer regardless of performance monitoring hardware. The >>"oprofile_timer=1" can still be used at module load to force use of the >>timer interrupt mechanism. > > > This is wrong, __setup() is obsolete, see the comment around __setup() > definition. > > You should be using module_param() API. Note in particular, > MODULE_PARAM_PREFIX in moduleparam.h. So it can simply be called "timer" > or maybe "force_timer" > > Sorry I wasn't clearer about this. > > regards, > john |
From: John L. <le...@mo...> - 2003-05-26 03:42:59
|
On Fri, May 09, 2003 at 05:32:50PM -0400, Will Cohen wrote: > Here is a revised version using the newer macros in I finally got round to checking it builtin. I'm soon sending your patch, modified as follows : > + if (!timer) { > + /* Architecture must fill in the interrupt ops and the > + * logical CPU type, or we can fall back to the timer > + * interrupt profiler. Fix spacing > +module_param_named(timer, timer, int, 0644); > +MODULE_PARM_DESC(timer, "force user of TIMER_INT mechanism"); "force use of timer interrupt" > + oprofile_timer= [HW] > + See drivers/oprofile/oprof.c > + And same here. Also it's "oprofile.timer" not "oprofile_timer" regards john |