From: Andi K. <an...@fi...> - 2010-06-04 11:51:01
|
Old patch, resubmitting. This fixes the Makefile to only install the event files for the current architecture. This shrinks a common oprofile install significantly. Can be still overriden with target= for cross builds. commit da3f45ad26de437b53a878cc744e0e2b5772349e Author: Andi Kleen <ak...@li...> Date: Fri Jun 4 13:47:34 2010 +0200 2010-06-04 Andi Kleen <an...@fi...> * events/Makefile.am (event_files): Split up. (target): Add. (event_files_{ppc,i386,x86_64,ppc64,avr32,mips,arm,alpha,ia64,ppc): Create from old event_files. Signed-off-by: Andi Kleen <ak...@li...> diff --git a/events/Makefile.am b/events/Makefile.am index 6c51f78..0605b53 100644 --- a/events/Makefile.am +++ b/events/Makefile.am @@ -1,9 +1,15 @@ -event_files = \ +target := $(shell uname -m) + +event_files_common = rtc/events rtc/unit_masks + +event_files_alpha = \ alpha/ev4/events alpha/ev4/unit_masks \ alpha/ev5/events alpha/ev5/unit_masks \ alpha/ev67/events alpha/ev67/unit_masks \ alpha/ev6/events alpha/ev6/unit_masks \ - alpha/pca56/events alpha/pca56/unit_masks \ + alpha/pca56/events alpha/pca56/unit_masks + +event_files_i386 = \ i386/athlon/events i386/athlon/unit_masks \ i386/core_2/events i386/core_2/unit_masks \ i386/p4/events i386/p4-ht/events \ @@ -17,9 +23,18 @@ event_files = \ i386/atom/events i386/atom/unit_masks \ i386/core_i7/events i386/core_i7/unit_masks \ i386/nehalem/events i386/nehalem/unit_masks \ + x86-64/hammer/events x86-64/hammer/unit_masks \ + x86-64/family10/events x86-64/family10/unit_masks \ + x86-64/family11h/events x86-64/family11h/unit_masks + +event_files_x86_64 = ${event_files_i386} + +event_files_ia64 = \ ia64/ia64/events ia64/ia64/unit_masks \ ia64/itanium2/events ia64/itanium2/unit_masks \ - ia64/itanium/events ia64/itanium/unit_masks \ + ia64/itanium/events ia64/itanium/unit_masks + +event_files_ppc64 = \ ppc64/power4/events ppc64/power4/event_mappings ppc64/power4/unit_masks \ ppc64/power5/events ppc64/power5/event_mappings ppc64/power5/unit_masks \ ppc64/power5+/events ppc64/power5+/event_mappings ppc64/power5+/unit_masks \ @@ -30,17 +45,18 @@ event_files = \ ppc64/970MP/events ppc64/970MP/event_mappings ppc64/970MP/unit_masks \ ppc64/ibm-compat-v1/events ppc64/ibm-compat-v1/event_mappings ppc64/ibm-compat-v1/unit_masks \ ppc64/pa6t/events ppc64/pa6t/event_mappings ppc64/pa6t/unit_masks \ - ppc64/cell-be/events ppc64/cell-be/unit_masks \ - rtc/events rtc/unit_masks \ - x86-64/hammer/events x86-64/hammer/unit_masks \ - x86-64/family10/events x86-64/family10/unit_masks \ - x86-64/family11h/events x86-64/family11h/unit_masks \ + ppc64/cell-be/events ppc64/cell-be/unit_masks + +event_files_arm = \ arm/xscale1/events arm/xscale1/unit_masks \ arm/xscale2/events arm/xscale2/unit_masks \ arm/armv6/events arm/armv6/unit_masks \ arm/armv7/events arm/armv7/unit_masks \ - arm/mpcore/events arm/mpcore/unit_masks \ - avr32/events avr32/unit_masks \ + arm/mpcore/events arm/mpcore/unit_masks + +event_files_avr32 = avr32/events avr32/unit_masks + +event_files_mips = \ mips/20K/events mips/20K/unit_masks \ mips/24K/events mips/24K/unit_masks \ mips/25K/events mips/25K/unit_masks \ @@ -53,12 +69,16 @@ event_files = \ mips/r12000/events mips/r12000/unit_masks \ mips/vr5432/events mips/vr5432/unit_masks \ mips/vr5500/events mips/vr5500/unit_masks \ - mips/loongson2/events mips/loongson2/unit_masks \ + mips/loongson2/events mips/loongson2/unit_masks + +event_files_ppc = \ ppc/7450/events ppc/7450/unit_masks \ ppc/e500/events ppc/e500/unit_masks \ ppc/e500v2/events ppc/e500v2/unit_masks \ ppc/e300/events ppc/e300/unit_masks +event_files := ${event_files_common} ${event_files_${target}} + install-data-local: for i in ${event_files} ; do \ dir=`dirname $$i` ; \ |
From: Maynard J. <may...@us...> - 2010-06-11 03:30:05
|
Andi Kleen wrote: > > Old patch, resubmitting. This fixes the Makefile to only > install the event files for the current architecture. This > shrinks a common oprofile install significantly. Andi, Yes, this is a great idea . . . but there are a few issues to discuss. First, concerning cross builds, that you mention below. I'm not experienced with cross builds, so maybe the answer to this question is obvious . . . but how should we inform those who do cross builds about the need for overriding 'target'? Regardless, I think a blurb in the "Installation" section of the OProfile manual might be a good idea, if only to let people know of this user-visible change to what's being installed. Second . . . Maybe no one uses this function, but with ophelp, you can specify '--cpu-type=<arch/processor_type>' and get the event list for a non-native processor. This function depends on the event files for that non-native processor being installed in the usual place. I think we could easily add a configure option to allow users to install all event files (default would just install those for the target machine). The selection of this option would set a variable that could be used in the events/Makefile.am to conditionally group the event files by architecture. There's also the issue of the ophelp program's behavior and how it's documented in its man page. If you run 'ophelp --cpu-type=<non-native-arch/processor_xyz>' with an oprofile installation that only has the events of the native architecture installed, you get the message: "oprofile: could not open unit mask description file <oprof-install-dir>/share/oprofile//i386/atom/unit_masks" Perhaps this message should be changed to something more helpful. Also, the man page and '--help' output should indicate that use of the '--cpu-type' option will display events for non-native architectures only if the user (via the new config option) chose to install *all* event files. On the other hand, maybe no one uses this function of ophelp, and no one would care if we simply took it away. I'd like to hear from community members about this. -Maynard > > Can be still overriden with target= for cross builds. > > commit da3f45ad26de437b53a878cc744e0e2b5772349e > Author: Andi Kleen <ak...@li...> > Date: Fri Jun 4 13:47:34 2010 +0200 > > 2010-06-04 Andi Kleen <an...@fi...> > > * events/Makefile.am (event_files): Split up. > (target): Add. > (event_files_{ppc,i386,x86_64,ppc64,avr32,mips,arm,alpha,ia64,ppc): > Create from old event_files. > > Signed-off-by: Andi Kleen <ak...@li...> > > diff --git a/events/Makefile.am b/events/Makefile.am > index 6c51f78..0605b53 100644 > --- a/events/Makefile.am > +++ b/events/Makefile.am > @@ -1,9 +1,15 @@ > -event_files = \ > +target := $(shell uname -m) > + > +event_files_common = rtc/events rtc/unit_masks > + > +event_files_alpha = \ > alpha/ev4/events alpha/ev4/unit_masks \ > alpha/ev5/events alpha/ev5/unit_masks \ > alpha/ev67/events alpha/ev67/unit_masks \ > alpha/ev6/events alpha/ev6/unit_masks \ > - alpha/pca56/events alpha/pca56/unit_masks \ > + alpha/pca56/events alpha/pca56/unit_masks > + > +event_files_i386 = \ > i386/athlon/events i386/athlon/unit_masks \ > i386/core_2/events i386/core_2/unit_masks \ > i386/p4/events i386/p4-ht/events \ > @@ -17,9 +23,18 @@ event_files = \ > i386/atom/events i386/atom/unit_masks \ > i386/core_i7/events i386/core_i7/unit_masks \ > i386/nehalem/events i386/nehalem/unit_masks \ > + x86-64/hammer/events x86-64/hammer/unit_masks \ > + x86-64/family10/events x86-64/family10/unit_masks \ > + x86-64/family11h/events x86-64/family11h/unit_masks > + > +event_files_x86_64 = ${event_files_i386} > + > +event_files_ia64 = \ > ia64/ia64/events ia64/ia64/unit_masks \ > ia64/itanium2/events ia64/itanium2/unit_masks \ > - ia64/itanium/events ia64/itanium/unit_masks \ > + ia64/itanium/events ia64/itanium/unit_masks > + > +event_files_ppc64 = \ > ppc64/power4/events ppc64/power4/event_mappings ppc64/power4/unit_masks \ > ppc64/power5/events ppc64/power5/event_mappings ppc64/power5/unit_masks \ > ppc64/power5+/events ppc64/power5+/event_mappings ppc64/power5+/unit_masks \ > @@ -30,17 +45,18 @@ event_files = \ > ppc64/970MP/events ppc64/970MP/event_mappings ppc64/970MP/unit_masks \ > ppc64/ibm-compat-v1/events ppc64/ibm-compat-v1/event_mappings ppc64/ibm-compat-v1/unit_masks \ > ppc64/pa6t/events ppc64/pa6t/event_mappings ppc64/pa6t/unit_masks \ > - ppc64/cell-be/events ppc64/cell-be/unit_masks \ > - rtc/events rtc/unit_masks \ > - x86-64/hammer/events x86-64/hammer/unit_masks \ > - x86-64/family10/events x86-64/family10/unit_masks \ > - x86-64/family11h/events x86-64/family11h/unit_masks \ > + ppc64/cell-be/events ppc64/cell-be/unit_masks > + > +event_files_arm = \ > arm/xscale1/events arm/xscale1/unit_masks \ > arm/xscale2/events arm/xscale2/unit_masks \ > arm/armv6/events arm/armv6/unit_masks \ > arm/armv7/events arm/armv7/unit_masks \ > - arm/mpcore/events arm/mpcore/unit_masks \ > - avr32/events avr32/unit_masks \ > + arm/mpcore/events arm/mpcore/unit_masks > + > +event_files_avr32 = avr32/events avr32/unit_masks > + > +event_files_mips = \ > mips/20K/events mips/20K/unit_masks \ > mips/24K/events mips/24K/unit_masks \ > mips/25K/events mips/25K/unit_masks \ > @@ -53,12 +69,16 @@ event_files = \ > mips/r12000/events mips/r12000/unit_masks \ > mips/vr5432/events mips/vr5432/unit_masks \ > mips/vr5500/events mips/vr5500/unit_masks \ > - mips/loongson2/events mips/loongson2/unit_masks \ > + mips/loongson2/events mips/loongson2/unit_masks > + > +event_files_ppc = \ > ppc/7450/events ppc/7450/unit_masks \ > ppc/e500/events ppc/e500/unit_masks \ > ppc/e500v2/events ppc/e500v2/unit_masks \ > ppc/e300/events ppc/e300/unit_masks > > +event_files := ${event_files_common} ${event_files_${target}} > + > install-data-local: > for i in ${event_files} ; do \ > dir=`dirname $$i` ; \ |
From: Andi K. <an...@fi...> - 2010-06-11 06:19:27
|
> > First, concerning cross builds, that you mention below. I'm not experienced with cross builds, so maybe the answer to this question is obvious . . . but how should we inform those who do cross builds about the need for overriding 'target'? Regardless, I think a blurb in the "Installation" section of the OProfile manual might be a good idea, if only to let people know of this user-visible change to what's being installed. Ok I can submit a followon patch with such a blurb. > > Second . . . Maybe no one uses this function, but with ophelp, you can specify '--cpu-type=<arch/processor_type>' and get the event list for a non-native processor. This function depends on the event files for that non-native processor being installed in the usual place. I think we could easily add a configure option to allow users to install all event files (default would just install those for the target machine). The selection of this option would set a variable that could be used in the events/Makefile.am to conditionally group the event files by architecture. I don't think that would be very useful, you couldn't set the non native processor types anyways. And on a target where you can set it you can run ophelp directly. -Andi -- ak...@li... -- Speaking for myself only. |
From: William C. <wc...@re...> - 2010-06-14 21:37:09
|
On 06/10/2010 07:25 PM, Maynard Johnson wrote: > Andi Kleen wrote: >> >> Old patch, resubmitting. This fixes the Makefile to only >> install the event files for the current architecture. This >> shrinks a common oprofile install significantly. > > Andi, > Yes, this is a great idea . . . but there are a few issues to discuss. > > First, concerning cross builds, that you mention below. I'm not experienced with cross builds, so maybe the answer to this question is obvious . . . but how should we inform those who do cross builds about the need for overriding 'target'? Regardless, I think a blurb in the "Installation" section of the OProfile manual might be a good idea, if only to let people know of this user-visible change to what's being installed. > > Second . . . Maybe no one uses this function, but with ophelp, you can specify '--cpu-type=<arch/processor_type>' and get the event list for a non-native processor. This function depends on the event files for that non-native processor being installed in the usual place. I think we could easily add a configure option to allow users to install all event files (default would just install those for the target machine). The selection of this option would set a variable that could be used in the events/Makefile.am to conditionally group the event files by architecture. > > There's also the issue of the ophelp program's behavior and how it's documented in its man page. If you run 'ophelp --cpu-type=<non-native-arch/processor_xyz>' with an oprofile installation that only has the events of the native architecture installed, you get the message: > "oprofile: could not open unit mask description file <oprof-install-dir>/share/oprofile//i386/atom/unit_masks" > > Perhaps this message should be changed to something more helpful. Also, the man page and '--help' output should indicate that use of the '--cpu-type' option will display events for non-native architectures only if the user (via the new config option) chose to install *all* event files. > > On the other hand, maybe no one uses this function of ophelp, and no one would care if we simply took it away. I'd like to hear from community members about this. > I have used the "ophelp --cpu-type=<arch/processor_type>" to get a list of events for other architectures, but not a huge loss it it isn't available. Need to add some check to ophelp to provide a clear error message when it cannot find the event/unit mask files. -Will |
From: Maynard J. <may...@us...> - 2010-06-28 14:03:25
|
On 06/24/2010 9:43 AM, Andi Kleen wrote: >> Well, as far as I can tell, the only problem we'd have in this >> regard is due to the aggregation of i386 and x86_64 in the >> events/Makefile.am. The ppc and ppc64 event files are separated in > > s390 / s390x, mips, mips64, ... can come out of uname. I don't see what s390 has to do with this since only timer-based profiling is supported on that architecture. And there are no mips64 oprofile events. So I don't grok what it is you're trying to say here. > > And you can run with linux32/setarch on powerpc64 too, then you get ppc > I suspect. If someone used this technique when starting oprofile, they'd be opening themselves up to completely unpredictable results. I stand by my original suggestion -- with the caveat that some special-casing would have to be done for the "i386" and "x86_64" arch strings. -Maynard > >> your patched events/Makefile.am -- as they should be, since they >> are, in fact, different architectures. Why are i386 and x86_64 >> lumped together? > > You can have both on the same kernel and some of the events are for i386 > and others for x86-64 for historical reasons. > > -Andi > |
From: Andi K. <an...@fi...> - 2010-07-01 20:51:07
|
On Mon, Jun 28, 2010 at 09:03:14AM -0500, Maynard Johnson wrote: > On 06/24/2010 9:43 AM, Andi Kleen wrote: > >>Well, as far as I can tell, the only problem we'd have in this > >>regard is due to the aggregation of i386 and x86_64 in the > >>events/Makefile.am. The ppc and ppc64 event files are separated in > > > >s390 / s390x, mips, mips64, ... can come out of uname. > > I don't see what s390 has to do with this since only timer-based > profiling is supported on that architecture. And there are no > mips64 oprofile events. So I don't grok what it is you're trying to > say here. uname returns mips64, ppc64 etc., the event files are mips without 64 etc. so it would not print the message. Anyways I revised the patch based on your suggestions. Please finally apply it now. -Andi * 2010-07-01 Andi Kleen <ak...@li...> utils/ophelp.c (main): Add note about non native archs. (cross_profile): Add. diff --git a/utils/ophelp.c b/utils/ophelp.c index 4086ada..0224e3e 100644 --- a/utils/ophelp.c +++ b/utils/ophelp.c @@ -14,6 +14,7 @@ #include <stdlib.h> #include <string.h> #include <limits.h> +#include <sys/utsname.h> #include "op_version.h" #include "op_events.h" @@ -359,6 +360,18 @@ static void cleanup(void) free(parsed_events); } +static int cross_profile(void) +{ + struct utsname u; + + if (!cpu_string) + return 0; + uname(&u); + if ((!strcmp(u.machine, "i386") || !strcmp(u.machine, "x86_64")) && + (!strncmp(cpu_string, "x86_64", 6) || !strncmp(cpu_string, "i386", 4))) + return 0; + return strncmp(u.machine, cpu_string, strlen(u.machine)) != 0; +} #define MAX_LINE 256 int main(int argc, char const * argv[]) @@ -386,6 +399,10 @@ int main(int argc, char const * argv[]) cpu_string ? cpu_string : "unset"); fprintf(stderr, "you should upgrade oprofile or force the " "use of timer mode\n"); + + if (cross_profile()) + fprintf(stderr, + "--cpu-type for non native architectures is not supported\n"); exit(EXIT_FAILURE); } |
From: Maynard J. <may...@us...> - 2010-06-14 22:11:10
|
William Cohen wrote: > On 06/10/2010 07:25 PM, Maynard Johnson wrote: >> Andi Kleen wrote: >>> >>> Old patch, resubmitting. This fixes the Makefile to only >>> install the event files for the current architecture. This >>> shrinks a common oprofile install significantly. >> >> Andi, >> Yes, this is a great idea . . . but there are a few issues to discuss. >> >> First, concerning cross builds, that you mention below. I'm not experienced with cross builds, so maybe the answer to this question is obvious . . . but how should we inform those who do cross builds about the need for overriding 'target'? Regardless, I think a blurb in the "Installation" section of the OProfile manual might be a good idea, if only to let people know of this user-visible change to what's being installed. >> >> Second . . . Maybe no one uses this function, but with ophelp, you can specify '--cpu-type=<arch/processor_type>' and get the event list for a non-native processor. This function depends on the event files for that non-native processor being installed in the usual place. I think we could easily add a configure option to allow users to install all event files (default would just install those for the target machine). The selection of this option would set a variable that could be used in the events/Makefile.am to conditionally group the event files by architecture. >> >> There's also the issue of the ophelp program's behavior and how it's documented in its man page. If you run 'ophelp --cpu-type=<non-native-arch/processor_xyz>' with an oprofile installation that only has the events of the native architecture installed, you get the message: >> "oprofile: could not open unit mask description file <oprof-install-dir>/share/oprofile//i386/atom/unit_masks" >> >> Perhaps this message should be changed to something more helpful. Also, the man page and '--help' output should indicate that use of the '--cpu-type' option will display events for non-native architectures only if the user (via the new config option) chose to install *all* event files. >> >> On the other hand, maybe no one uses this function of ophelp, and no one would care if we simply took it away. I'd like to hear from community members about this. >> > > I have used the "ophelp --cpu-type=<arch/processor_type>" to get a list of events for other architectures, but not a huge loss it it isn't available. Thanks for voicing your opinion, Will. I'll give John and others in the community a couple days to respond. If there's no hue and cry over this function going away, then that's what we'll do. -Maynard > > Need to add some check to ophelp to provide a clear error message when it cannot find the event/unit mask files. > > -Will > > ------------------------------------------------------------------------------ > ThinkGeek and WIRED's GeekDad team up for the Ultimate > GeekDad Father's Day Giveaway. ONE MASSIVE PRIZE to the > lucky parental unit. See the prize list and enter to win: > http://p.sf.net/sfu/thinkgeek-promo > _______________________________________________ > oprofile-list mailing list > opr...@li... > https://lists.sourceforge.net/lists/listinfo/oprofile-list |
From: Maynard J. <may...@us...> - 2010-06-17 14:38:56
|
Maynard Johnson wrote: > William Cohen wrote: >> On 06/10/2010 07:25 PM, Maynard Johnson wrote: >>> Andi Kleen wrote: >>>> >>>> Old patch, resubmitting. This fixes the Makefile to only >>>> install the event files for the current architecture. This >>>> shrinks a common oprofile install significantly. >>> >>> Andi, >>> Yes, this is a great idea . . . but there are a few issues to discuss. >>> >>> First, concerning cross builds, that you mention below. I'm not experienced with cross builds, so maybe the answer to this question is obvious . . . but how should we inform those who do cross builds about the need for overriding 'target'? Regardless, I think a blurb in the "Installation" section of the OProfile manual might be a good idea, if only to let people know of this user-visible change to what's being installed. >>> >>> Second . . . Maybe no one uses this function, but with ophelp, you can specify '--cpu-type=<arch/processor_type>' and get the event list for a non-native processor. This function depends on the event files for that non-native processor being installed in the usual place. I think we could easily add a configure option to allow users to install all event files (default would just install those for the target machine). The selection of this option would set a variable that could be used in the events/Makefile.am to conditionally group the event files by architecture. >>> >>> There's also the issue of the ophelp program's behavior and how it's documented in its man page. If you run 'ophelp --cpu-type=<non-native-arch/processor_xyz>' with an oprofile installation that only has the events of the native architecture installed, you get the message: >>> "oprofile: could not open unit mask description file <oprof-install-dir>/share/oprofile//i386/atom/unit_masks" >>> >>> Perhaps this message should be changed to something more helpful. Also, the man page and '--help' output should indicate that use of the '--cpu-type' option will display events for non-native architectures only if the user (via the new config option) chose to install *all* event files. >>> >>> On the other hand, maybe no one uses this function of ophelp, and no one would care if we simply took it away. I'd like to hear from community members about this. >>> >> >> I have used the "ophelp --cpu-type=<arch/processor_type>" to get a list of events for other architectures, but not a huge loss it it isn't available. > > Thanks for voicing your opinion, Will. I'll give John and others in the community a couple days to respond. If there's no hue and cry over this function going away, then that's what we'll do. Andi, hearing no objection, let's go ahead and remove the 'ophelp --cpu-type' option for non-native arch. I suggest fixing up ophelp:main so we recognize when a non-native arch is in the passed cpu_string, and put out a message at that point that such is not supported. Along with that change, we'd need a change to ophelp's --help output to indicate the --cpu-type is only valid for the native arch. Same for ophelp's man page. Thanks. -Maynard > > -Maynard > >> >> Need to add some check to ophelp to provide a clear error message when it cannot find the event/unit mask files. >> >> -Will >> >> ------------------------------------------------------------------------------ >> ThinkGeek and WIRED's GeekDad team up for the Ultimate >> GeekDad Father's Day Giveaway. ONE MASSIVE PRIZE to the >> lucky parental unit. See the prize list and enter to win: >> http://p.sf.net/sfu/thinkgeek-promo >> _______________________________________________ >> oprofile-list mailing list >> opr...@li... >> https://lists.sourceforge.net/lists/listinfo/oprofile-list > > > ------------------------------------------------------------------------------ > ThinkGeek and WIRED's GeekDad team up for the Ultimate > GeekDad Father's Day Giveaway. ONE MASSIVE PRIZE to the > lucky parental unit. See the prize list and enter to win: > http://p.sf.net/sfu/thinkgeek-promo > _______________________________________________ > oprofile-list mailing list > opr...@li... > https://lists.sourceforge.net/lists/listinfo/oprofile-list |
From: Maynard J. <may...@us...> - 2010-07-02 16:35:17
Attachments:
ophelp_msg.patch
|
Andi Kleen wrote: > On Mon, Jun 28, 2010 at 09:03:14AM -0500, Maynard Johnson wrote: >> On 06/24/2010 9:43 AM, Andi Kleen wrote: >>>> Well, as far as I can tell, the only problem we'd have in this >>>> regard is due to the aggregation of i386 and x86_64 in the >>>> events/Makefile.am. The ppc and ppc64 event files are separated in >>> >>> s390 / s390x, mips, mips64, ... can come out of uname. >> >> I don't see what s390 has to do with this since only timer-based >> profiling is supported on that architecture. And there are no >> mips64 oprofile events. So I don't grok what it is you're trying to >> say here. > > uname returns mips64, ppc64 etc., the event files are mips without 64 etc. > so it would not print the message. Well, I don't want to integrate a fix that doesn't work for all architectures. Also -- your patch doesn't avoid the ugly message: "oprofile: could not open unit mask description file <oprof-install-dir>/share/oprofile/<blah>" Your new message never gets printed out, because you call cross_profile() too late. Here's a patch I whipped up this morning. It's longer than yours, but it doesn't rely on uname, which (you pointed out) can be ambiguous. Can you take a look at it and verify it works as it should for the i386/x86_64 case? Signed-off-by: Maynard Johnson <may...@us...> |
From: Andi K. <an...@fi...> - 2010-07-02 18:14:16
|
On Fri, Jul 02, 2010 at 11:35:15AM -0500, Maynard Johnson wrote: > Andi Kleen wrote: > > On Mon, Jun 28, 2010 at 09:03:14AM -0500, Maynard Johnson wrote: > >> On 06/24/2010 9:43 AM, Andi Kleen wrote: > >>>> Well, as far as I can tell, the only problem we'd have in this > >>>> regard is due to the aggregation of i386 and x86_64 in the > >>>> events/Makefile.am. The ppc and ppc64 event files are separated in > >>> > >>> s390 / s390x, mips, mips64, ... can come out of uname. > >> > >> I don't see what s390 has to do with this since only timer-based > >> profiling is supported on that architecture. And there are no > >> mips64 oprofile events. So I don't grok what it is you're trying to > >> say here. > > > > uname returns mips64, ppc64 etc., the event files are mips without 64 etc. > > so it would not print the message. > > Well, I don't want to integrate a fix that doesn't work for all architectures. Also -- your patch doesn't avoid the ugly message: My original patch worked on all architectures. > "oprofile: could not open unit mask description file <oprof-install-dir>/share/oprofile/<blah>" > Your new message never gets printed out, because you call cross_profile() too late. > > Here's a patch I whipped up this morning. It's longer than yours, but it doesn't rely on uname, which (you pointed out) can be ambiguous. Can you take a look at it and verify it works as it should for the i386/x86_64 case? % uname -m x86_64 % ./utils/ophelp --cpu-type=i386-pii Unable to open cpu_type file for reading Make sure you have done opcontrol --init --cpu-type for non native architectures is not supported -Andi -- ak...@li... -- Speaking for myself only. |
From: Maynard J. <may...@us...> - 2010-07-06 14:06:09
|
Andi Kleen wrote: > On Fri, Jul 02, 2010 at 11:35:15AM -0500, Maynard Johnson wrote: >> Andi Kleen wrote: >>> On Mon, Jun 28, 2010 at 09:03:14AM -0500, Maynard Johnson wrote: >>>> On 06/24/2010 9:43 AM, Andi Kleen wrote: >>>>>> Well, as far as I can tell, the only problem we'd have in this >>>>>> regard is due to the aggregation of i386 and x86_64 in the >>>>>> events/Makefile.am. The ppc and ppc64 event files are separated in >>>>> >>>>> s390 / s390x, mips, mips64, ... can come out of uname. >>>> >>>> I don't see what s390 has to do with this since only timer-based >>>> profiling is supported on that architecture. And there are no >>>> mips64 oprofile events. So I don't grok what it is you're trying to >>>> say here. >>> >>> uname returns mips64, ppc64 etc., the event files are mips without 64 etc. >>> so it would not print the message. >> >> Well, I don't want to integrate a fix that doesn't work for all architectures. Also -- your patch doesn't avoid the ugly message: > > My original patch worked on all architectures. No, your patch does *NOT* work. In your testing below, you're passing in '--cpu-type=i386-pii'. That's not a valid cpu type. It should be "i386/pii". And what about mips? You stated above "uname returns mips64, ppc64 etc., the event files are mips without 64 etc. so it would not print the message." What exactly do you mean by that? I took it to mean there would be some kind of breakage with your patch on mips. -Maynard > >> "oprofile: could not open unit mask description file <oprof-install-dir>/share/oprofile/<blah>" >> Your new message never gets printed out, because you call cross_profile() too late. >> >> Here's a patch I whipped up this morning. It's longer than yours, but it doesn't rely on uname, which (you pointed out) can be ambiguous. Can you take a look at it and verify it works as it should for the i386/x86_64 case? > > % uname -m > x86_64 > % ./utils/ophelp --cpu-type=i386-pii > Unable to open cpu_type file for reading > Make sure you have done opcontrol --init > --cpu-type for non native architectures is not supported > > -Andi > > |
From: Andi K. <an...@fi...> - 2010-07-07 09:50:01
|
Maynard Johnson <may...@us...> writes: > Andi Kleen wrote: >> On Fri, Jul 02, 2010 at 11:35:15AM -0500, Maynard Johnson wrote: >>> Andi Kleen wrote: >>>> On Mon, Jun 28, 2010 at 09:03:14AM -0500, Maynard Johnson wrote: >>>>> On 06/24/2010 9:43 AM, Andi Kleen wrote: >>>>>>> Well, as far as I can tell, the only problem we'd have in this >>>>>>> regard is due to the aggregation of i386 and x86_64 in the >>>>>>> events/Makefile.am. The ppc and ppc64 event files are separated in >>>>>> >>>>>> s390 / s390x, mips, mips64, ... can come out of uname. >>>>> >>>>> I don't see what s390 has to do with this since only timer-based >>>>> profiling is supported on that architecture. And there are no >>>>> mips64 oprofile events. So I don't grok what it is you're trying to >>>>> say here. >>>> >>>> uname returns mips64, ppc64 etc., the event files are mips without 64 etc. >>>> so it would not print the message. >>> >>> Well, I don't want to integrate a fix that doesn't work for all architectures. Also -- your patch doesn't avoid the ugly message: >> >> My original patch worked on all architectures. > > No, your patch does *NOT* work. In your testing below, you're passing in '--cpu-type=i386-pii'. That's not a valid cpu type. It should be "i386/pii". That was your patch, not mine. But yes I used the wrong command line. Here is a updated test with your patch: ./utils/ophelp --cpu-type=i386/pii Unable to open cpu_type file for reading Make sure you have done opcontrol --init --cpu-type for non native architectures is not supported So it seems to work for this case. > And what about mips? You stated above "uname returns mips64, ppc64 etc., the event files are mips without 64 etc. so it would not print the message." What exactly do you mean by that? I took it to mean there would be some kind of breakage with your patch on mips. I'm just saying that there's the same situation as on x86 because the same userland runs on different 'uname -m's and we could probably spend 50-100 lines of code just for that single usage() line. For very small values of "breakage" as in a extremly obscure usage() line for a rarely to never used option may not be printed. -Andi -- ak...@li... -- Speaking for myself only. |
From: Maynard J. <may...@us...> - 2010-07-07 17:07:17
Attachments:
op_arch_evt_files_v4.patch
|
Andi Kleen wrote: > Maynard Johnson <may...@us...> writes: > >> Andi Kleen wrote: >>> On Fri, Jul 02, 2010 at 11:35:15AM -0500, Maynard Johnson wrote: >>>> Andi Kleen wrote: >>>>> On Mon, Jun 28, 2010 at 09:03:14AM -0500, Maynard Johnson wrote: >>>>>> On 06/24/2010 9:43 AM, Andi Kleen wrote: >>>>>>>> Well, as far as I can tell, the only problem we'd have in this >>>>>>>> regard is due to the aggregation of i386 and x86_64 in the >>>>>>>> events/Makefile.am. The ppc and ppc64 event files are separated in >>>>>>> >>>>>>> s390 / s390x, mips, mips64, ... can come out of uname. >>>>>> >>>>>> I don't see what s390 has to do with this since only timer-based >>>>>> profiling is supported on that architecture. And there are no >>>>>> mips64 oprofile events. So I don't grok what it is you're trying to >>>>>> say here. >>>>> >>>>> uname returns mips64, ppc64 etc., the event files are mips without 64 etc. >>>>> so it would not print the message. >>>> >>>> Well, I don't want to integrate a fix that doesn't work for all architectures. Also -- your patch doesn't avoid the ugly message: >>> >>> My original patch worked on all architectures. >> >> No, your patch does *NOT* work. In your testing below, you're passing in '--cpu-type=i386-pii'. That's not a valid cpu type. It should be "i386/pii". > > That was your patch, not mine. But yes I used the wrong command line. > > Here is a updated test with your patch: > > ./utils/ophelp --cpu-type=i386/pii > Unable to open cpu_type file for reading > Make sure you have done opcontrol --init > --cpu-type for non native architectures is not supported > > So it seems to work for this case. > >> And what about mips? You stated above "uname returns mips64, ppc64 etc., the event files are mips without 64 etc. so it would not print the message." What exactly do you mean by that? I took it to mean there would be some kind of breakage with your patch on mips. > > > I'm just saying that there's the same situation as on x86 because the > same userland runs on different 'uname -m's and we could probably spend > 50-100 lines of code just for that single usage() line. > > For very small values of "breakage" as in a extremly obscure usage() line for > a rarely to never used option may not be printed. OK, one last (hopefully) round . . . I like your uname-based patch better than mine for its simplicity. The main issue was that your cross_profile function was being called from inside the "if cpu_type == CPU_NO_GOOD" block, which wasn't the right place. That's because if you pass in a valid cpu-type string (i.e., anything that can be found in the cpu_descrs[] of libop/op_cpu_type.c), the cpu-type will be valid and then we go on to try to load the events for it, and then fail with the ugly message, since the event files aren't installed for that cpu-type. So, I've moved the call to cross_profile so it's done at the right time. I've also added another check in cross_profile to check for the case of uname -m of "mips64" and the cpu_string starting with "mips". Would this cover the case you've mentioned earlier in regards to mips? The full patch is attached. I've tested this both before and after doing 'opcontrol --init' using various cpu-type arguments, such as: - valid cpu-type of current running arch - invalid cpu-type, with and without correct arch (e.g., on a ppc64 machine: "ppc64/bogus", "badarch/badproc" - invalidly formatted cpu-type (e.g, "ppc64-power5" (note the invalid "-" vs "/")) - valid cpu-type of wrong arch (e.g., "i386/core_i7" on a ppc64 machine) - invalid cpu-type of wrong arch (e.g., "i386/bogus" on a ppc64 machine) Andi, Ralf . . . please test this patch on Intel and MIPS machines, respectively. In general, let me know if there are any concerns with this version of the patch. Thanks. -Maynard > > -Andi > |
From: Andi K. <an...@fi...> - 2010-06-21 00:30:27
|
> Andi, hearing no objection, let's go ahead and remove the 'ophelp --cpu-type' option for non-native arch. I suggest fixing up ophelp:main so we recognize when a non-native arch is in the passed cpu_string, and put out a message at that point that such is not supported. Along with that change, we'd need a change to ophelp's --help output to indicate the --cpu-type is only valid for the native arch. Same for ophelp's man page. I looked at this. The problem is currently oprofile has no clue what's native and what's not. A new table could be added, but that would be another maintenance issue. I chose not do to this. I think oprofile also doesn't know it's non native. Here's a patch with the ophelp help updates. Please apply, -Andi 2010-06-04 Andi Kleen <an...@fi...> * events/Makefile.am (event_files): Split up. (target): Add. (event_files_{ppc,i386,x86_64,ppc64,avr32,mips,arm,alpha,ia64,ppc): Create from old event_files. * doc/ophelp.1.in: Add note to --cpu-type. * utils/ophelp.c (options): Add note to --cpu-type. Signed-off-by: Andi Kleen <ak...@li...> diff --git a/doc/ophelp.1.in b/doc/ophelp.1.in index 3548d74..ce4a429 100644 --- a/doc/ophelp.1.in +++ b/doc/ophelp.1.in @@ -20,7 +20,8 @@ value (e.g. "ophelp DATA_MEM_REFS"). .SH OPTIONS .TP .BI "--cpu-type / -c" -Show the events for the given numerical CPU type. +Show the events for the given numerical CPU type. This only works +for architectures supported by the currently running kernel. .br .TP .BI "--get-cpu-type / -r" diff --git a/events/Makefile.am b/events/Makefile.am index 6c51f78..0605b53 100644 --- a/events/Makefile.am +++ b/events/Makefile.am @@ -1,9 +1,15 @@ -event_files = \ +target := $(shell uname -m) + +event_files_common = rtc/events rtc/unit_masks + +event_files_alpha = \ alpha/ev4/events alpha/ev4/unit_masks \ alpha/ev5/events alpha/ev5/unit_masks \ alpha/ev67/events alpha/ev67/unit_masks \ alpha/ev6/events alpha/ev6/unit_masks \ - alpha/pca56/events alpha/pca56/unit_masks \ + alpha/pca56/events alpha/pca56/unit_masks + +event_files_i386 = \ i386/athlon/events i386/athlon/unit_masks \ i386/core_2/events i386/core_2/unit_masks \ i386/p4/events i386/p4-ht/events \ @@ -17,9 +23,18 @@ event_files = \ i386/atom/events i386/atom/unit_masks \ i386/core_i7/events i386/core_i7/unit_masks \ i386/nehalem/events i386/nehalem/unit_masks \ + x86-64/hammer/events x86-64/hammer/unit_masks \ + x86-64/family10/events x86-64/family10/unit_masks \ + x86-64/family11h/events x86-64/family11h/unit_masks + +event_files_x86_64 = ${event_files_i386} + +event_files_ia64 = \ ia64/ia64/events ia64/ia64/unit_masks \ ia64/itanium2/events ia64/itanium2/unit_masks \ - ia64/itanium/events ia64/itanium/unit_masks \ + ia64/itanium/events ia64/itanium/unit_masks + +event_files_ppc64 = \ ppc64/power4/events ppc64/power4/event_mappings ppc64/power4/unit_masks \ ppc64/power5/events ppc64/power5/event_mappings ppc64/power5/unit_masks \ ppc64/power5+/events ppc64/power5+/event_mappings ppc64/power5+/unit_masks \ @@ -30,17 +45,18 @@ event_files = \ ppc64/970MP/events ppc64/970MP/event_mappings ppc64/970MP/unit_masks \ ppc64/ibm-compat-v1/events ppc64/ibm-compat-v1/event_mappings ppc64/ibm-compat-v1/unit_masks \ ppc64/pa6t/events ppc64/pa6t/event_mappings ppc64/pa6t/unit_masks \ - ppc64/cell-be/events ppc64/cell-be/unit_masks \ - rtc/events rtc/unit_masks \ - x86-64/hammer/events x86-64/hammer/unit_masks \ - x86-64/family10/events x86-64/family10/unit_masks \ - x86-64/family11h/events x86-64/family11h/unit_masks \ + ppc64/cell-be/events ppc64/cell-be/unit_masks + +event_files_arm = \ arm/xscale1/events arm/xscale1/unit_masks \ arm/xscale2/events arm/xscale2/unit_masks \ arm/armv6/events arm/armv6/unit_masks \ arm/armv7/events arm/armv7/unit_masks \ - arm/mpcore/events arm/mpcore/unit_masks \ - avr32/events avr32/unit_masks \ + arm/mpcore/events arm/mpcore/unit_masks + +event_files_avr32 = avr32/events avr32/unit_masks + +event_files_mips = \ mips/20K/events mips/20K/unit_masks \ mips/24K/events mips/24K/unit_masks \ mips/25K/events mips/25K/unit_masks \ @@ -53,12 +69,16 @@ event_files = \ mips/r12000/events mips/r12000/unit_masks \ mips/vr5432/events mips/vr5432/unit_masks \ mips/vr5500/events mips/vr5500/unit_masks \ - mips/loongson2/events mips/loongson2/unit_masks \ + mips/loongson2/events mips/loongson2/unit_masks + +event_files_ppc = \ ppc/7450/events ppc/7450/unit_masks \ ppc/e500/events ppc/e500/unit_masks \ ppc/e500v2/events ppc/e500v2/unit_masks \ ppc/e300/events ppc/e300/unit_masks +event_files := ${event_files_common} ${event_files_${target}} + install-data-local: for i in ${event_files} ; do \ dir=`dirname $$i` ; \ diff --git a/utils/ophelp.c b/utils/ophelp.c index 0bb8780..4086ada 100644 --- a/utils/ophelp.c +++ b/utils/ophelp.c @@ -296,7 +296,7 @@ static int get_default_event; static struct poptOption options[] = { { "cpu-type", 'c', POPT_ARG_STRING, &cpu_string, 0, - "use the given CPU type", "cpu type", }, + "use the given CPU type (only for native CPUs)", "cpu type", }, { "check-events", 'e', POPT_ARG_NONE, &check_events, 0, "check the given event descriptions for validity", NULL, }, { "unit-mask", 'u', POPT_ARG_NONE, &unit_mask, 0, |
From: Maynard J. <may...@us...> - 2010-06-23 15:59:59
|
Andi Kleen wrote: >> Andi, hearing no objection, let's go ahead and remove the 'ophelp --cpu-type' option for non-native arch. I suggest fixing up ophelp:main so we recognize when a non-native arch is in the passed cpu_string, and put out a message at that point that such is not supported. Along with that change, we'd need a change to ophelp's --help output to indicate the --cpu-type is only valid for the native arch. Same for ophelp's man page. > > I looked at this. The problem is currently oprofile has no clue > what's native and what's not. A new table could be added, but > that would be another maintenance issue. I chose not do to > this. > > I think oprofile also doesn't know it's non native. True, but with a small hack, we could make it know. Something like the following (in ophelp::main): ==================================================== if (cpu_string) { // existing code op_cpu native_cputype; // begin new code char * native_cpustr; native_cputype = op_get_cpu_type(); native_cpustr - op_get_cpu_type_str(native_cputype); if ((cpu_string up to first '/') != (native_cpustr up to first '/')) { // pseudo code fprintf(stderr, "Events for non-native architecture are not available\n"); exit(EXIT_FAILURE); } // end new code cpu_type = op_get_cpu_number(cpu_string); } ==================================================== Without doing something like that, the user will get the unhelpful message of: "oprofile: could not open unit mask description file <oprof-install-dir>/share/oprofile/<blah>" if they do 'ophelp --cpu-type=<non-native_arch>/<some_processor_name>. -Maynard > > Here's a patch with the ophelp help updates. > > Please apply, > > -Andi > > > 2010-06-04 Andi Kleen <an...@fi...> > > * events/Makefile.am (event_files): Split up. > (target): Add. > (event_files_{ppc,i386,x86_64,ppc64,avr32,mips,arm,alpha,ia64,ppc): > Create from old event_files. > * doc/ophelp.1.in: Add note to --cpu-type. > * utils/ophelp.c (options): Add note to --cpu-type. > > Signed-off-by: Andi Kleen <ak...@li...> > > diff --git a/doc/ophelp.1.in b/doc/ophelp.1.in > index 3548d74..ce4a429 100644 > --- a/doc/ophelp.1.in > +++ b/doc/ophelp.1.in > @@ -20,7 +20,8 @@ value (e.g. "ophelp DATA_MEM_REFS"). > .SH OPTIONS > .TP > .BI "--cpu-type / -c" > -Show the events for the given numerical CPU type. > +Show the events for the given numerical CPU type. This only works > +for architectures supported by the currently running kernel. > .br > .TP > .BI "--get-cpu-type / -r" > diff --git a/events/Makefile.am b/events/Makefile.am > index 6c51f78..0605b53 100644 > --- a/events/Makefile.am > +++ b/events/Makefile.am > @@ -1,9 +1,15 @@ > -event_files = \ > +target := $(shell uname -m) > + > +event_files_common = rtc/events rtc/unit_masks > + > +event_files_alpha = \ > alpha/ev4/events alpha/ev4/unit_masks \ > alpha/ev5/events alpha/ev5/unit_masks \ > alpha/ev67/events alpha/ev67/unit_masks \ > alpha/ev6/events alpha/ev6/unit_masks \ > - alpha/pca56/events alpha/pca56/unit_masks \ > + alpha/pca56/events alpha/pca56/unit_masks > + > +event_files_i386 = \ > i386/athlon/events i386/athlon/unit_masks \ > i386/core_2/events i386/core_2/unit_masks \ > i386/p4/events i386/p4-ht/events \ > @@ -17,9 +23,18 @@ event_files = \ > i386/atom/events i386/atom/unit_masks \ > i386/core_i7/events i386/core_i7/unit_masks \ > i386/nehalem/events i386/nehalem/unit_masks \ > + x86-64/hammer/events x86-64/hammer/unit_masks \ > + x86-64/family10/events x86-64/family10/unit_masks \ > + x86-64/family11h/events x86-64/family11h/unit_masks > + > +event_files_x86_64 = ${event_files_i386} > + > +event_files_ia64 = \ > ia64/ia64/events ia64/ia64/unit_masks \ > ia64/itanium2/events ia64/itanium2/unit_masks \ > - ia64/itanium/events ia64/itanium/unit_masks \ > + ia64/itanium/events ia64/itanium/unit_masks > + > +event_files_ppc64 = \ > ppc64/power4/events ppc64/power4/event_mappings ppc64/power4/unit_masks \ > ppc64/power5/events ppc64/power5/event_mappings ppc64/power5/unit_masks \ > ppc64/power5+/events ppc64/power5+/event_mappings ppc64/power5+/unit_masks \ > @@ -30,17 +45,18 @@ event_files = \ > ppc64/970MP/events ppc64/970MP/event_mappings ppc64/970MP/unit_masks \ > ppc64/ibm-compat-v1/events ppc64/ibm-compat-v1/event_mappings ppc64/ibm-compat-v1/unit_masks \ > ppc64/pa6t/events ppc64/pa6t/event_mappings ppc64/pa6t/unit_masks \ > - ppc64/cell-be/events ppc64/cell-be/unit_masks \ > - rtc/events rtc/unit_masks \ > - x86-64/hammer/events x86-64/hammer/unit_masks \ > - x86-64/family10/events x86-64/family10/unit_masks \ > - x86-64/family11h/events x86-64/family11h/unit_masks \ > + ppc64/cell-be/events ppc64/cell-be/unit_masks > + > +event_files_arm = \ > arm/xscale1/events arm/xscale1/unit_masks \ > arm/xscale2/events arm/xscale2/unit_masks \ > arm/armv6/events arm/armv6/unit_masks \ > arm/armv7/events arm/armv7/unit_masks \ > - arm/mpcore/events arm/mpcore/unit_masks \ > - avr32/events avr32/unit_masks \ > + arm/mpcore/events arm/mpcore/unit_masks > + > +event_files_avr32 = avr32/events avr32/unit_masks > + > +event_files_mips = \ > mips/20K/events mips/20K/unit_masks \ > mips/24K/events mips/24K/unit_masks \ > mips/25K/events mips/25K/unit_masks \ > @@ -53,12 +69,16 @@ event_files = \ > mips/r12000/events mips/r12000/unit_masks \ > mips/vr5432/events mips/vr5432/unit_masks \ > mips/vr5500/events mips/vr5500/unit_masks \ > - mips/loongson2/events mips/loongson2/unit_masks \ > + mips/loongson2/events mips/loongson2/unit_masks > + > +event_files_ppc = \ > ppc/7450/events ppc/7450/unit_masks \ > ppc/e500/events ppc/e500/unit_masks \ > ppc/e500v2/events ppc/e500v2/unit_masks \ > ppc/e300/events ppc/e300/unit_masks > > +event_files := ${event_files_common} ${event_files_${target}} > + > install-data-local: > for i in ${event_files} ; do \ > dir=`dirname $$i` ; \ > diff --git a/utils/ophelp.c b/utils/ophelp.c > index 0bb8780..4086ada 100644 > --- a/utils/ophelp.c > +++ b/utils/ophelp.c > @@ -296,7 +296,7 @@ static int get_default_event; > > static struct poptOption options[] = { > { "cpu-type", 'c', POPT_ARG_STRING, &cpu_string, 0, > - "use the given CPU type", "cpu type", }, > + "use the given CPU type (only for native CPUs)", "cpu type", }, > { "check-events", 'e', POPT_ARG_NONE, &check_events, 0, > "check the given event descriptions for validity", NULL, }, > { "unit-mask", 'u', POPT_ARG_NONE, &unit_mask, 0, |
From: Andi K. <an...@fi...> - 2010-06-23 21:03:31
|
On Wed, Jun 23, 2010 at 10:59:59AM -0500, Maynard Johnson wrote: > Andi Kleen wrote: > >> Andi, hearing no objection, let's go ahead and remove the 'ophelp --cpu-type' option for non-native arch. I suggest fixing up ophelp:main so we recognize when a non-native arch is in the passed cpu_string, and put out a message at that point that such is not supported. Along with that change, we'd need a change to ophelp's --help output to indicate the --cpu-type is only valid for the native arch. Same for ophelp's man page. > > > > I looked at this. The problem is currently oprofile has no clue > > what's native and what's not. A new table could be added, but > > that would be another maintenance issue. I chose not do to > > this. > > > > I think oprofile also doesn't know it's non native. > > True, but with a small hack, we could make it know. Something like the following (in ophelp::main): That doesn't work for all the architectures which have multiple names e.g. x86_64 vs i[3-6]86, ppc32 vs ppc64 etc. etc. This patch just adds an unconditional fprintf to the help statement. Please commit these changes now. -Andi commit ae675a8626c1c694814cc78bdd448abcace6c951 Author: Andi Kleen <ak...@li...> Date: Wed Jun 23 23:00:18 2010 +0200 * 2010-06-23 Andi Kleen <ak...@li...> utils/ophelp.c (main): Add note about non native archs. diff --git a/utils/ophelp.c b/utils/ophelp.c index 4086ada..fcbbe05 100644 --- a/utils/ophelp.c +++ b/utils/ophelp.c @@ -386,6 +386,8 @@ int main(int argc, char const * argv[]) cpu_string ? cpu_string : "unset"); fprintf(stderr, "you should upgrade oprofile or force the " "use of timer mode\n"); + fprintf(stderr, + "--cpu-type for non native architectures is not supported\n"); exit(EXIT_FAILURE); } |
From: Maynard J. <may...@us...> - 2010-06-24 14:30:59
|
Andi Kleen wrote: > On Wed, Jun 23, 2010 at 10:59:59AM -0500, Maynard Johnson wrote: >> Andi Kleen wrote: >>>> Andi, hearing no objection, let's go ahead and remove the 'ophelp --cpu-type' option for non-native arch. I suggest fixing up ophelp:main so we recognize when a non-native arch is in the passed cpu_string, and put out a message at that point that such is not supported. Along with that change, we'd need a change to ophelp's --help output to indicate the --cpu-type is only valid for the native arch. Same for ophelp's man page. >>> >>> I looked at this. The problem is currently oprofile has no clue >>> what's native and what's not. A new table could be added, but >>> that would be another maintenance issue. I chose not do to >>> this. >>> >>> I think oprofile also doesn't know it's non native. >> >> True, but with a small hack, we could make it know. Something like the following (in ophelp::main): > > That doesn't work for all the architectures which have multiple names > e.g. x86_64 vs i[3-6]86, ppc32 vs ppc64 etc. etc. Well, as far as I can tell, the only problem we'd have in this regard is due to the aggregation of i386 and x86_64 in the events/Makefile.am. The ppc and ppc64 event files are separated in your patched events/Makefile.am -- as they should be, since they are, in fact, different architectures. Why are i386 and x86_64 lumped together? -Maynard > > This patch just adds an unconditional fprintf to the help statement. > > Please commit these changes now. > > -Andi > > > commit ae675a8626c1c694814cc78bdd448abcace6c951 > Author: Andi Kleen <ak...@li...> > Date: Wed Jun 23 23:00:18 2010 +0200 > > * 2010-06-23 Andi Kleen <ak...@li...> > > utils/ophelp.c (main): Add note about non native archs. > > diff --git a/utils/ophelp.c b/utils/ophelp.c > index 4086ada..fcbbe05 100644 > --- a/utils/ophelp.c > +++ b/utils/ophelp.c > @@ -386,6 +386,8 @@ int main(int argc, char const * argv[]) > cpu_string ? cpu_string : "unset"); > fprintf(stderr, "you should upgrade oprofile or force the " > "use of timer mode\n"); > + fprintf(stderr, > + "--cpu-type for non native architectures is not supported\n"); > exit(EXIT_FAILURE); > } > |
From: Andi K. <an...@fi...> - 2010-06-24 14:43:23
|
> Well, as far as I can tell, the only problem we'd have in this > regard is due to the aggregation of i386 and x86_64 in the > events/Makefile.am. The ppc and ppc64 event files are separated in s390 / s390x, mips, mips64, ... can come out of uname. And you can run with linux32/setarch on powerpc64 too, then you get ppc I suspect. > your patched events/Makefile.am -- as they should be, since they > are, in fact, different architectures. Why are i386 and x86_64 > lumped together? You can have both on the same kernel and some of the events are for i386 and others for x86-64 for historical reasons. -Andi -- ak...@li... -- Speaking for myself only. |