Kevin, On Thu, Jun 15, 2006 at 03:12:29PM -0500, Kevin Corry wrote: > Here's a patch I wrote today while testing pfmon with the new unit-mask > capabilities. >
Thanks for the patch. I added this last week but could not really test the code as I did not have the underlying arch-specific support. Also there are some parts missing in the rest of pfmon where we do print the event name. We need to track this down to make sure the unit mask is shown as well. On this same topic, at the PMU modeul-specific level, there needs to be a mechanism to prevent certain unit mask combinations. It is not because you have a selection of unit masks that they can necessarily be combined. I don't know if we can come up with generic code to do this kind of checks before calling the mode specific dispatch routine. > -- > Kevin Corry > [EMAIL PROTECTED] > http://www.ibm.com/linux/ > http://evms.sourceforge.net/ > > > The new code that parses event names and unit-mask names from the command > line doesn't handle specifying multiple events, each with one or more masks. > > For instance, in the command-line: > pfmon -e instr_retired:NBOGUSNTAG,global_power_events:RUNNING > the current code interprets "NBOGUSNTAG,global_power_events" as the > unit-mask name for the "instr_retired" event. > > This new algorithm should be a bit simpler. As it walks through the event > argument string, it keeps track of the current and next event names, and > within the current event, keeps track of the current and next unit-mask > names. > > This patch also removes a check for events that have unit-masks available, > but no unit-masks specified on the command-line for that event. On Pentium4, > while it is somewhat pointless to not specify a unit-mask, it is technically > allowed (it will just return a count of zero). And it's conceivable that > there might be platforms where it's possible to count an event without > specifying any unit-masks, event if that event allows using unit-masks. > > This also changes the calculation of "l". In the original code, it looks > like "l" was simply the length of the event name. In the latest code, it > looks like "l" was then incremented by the length of each unit-mask name. > To simplify this, this new code calculates "l" before splitting up the > unit-mask names from the event names, thus getting the length of the event > and all of its unit-masks in one operation. > > I have successfully tested several combinations of event names with and > without unit-mask names on the pfmon command-line. > > Signed-Off-By: Kevin Corry <[EMAIL PROTECTED]> > > Index: pfmon/pfmon_util.c > =================================================================== > RCS file: /cvsroot/perfmon2/pfmon/pfmon/pfmon_util.c,v > retrieving revision 1.2 > diff -u -b -B -r1.2 pfmon_util.c > --- pfmon/pfmon_util.c 13 Jun 2006 15:45:03 -0000 1.2 > +++ pfmon/pfmon_util.c 15 Jun 2006 19:50:50 -0000 > @@ -232,78 +232,61 @@ > void > setup_event_set(pfmon_event_set_t *set) > { > - char *arg; > - char *p, old_p; > + char *event_name, *next_event_name; > + char *mask_name, *next_mask_name; > size_t l; > - unsigned int ev, um, n; > - unsigned int cnt=0, num; > + unsigned int ev, um; > + unsigned int cnt, num; > + unsigned int num_masks; > + > + for (event_name = set->events_str, cnt = 0; > + event_name; > + event_name = next_event_name, cnt++) { > > - arg = set->events_str; > - > - > - while (arg) { > if (cnt == options.max_counters) > goto too_many; > > - p = strchr(arg,':'); > - if (p == NULL) > - p = strchr(arg, ','); > - > - if (p) { > - old_p = *p; > - *p++ = '\0'; > - } else > - old_p = '\0'; > + next_event_name = strchr(event_name, ','); > + if (next_event_name) > + *next_event_name++ = '\0'; > > - if (pfm_find_event(arg, &ev) != PFMLIB_SUCCESS) > - goto error; > + l = strlen(event_name); > > - l = strlen(arg); > + mask_name = strchr(event_name, ':'); > + if (mask_name) > + *mask_name++ = '\0'; > > - pfm_get_num_event_masks(ev, &n); > + if (pfm_find_event(event_name, &ev) != PFMLIB_SUCCESS) > + goto error; > > - if (n && old_p != ':') > - goto missing_umask; > + set->inp.pfp_events[cnt].event = ev; > > - if (n == 0 && old_p == ':') > + if (mask_name) { > + pfm_get_num_event_masks(ev, &num_masks); > + if (num_masks == 0) > goto no_umask; > + } > > - set->inp.pfp_events[cnt].event = ev; > - > - if (n && old_p == ':') { > - arg = p; > - num = 0; > - while ((p = strchr(arg, ':'))) { > - if (num == PFMLIB_MAX_MASKS_PER_EVENT) > + for (num = 0; mask_name; mask_name = next_mask_name, num++) { > + if (num == num_masks || > + num == PFMLIB_MAX_MASKS_PER_EVENT) > goto many_umask; > - *p = '\0'; > - if (pfm_find_event_mask(ev, arg, &um) != > PFMLIB_SUCCESS) > - goto invalid_umask; > > - set->inp.pfp_events[cnt].unit_masks[num]= um; > - num++; > - l += strlen(arg) + 1; > - arg = p + 1; > - } > - p = strchr(arg, ','); > - if (p) > - *p++ = '\0'; > + next_mask_name = strchr(mask_name, ':'); > + if (next_mask_name) > + *next_mask_name++ = '\0'; > > - if (num == PFMLIB_MAX_MASKS_PER_EVENT) > - goto many_umask; > - if (pfm_find_event_mask(ev, arg, &um) != PFMLIB_SUCCESS) > + if (pfm_find_event_mask(ev, mask_name, &um) != > PFMLIB_SUCCESS) > goto invalid_umask; > > set->inp.pfp_events[cnt].unit_masks[num] = um; > - l += strlen(arg) + 1; > - set->inp.pfp_events[cnt].num_masks = num+1; > } > + set->inp.pfp_events[cnt].num_masks = num; > + > if (l > options.max_event_name_len) > options.max_event_name_len = l; > - > - arg = p; > - cnt++; > } > + > /* setup event_count */ > set->inp.pfp_event_count = set->event_count = cnt; > return; > @@ -307,18 +290,17 @@ > /* setup event_count */ > set->inp.pfp_event_count = set->event_count = cnt; > return; > + > error: > - fatal_error("unknown event %s\n", arg); > + fatal_error("unknown event %s\n", event_name); > too_many: > fatal_error("too many events specified, max=%d\n", > options.max_counters); > -missing_umask: > - fatal_error("event %s takes at least a unit mask\n", arg); > no_umask: > - fatal_error("event %s does not take have unit masks\n", arg); > + fatal_error("event %s does not take any unit masks\n", event_name); > invalid_umask: > - fatal_error("unit mask %s does not exist\n", arg); > + fatal_error("unit mask %s does not exist\n", mask_name); > many_umask: > - fatal_error("too many umask defined\n"); > + fatal_error("too many umasks specified for event %s\n", event_name); > } > > void > _______________________________________________ > perfmon mailing list > [email protected] > http://www.hpl.hp.com/hosted/linux/mail-archives/perfmon/ -- -Stephane _______________________________________________ perfmon mailing list [email protected] http://www.hpl.hp.com/hosted/linux/mail-archives/perfmon/
