Kevin,
I looked at the patch and it looks good!
I have the following remarks:
- I would use a #define instead of hardcoding 32
- I would define the following macro to simplify code reading:
#define PFM_HAS_EVENT_MASK() \
(pfm_current->get_num_event_masks &&
pfm_current->get_event_mask_name)
I think we should only have one function to return the maximum name length for
both
event and unit mask names. Otherwise, it gets a little bit too complicated. I
would
add a function called pfm_get_max_name_len(size_t *len). If it is not too much
pain, we could replace the existing function with this one.
Another question is about unit mask enumeration. How would a user application
enumerate all the unit masks for a given event?
I think if we guarantee that unit mask indexes are contiguous then we can
ierate from 0 to pfm_get_num_event_masks() with a simple for loop.
Thanks.
On Fri, Jun 09, 2006 at 02:48:27PM -0500, Kevin Corry wrote:
> Hi Stephane,
>
> I've gone ahead and coded some of the APIs we've been discussing. There
> are 5 new external APIs:
> pfm_find_event_mask()
> pfm_get_max_event_mask_name_len()
> pfm_get_num_event_masks()
> pfm_get_event_mask_name()
> pfm_get_event_mask_description()
>
> and 3 new arch-module APIs (in pfm_pmu_support_t):
> get_event_mask_name()
> get_num_event_masks()
> get_event_mask_desc()
>
> The pfmlib_event_t structure gets an array of unit-mask identifiers and a
> count of unit-masks. Both of these new fields could be changed from unsigned
> int to unsigned short or unsigned char if we'd like to reduce the memory
> usage.
>
> I also made some additions to pfm_print_event_info() and
> pfm_print_event_info_byindex() to output info about the unit-masks.
>
> Here's my current patch against the libpfm CVS tree. Let me know what
> you think. I have not yet added the unit-mask code to the pentium4 module
> I'm working on, so this new code is untested right now. But it does
> compile - so it must work. :)
>
> Thanks,
> --
> Kevin Corry
> [EMAIL PROTECTED]
> http://www.ibm.com/linux/
> http://evms.sourceforge.net/
>
>
> Add unit-mask APIs to libpfm.
>
> Signed-Off-By: Kevin Corry <[EMAIL PROTECTED]>
>
> Index: include/perfmon/pfmlib.h
> ===================================================================
> RCS file: /cvsroot/perfmon2/libpfm/include/perfmon/pfmlib.h,v
> retrieving revision 1.3
> diff -u -r1.3 pfmlib.h
> --- include/perfmon/pfmlib.h 15 May 2006 13:57:01 -0000 1.3
> +++ include/perfmon/pfmlib.h 9 Jun 2006 19:34:40 -0000
> @@ -79,6 +79,8 @@
> unsigned int event; /* event descriptor */
> unsigned int plm; /* event privilege level mask */
> unsigned long flags; /* per-event flag */
> + unsigned int unit_masks[32]; /* unit-mask identifiers */
> + unsigned int num_masks; /* number of masks specified in
> 'unit_masks' */
> unsigned long reserved[2]; /* for future use */
> } pfmlib_event_t;
>
> @@ -164,14 +166,19 @@
> extern int pfm_find_event_byname(const char *name, unsigned int *idx);
> extern int pfm_find_event_bycode(int code, unsigned int *idx);
> extern int pfm_find_event_bycode_next(int code, unsigned int start, unsigned
> int *next);
> +extern int pfm_find_event_mask(unsigned int event_idx, const char *str,
> unsigned int *mask_idx);
> extern int pfm_get_max_event_name_len(size_t *len);
> +extern int pfm_get_max_event_mask_name_len(unsigned int event_idx, size_t
> *len);
>
> extern int pfm_get_num_events(unsigned int *count);
> +extern int pfm_get_num_event_masks(unsigned int event_idx, unsigned int
> *count);
> extern int pfm_get_event_name(unsigned int idx, char *name, size_t maxlen);
> extern int pfm_get_event_code(unsigned int idx, int *code);
> extern int pfm_get_event_counters(unsigned int idx, pfmlib_regmask_t
> *counters);
> extern int pfm_get_event_description(unsigned int idx, char **str);
> extern int pfm_get_event_code_counter(unsigned int idx, unsigned int cnt,
> int *code);
> +extern int pfm_get_event_mask_name(unsigned int event_idx, unsigned int
> mask_idx, char *name, size_t maxlen);
> +extern int pfm_get_event_mask_description(unsigned int event_idx, unsigned
> int mask_idx, char **desc);
>
> extern int pfm_print_event_info(const char *name, int (*pf)(const char
> *fmt,...));
> extern int pfm_print_event_info_byindex(unsigned int idx, int (*pf)(const
> char *fmt,...));
> Index: lib/pfmlib_common.c
> ===================================================================
> RCS file: /cvsroot/perfmon2/libpfm/lib/pfmlib_common.c,v
> retrieving revision 1.4
> diff -u -r1.4 pfmlib_common.c
> --- lib/pfmlib_common.c 2 Jun 2006 12:55:01 -0000 1.4
> +++ lib/pfmlib_common.c 9 Jun 2006 19:34:40 -0000
> @@ -335,6 +335,29 @@
> }
>
> int
> +pfm_find_event_mask(unsigned int event_idx, const char *str, unsigned int
> *mask_idx)
> +{
> + unsigned int i, num_masks = 0;
> +
> + if (PFMLIB_INITIALIZED() == 0) return PFMLIB_ERR_NOINIT;
> +
> + if (str == NULL || mask_idx == NULL) return PFMLIB_ERR_INVAL;
> +
> + if (pfm_current->get_num_event_masks &&
> + pfm_current->get_event_mask_name) {
> + pfm_current->get_num_event_masks(event_idx, &num_masks);
> + for (i = 0; i < num_masks; i++) {
> + if
> (!strcasecmp(pfm_current->get_event_mask_name(event_idx, i), str)) {
> + *mask_idx = i;
> + return PFMLIB_SUCCESS;
> + }
> + }
> + }
> +
> + return PFMLIB_ERR_NOTFOUND;
> +}
> +
> +int
> pfm_get_event_name(unsigned int i, char *name, size_t maxlen)
> {
> if (PFMLIB_INITIALIZED() == 0) return PFMLIB_ERR_NOINIT;
> @@ -381,6 +404,20 @@
> return PFMLIB_SUCCESS;
> }
>
> +int
> +pfm_get_event_mask_name(unsigned int event_idx, unsigned int mask_idx, char
> *name, size_t maxlen)
> +{
> + if (PFMLIB_INITIALIZED() == 0) return PFMLIB_ERR_NOINIT;
> +
> + if (event_idx >= pfm_current->pme_count || name == NULL || maxlen < 1)
> return PFMLIB_ERR_INVAL;
> +
> + if (pfm_current->get_event_mask_name) {
> + strncpy(name, pfm_current->get_event_mask_name(event_idx,
> mask_idx), maxlen-1);
> + name[maxlen-1] = '\0';
> + }
> +
> + return PFMLIB_SUCCESS;
> +}
>
> int
> pfm_get_num_events(unsigned int *count)
> @@ -394,6 +431,20 @@
> return PFMLIB_SUCCESS;
> }
>
> +int
> +pfm_get_num_event_masks(unsigned int event_idx, unsigned int *count)
> +{
> + if (PFMLIB_INITIALIZED() == 0) return PFMLIB_ERR_NOINIT;
> +
> + if (event_idx >= pfm_current->pme_count || count == NULL) return
> PFMLIB_ERR_INVAL;
> +
> + if (pfm_current->get_num_event_masks == NULL) {
> + *count = 0;
> + return PFMLIB_SUCCESS;
> + }
> +
> + return pfm_current->get_num_event_masks(event_idx, count);
> +}
>
> /*
> * Function used to print information about a specific events. More than
> @@ -408,6 +459,7 @@
> int code, ret, n;
> int event_is_digit = 0;
> unsigned int idx, next_idx, num_counters, i;
> + unsigned int num_masks;
>
> if (PFMLIB_INITIALIZED() == 0) return PFMLIB_ERR_NOINIT;
>
> @@ -457,6 +509,15 @@
> }
> (*pf)( "]\n");
>
> + if (pfm_current->get_num_event_masks &&
> + pfm_current->get_event_mask_name) {
> + pfm_current->get_num_event_masks(idx, &num_masks);
> + for (i = 0; i < num_masks; i++) {
> + (*pf)("Unit-mask %d: %s\n",
> + i,
> pfm_current->get_event_mask_name(idx, i));
> + }
> + }
> +
> /* print PMU specific information */
> if (pfm_current->print_info) {
> pfm_current->print_info(idx, pf);
> @@ -473,7 +534,7 @@
> pfm_print_event_info_byindex(unsigned int v, int (*pf)(const char *fmt,...))
> {
> pfmlib_regmask_t cmask, impl_counters;
> - unsigned int i, n;
> + unsigned int i, n, num_masks;
> int code;
>
> if (PFMLIB_INITIALIZED() == 0) return PFMLIB_ERR_NOINIT;
> @@ -499,6 +560,15 @@
> }
> (*pf)( "]\n");
>
> + if (pfm_current->get_num_event_masks &&
> + pfm_current->get_event_mask_name) {
> + pfm_current->get_num_event_masks(v, &num_masks);
> + for (i = 0; i < num_masks; i++) {
> + (*pf)("Unit-mask %d: %s\n",
> + v, pfm_current->get_event_mask_name(v, i));
> + }
> + }
> +
> /* print PMU specific information */
> if (pfm_current->print_info) {
> pfm_current->print_info(v, pf);
> @@ -721,6 +791,30 @@
> return PFMLIB_SUCCESS;
> }
>
> +int
> +pfm_get_max_event_mask_name_len(unsigned int event_idx, size_t *len)
> +{
> + unsigned int i, num_masks;
> + size_t l, max = 0;
> +
> + if (PFMLIB_INITIALIZED() == 0) return PFMLIB_ERR_NOINIT;
> +
> + if (event_idx >= pfm_current->pme_count || len == NULL) return
> PFMLIB_ERR_INVAL;
> +
> + if (pfm_current->get_num_event_masks &&
> + pfm_current->get_event_mask_name) {
> + pfm_current->get_num_event_masks(event_idx, &num_masks);
> + for (i = 0; i < num_masks; i++) {
> + l = strlen(pfm_current->get_event_mask_name(event_idx,
> i));
> + if (l > max) max = l;
> + }
> + }
> +
> + *len = max;
> +
> + return PFMLIB_SUCCESS;
> +}
> +
> /*
> * return the index of the event that counts elapsed cycles
> */
> @@ -768,3 +862,19 @@
> }
> return pfm_current->get_event_desc(i, str);
> }
> +
> +int
> +pfm_get_event_mask_description(unsigned int event_idx, unsigned int
> mask_idx, char **desc)
> +{
> + if (PFMLIB_INITIALIZED() == 0) return PFMLIB_ERR_NOINIT;
> +
> + if (event_idx >= pfm_current->pme_count || desc == NULL) return
> PFMLIB_ERR_INVAL;
> +
> + if (pfm_current->get_event_mask_desc == NULL) {
> + *desc = NULL;
> + return PFMLIB_SUCCESS;
> + }
> +
> + return pfm_current->get_event_mask_desc(event_idx, mask_idx, desc);
> +}
> +
> Index: lib/pfmlib_priv.h
> ===================================================================
> RCS file: /cvsroot/perfmon2/libpfm/lib/pfmlib_priv.h,v
> retrieving revision 1.3
> diff -u -r1.3 pfmlib_priv.h
> --- lib/pfmlib_priv.h 2 Jun 2006 12:55:01 -0000 1.3
> +++ lib/pfmlib_priv.h 9 Jun 2006 19:34:40 -0000
> @@ -41,7 +41,9 @@
> unsigned int flags;
> int (*get_event_code)(unsigned int i, unsigned int cnt, int
> *code);
> char *(*get_event_name)(unsigned int i);
> + char *(*get_event_mask_name)(unsigned int event_idx,
> unsigned int mask_idx);
> void (*get_event_counters)(unsigned int i, pfmlib_regmask_t
> *counters);
> + int (*get_num_event_masks)(unsigned int event_idx, unsigned
> int *count);
> int (*print_info)(unsigned int v, int (*pf)(const char
> *fmt,...));
> int (*dispatch_events)(pfmlib_input_param_t *p, void
> *model_in, pfmlib_output_param_t *q, void *model_out);
> int (*pmu_detect)(void);
> @@ -50,6 +52,7 @@
> void (*get_impl_counters)(pfmlib_regmask_t *impl_counters);
> void (*get_hw_counter_width)(unsigned int *width);
> int (*get_event_desc)(unsigned int i, char **buf);
> + int (*get_event_mask_desc)(unsigned int event_idx, unsigned
> int mask_idx, char **buf);
> } pfm_pmu_support_t;
>
> #define PFMLIB_MULT_CODE_EVENT 0x1 /* more than one code per event
> (depending on counter) */
> _______________________________________________
> 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/
