From: Maynard J. <may...@us...> - 2013-07-19 16:07:21
|
A fix to allow named unit masks to be used as the default was recently pushed upstream (Jun 24: Add support for named default unit masks), but unfortunately, we all missed the fact that using a named default unit mask didn't actually work insofar as counting events. Here's an example on Sandybridge that should use the default named unit mask "any": operf -e uops_issued:2000000 ./my_test and opreport shows "opreport error: No sample file found". The patch below fixes this. *Suravee, Andi*, I would appreciate a quick review of this patch, please. ----------------------------------------------------------------- Defaulted named unit mask does not work When the user does not specify a unit mask, the profiling tools (as well as ocount) will use 'ophelp --unit-mask' to determine what the default unit mask should be. All of the oprofile tools -- operf, opcontrol, ocount -- expect a numerical value to be returned. So, in the case of a named default unit mask, the unit mask name returned by ophelp was not being handled properly, and the end result was usually "No samples found" by opreport (or zero event counts by ocount). This patch fixes this problem. Signed-off-by: Maynard Johnson <may...@us...> --- libpe_utils/op_pe_utils.cpp | 10 +++++++++- pe_profiling/operf.cpp | 10 +++++++++- utils/opcontrol | 2 +- 3 files changed, 19 insertions(+), 3 deletions(-) diff --git a/libpe_utils/op_pe_utils.cpp b/libpe_utils/op_pe_utils.cpp index 73ea75a..e82040a 100644 --- a/libpe_utils/op_pe_utils.cpp +++ b/libpe_utils/op_pe_utils.cpp @@ -459,6 +459,7 @@ static void _get_event_code(operf_event_t * event, op_cpu cpu_type) config |= base_code & 0xFFULL; // Setup unitmask field +handle_named_um: if (event->um_name[0]) { command = OP_BINDIR; command += "ophelp "; @@ -495,6 +496,7 @@ static void _get_event_code(operf_event_t * event, op_cpu cpu_type) else config |= ((event->evt_um & 0xFFULL) << 8); } else if (!event->evt_um) { + char * endptr; command.clear(); command = OP_BINDIR; command += "ophelp "; @@ -514,7 +516,13 @@ static void _get_event_code(operf_event_t * event, op_cpu cpu_type) exit(EXIT_FAILURE); } pclose(fp); - event->evt_um = strtoull(mask, (char **) NULL, 10); + event->evt_um = strtoull(mask, &endptr, 10); + if ((endptr >= mask) && + (endptr <= (mask + strlen(mask) - 1))) { + // Must be a default named unit mask + strncpy(event->um_name, mask, OP_MAX_UM_NAME_LEN); + goto handle_named_um; + } config |= ((event->evt_um & 0xFFULL) << 8); } else { config |= ((event->evt_um & 0xFFULL) << 8); diff --git a/pe_profiling/operf.cpp b/pe_profiling/operf.cpp index a558364..3fec123 100644 --- a/pe_profiling/operf.cpp +++ b/pe_profiling/operf.cpp @@ -1123,6 +1123,7 @@ static void _get_event_code(operf_event_t * event) config |= base_code & 0xFFULL; // Setup unitmask field +handle_named_um: if (event->um_name[0]) { command = OP_BINDIR; command += "ophelp "; @@ -1159,6 +1160,7 @@ static void _get_event_code(operf_event_t * event) else config |= ((event->evt_um & 0xFFULL) << 8); } else if (!event->evt_um) { + char * endptr; command.clear(); command = OP_BINDIR; command += "ophelp "; @@ -1178,7 +1180,13 @@ static void _get_event_code(operf_event_t * event) exit(EXIT_FAILURE); } pclose(fp); - event->evt_um = strtoull(mask, (char **) NULL, 10); + event->evt_um = strtoull(mask, &endptr, 10); + if ((endptr >= mask) && + (endptr <= (mask + strlen(mask) - 1))) { + // Must be a default named unit mask + strncpy(event->um_name, mask, OP_MAX_UM_NAME_LEN); + goto handle_named_um; + } config |= ((event->evt_um & 0xFFULL) << 8); } else { config |= ((event->evt_um & 0xFFULL) << 8); diff --git a/utils/opcontrol b/utils/opcontrol index d87641e..38bb1ac 100644 --- a/utils/opcontrol +++ b/utils/opcontrol @@ -722,8 +722,8 @@ normalise_events() KERNEL=`echo $GOTEVENT | awk -F: '{print $4}'` USER=`echo $GOTEVENT | awk -F: '{print $5}'` TMPEVENT="$EVENT:$COUNT:$UNIT_MASK" - UNIT_MASK_NAMED="$UNIT_MASK" UNIT_MASK=`$OPHELP --unit-mask $TMPEVENT` + UNIT_MASK_NAMED="$UNIT_MASK" if test "$?" != 0; then exit 1 fi -- 1.7.1 |
From: Maynard J. <may...@us...> - 2013-07-23 11:55:39
|
On 07/19/2013 11:07 AM, Maynard Johnson wrote: > A fix to allow named unit masks to be used as the default was recently > pushed upstream (Jun 24: Add support for named default unit masks), but > unfortunately, we all missed the fact that using a named default > unit mask didn't actually work insofar as counting events. > Here's an example on Sandybridge that should use the default named > unit mask "any": > > operf -e uops_issued:2000000 ./my_test > and opreport shows "opreport error: No sample file found". > > The patch below fixes this. *Suravee, Andi*, I would appreciate > a quick review of this patch, please. I plan on rolling out release candidate 1 for oprofile 0.9.9 later today. If anyone has any issues with this patch, please say so now. -Maynard > > > ----------------------------------------------------------------- > > Defaulted named unit mask does not work > > When the user does not specify a unit mask, the profiling tools > (as well as ocount) will use 'ophelp --unit-mask' to determine > what the default unit mask should be. All of the oprofile > tools -- operf, opcontrol, ocount -- expect a numerical value > to be returned. So, in the case of a named default unit mask, > the unit mask name returned by ophelp was not being handled > properly, and the end result was usually "No samples found" > by opreport (or zero event counts by ocount). This patch > fixes this problem. > > Signed-off-by: Maynard Johnson <may...@us...> > --- > libpe_utils/op_pe_utils.cpp | 10 +++++++++- > pe_profiling/operf.cpp | 10 +++++++++- > utils/opcontrol | 2 +- > 3 files changed, 19 insertions(+), 3 deletions(-) > > diff --git a/libpe_utils/op_pe_utils.cpp b/libpe_utils/op_pe_utils.cpp > index 73ea75a..e82040a 100644 > --- a/libpe_utils/op_pe_utils.cpp > +++ b/libpe_utils/op_pe_utils.cpp > @@ -459,6 +459,7 @@ static void _get_event_code(operf_event_t * event, op_cpu cpu_type) > config |= base_code & 0xFFULL; > > // Setup unitmask field > +handle_named_um: > if (event->um_name[0]) { > command = OP_BINDIR; > command += "ophelp "; > @@ -495,6 +496,7 @@ static void _get_event_code(operf_event_t * event, op_cpu cpu_type) > else > config |= ((event->evt_um & 0xFFULL) << 8); > } else if (!event->evt_um) { > + char * endptr; > command.clear(); > command = OP_BINDIR; > command += "ophelp "; > @@ -514,7 +516,13 @@ static void _get_event_code(operf_event_t * event, op_cpu cpu_type) > exit(EXIT_FAILURE); > } > pclose(fp); > - event->evt_um = strtoull(mask, (char **) NULL, 10); > + event->evt_um = strtoull(mask, &endptr, 10); > + if ((endptr >= mask) && > + (endptr <= (mask + strlen(mask) - 1))) { > + // Must be a default named unit mask > + strncpy(event->um_name, mask, OP_MAX_UM_NAME_LEN); > + goto handle_named_um; > + } > config |= ((event->evt_um & 0xFFULL) << 8); > } else { > config |= ((event->evt_um & 0xFFULL) << 8); > diff --git a/pe_profiling/operf.cpp b/pe_profiling/operf.cpp > index a558364..3fec123 100644 > --- a/pe_profiling/operf.cpp > +++ b/pe_profiling/operf.cpp > @@ -1123,6 +1123,7 @@ static void _get_event_code(operf_event_t * event) > config |= base_code & 0xFFULL; > > // Setup unitmask field > +handle_named_um: > if (event->um_name[0]) { > command = OP_BINDIR; > command += "ophelp "; > @@ -1159,6 +1160,7 @@ static void _get_event_code(operf_event_t * event) > else > config |= ((event->evt_um & 0xFFULL) << 8); > } else if (!event->evt_um) { > + char * endptr; > command.clear(); > command = OP_BINDIR; > command += "ophelp "; > @@ -1178,7 +1180,13 @@ static void _get_event_code(operf_event_t * event) > exit(EXIT_FAILURE); > } > pclose(fp); > - event->evt_um = strtoull(mask, (char **) NULL, 10); > + event->evt_um = strtoull(mask, &endptr, 10); > + if ((endptr >= mask) && > + (endptr <= (mask + strlen(mask) - 1))) { > + // Must be a default named unit mask > + strncpy(event->um_name, mask, OP_MAX_UM_NAME_LEN); > + goto handle_named_um; > + } > config |= ((event->evt_um & 0xFFULL) << 8); > } else { > config |= ((event->evt_um & 0xFFULL) << 8); > diff --git a/utils/opcontrol b/utils/opcontrol > index d87641e..38bb1ac 100644 > --- a/utils/opcontrol > +++ b/utils/opcontrol > @@ -722,8 +722,8 @@ normalise_events() > KERNEL=`echo $GOTEVENT | awk -F: '{print $4}'` > USER=`echo $GOTEVENT | awk -F: '{print $5}'` > TMPEVENT="$EVENT:$COUNT:$UNIT_MASK" > - UNIT_MASK_NAMED="$UNIT_MASK" > UNIT_MASK=`$OPHELP --unit-mask $TMPEVENT` > + UNIT_MASK_NAMED="$UNIT_MASK" > if test "$?" != 0; then > exit 1 > fi > |
From: Maynard J. <may...@us...> - 2013-07-24 17:37:13
|
On 07/23/2013 06:55 AM, Maynard Johnson wrote: > On 07/19/2013 11:07 AM, Maynard Johnson wrote: >> A fix to allow named unit masks to be used as the default was recently >> pushed upstream (Jun 24: Add support for named default unit masks), but >> unfortunately, we all missed the fact that using a named default >> unit mask didn't actually work insofar as counting events. >> Here's an example on Sandybridge that should use the default named >> unit mask "any": >> >> operf -e uops_issued:2000000 ./my_test >> and opreport shows "opreport error: No sample file found". >> >> The patch below fixes this. *Suravee, Andi*, I would appreciate >> a quick review of this patch, please. Patch committed. -Maynard > > I plan on rolling out release candidate 1 for oprofile 0.9.9 later today. > If anyone has any issues with this patch, please say so now. > > -Maynard >> >> >> ----------------------------------------------------------------- >> >> Defaulted named unit mask does not work >> >> When the user does not specify a unit mask, the profiling tools >> (as well as ocount) will use 'ophelp --unit-mask' to determine >> what the default unit mask should be. All of the oprofile >> tools -- operf, opcontrol, ocount -- expect a numerical value >> to be returned. So, in the case of a named default unit mask, >> the unit mask name returned by ophelp was not being handled >> properly, and the end result was usually "No samples found" >> by opreport (or zero event counts by ocount). This patch >> fixes this problem. >> >> Signed-off-by: Maynard Johnson <may...@us...> >> --- >> libpe_utils/op_pe_utils.cpp | 10 +++++++++- >> pe_profiling/operf.cpp | 10 +++++++++- >> utils/opcontrol | 2 +- >> 3 files changed, 19 insertions(+), 3 deletions(-) >> >> diff --git a/libpe_utils/op_pe_utils.cpp b/libpe_utils/op_pe_utils.cpp >> index 73ea75a..e82040a 100644 >> --- a/libpe_utils/op_pe_utils.cpp >> +++ b/libpe_utils/op_pe_utils.cpp >> @@ -459,6 +459,7 @@ static void _get_event_code(operf_event_t * event, op_cpu cpu_type) >> config |= base_code & 0xFFULL; >> >> // Setup unitmask field >> +handle_named_um: >> if (event->um_name[0]) { >> command = OP_BINDIR; >> command += "ophelp "; >> @@ -495,6 +496,7 @@ static void _get_event_code(operf_event_t * event, op_cpu cpu_type) >> else >> config |= ((event->evt_um & 0xFFULL) << 8); >> } else if (!event->evt_um) { >> + char * endptr; >> command.clear(); >> command = OP_BINDIR; >> command += "ophelp "; >> @@ -514,7 +516,13 @@ static void _get_event_code(operf_event_t * event, op_cpu cpu_type) >> exit(EXIT_FAILURE); >> } >> pclose(fp); >> - event->evt_um = strtoull(mask, (char **) NULL, 10); >> + event->evt_um = strtoull(mask, &endptr, 10); >> + if ((endptr >= mask) && >> + (endptr <= (mask + strlen(mask) - 1))) { >> + // Must be a default named unit mask >> + strncpy(event->um_name, mask, OP_MAX_UM_NAME_LEN); >> + goto handle_named_um; >> + } >> config |= ((event->evt_um & 0xFFULL) << 8); >> } else { >> config |= ((event->evt_um & 0xFFULL) << 8); >> diff --git a/pe_profiling/operf.cpp b/pe_profiling/operf.cpp >> index a558364..3fec123 100644 >> --- a/pe_profiling/operf.cpp >> +++ b/pe_profiling/operf.cpp >> @@ -1123,6 +1123,7 @@ static void _get_event_code(operf_event_t * event) >> config |= base_code & 0xFFULL; >> >> // Setup unitmask field >> +handle_named_um: >> if (event->um_name[0]) { >> command = OP_BINDIR; >> command += "ophelp "; >> @@ -1159,6 +1160,7 @@ static void _get_event_code(operf_event_t * event) >> else >> config |= ((event->evt_um & 0xFFULL) << 8); >> } else if (!event->evt_um) { >> + char * endptr; >> command.clear(); >> command = OP_BINDIR; >> command += "ophelp "; >> @@ -1178,7 +1180,13 @@ static void _get_event_code(operf_event_t * event) >> exit(EXIT_FAILURE); >> } >> pclose(fp); >> - event->evt_um = strtoull(mask, (char **) NULL, 10); >> + event->evt_um = strtoull(mask, &endptr, 10); >> + if ((endptr >= mask) && >> + (endptr <= (mask + strlen(mask) - 1))) { >> + // Must be a default named unit mask >> + strncpy(event->um_name, mask, OP_MAX_UM_NAME_LEN); >> + goto handle_named_um; >> + } >> config |= ((event->evt_um & 0xFFULL) << 8); >> } else { >> config |= ((event->evt_um & 0xFFULL) << 8); >> diff --git a/utils/opcontrol b/utils/opcontrol >> index d87641e..38bb1ac 100644 >> --- a/utils/opcontrol >> +++ b/utils/opcontrol >> @@ -722,8 +722,8 @@ normalise_events() >> KERNEL=`echo $GOTEVENT | awk -F: '{print $4}'` >> USER=`echo $GOTEVENT | awk -F: '{print $5}'` >> TMPEVENT="$EVENT:$COUNT:$UNIT_MASK" >> - UNIT_MASK_NAMED="$UNIT_MASK" >> UNIT_MASK=`$OPHELP --unit-mask $TMPEVENT` >> + UNIT_MASK_NAMED="$UNIT_MASK" >> if test "$?" != 0; then >> exit 1 >> fi >> > > > ------------------------------------------------------------------------------ > See everything from the browser to the database with AppDynamics > Get end-to-end visibility with application monitoring from AppDynamics > Isolate bottlenecks and diagnose root cause in seconds. > Start your free trial of AppDynamics Pro today! > http://pubads.g.doubleclick.net/gampad/clk?id=48808831&iu=/4140/ostg.clktrk > _______________________________________________ > oprofile-list mailing list > opr...@li... > https://lists.sourceforge.net/lists/listinfo/oprofile-list > |