Hi Stephane,
stephane eranian wrote:
> Corey,
>
> On Wed, Nov 25, 2009 at 8:56 PM, Corey Ashford
> <cjash...@linux.vnet.ibm.com> wrote:
>>> Ok, I see. The issue here is that you are assuming that attribute values
>>> are all integers. The way it is setup right now, an attribute can have any
>>> type, incl. a string.
>> In my original email, I had a parenthetical question/comment about
>> attributes which have string values. In the libpfm code, though, the
>> attribute type appears only to have the choices of umask, boolean, or
>> integer:
>>
>> typedef enum {
>> PFM_ATTR_NONE=0, /* no attribute */
>> PFM_ATTR_UMASK, /* unit mask */
>> PFM_ATTR_MOD_BOOL, /* register modifier */
>> PFM_ATTR_MOD_INTEGER, /* register modifier */
>>
>> PFM_ATTR_MAX /* end-marker */
>> } pfm_attr_t;
>>
>> So I am sure I am missing something somewhere.
>>
> This could be extended with PFM_ATTR_MOD_STRING for instance.
>
>> That said, we could also support strings by returning, in the array, a
>> pointer to the string instead of a boolean/integer value. In this case, it
>> would be convenient to return an array of pfm_attr_t values as well, so that
>> it would be easier for the tool to display the values.
>>
> Yes. But this is getting complicated.
>
>>> If I look at the X86 event tables, some events or unit masks do have
>>> certain modifiers hardcoded. For instance,
>>>
>>> UOPS_ISSUED:STALLED_CYCLES has i=1, cmask=1
>>>
>>> This unit mask does not exist in the official documentation, but it is
>>> added for *convenience* because this is a common usage of the event.
>>>
>>> With existing libpfm4, if you try to pass
>>>
>>> UOPS_ISSUED:STALLED_CYCLES:cmask=2
>>>
>>> It will fail. It is not clear to me what knowing that cmask=1 would do
>>> in your tool. I understand if you are listing events. The tool could
>>> obviously check that the user is not passing an already hardcoded
>>> attributes, but then the library does that work already.
>> In this case you are right, it wouldn't provide anything useful, and these
>> defaults might be excluded from the return values, since they are not
>> modifiable.
>>
> Ok, so you are looking at this to set some default modifiable attributes
> such as a default unit mask. For instance, you'd like to have it such that
> if I were to pass only UOPS_ISSUED, the library would not reject but
> instead it would use a default unit mask. Note that it does that already, but
> without telling you explicitly which one it picked, i.e., there is no API
> to retrieve that.
Yes, that's the idea. The writer of the library knows what's wanted most
often,
but can't state categorically that that choice is what will always be wanted.
In this case, there are two solutions, I think:
1) Expose the defaults, as I've suggested.
2) Force the caller to pick one of the options.
Neither option is that great.
>
>>> It seems that it you pull this knowledge out of the library you will end
>>> up duplicating some of the work of the library in your tool.
>> Of course the user can choose to ignore what the specific defaults are, in
>> which case nothing changes in the usage model.
>>
>> The issue I'm facing is that we have events for which you can choose one
>> attribute value or another, and it's not always clear which value the user
>> would want to use. And this can occur on more than one attribute for an
>> event. I want the user to be able to determine what value the library has
>> chosen for the default, rather than just shield him from the choices it will
>> make.
>>
>
> That seems contradictory to me. You are saying it is not clear which one
> the user would want to use and then you say the library would pick a default.
> How could the library make a better choice than the user? If an event has two
> distinct attributes to choose from and they don't do the same thing, then it
> is
> up to the user to choose one.
Is that the case for the UOPS_ISSUED case you gave above? Are there other
choices for valid umasks besides the default that the library chooses?
>
> It seems to me you have to systematically do an a priori check for each
> event you want to measure:
>
> idx = pfm_find_event("UOPS_ISSUED");
> pfm_get_event_dfl_attrs(idx, -1, &info, &n);
>
> And then what?
> You reconstruct the event string with all the defaults.
> Assuming the default is a unit mask, STALLED_CYCLES.
> Then info would contain:
> info->name = "STALLED_CYCLES";
> info->type = PFM_ATTR_UMASK;
>
> You would have to scan the event string to see if there is a unit
> mask. If there is, then you ignore the default. Otherwise you would
> append the default unit mask to your string and eventually call
> pfm_get_perf_event_encoding().
I think the user would do this extra work, perhaps, when he realizes he's not
getting the expected results from measuring a particular event.
In this case, instead of programmatically doing the scan and constructing a new
event string, the user would simply look at the defaults, as shown by some tool
like showevtinfo and then say, "oh! it's counting cycles while asserted instead
of edges... no wonder. I'll just append an ':e=1' to the event string."
>
> I am wondering what you do when the number of defaults returned
> by the call > 1. I would think you could only get one UMASK attr
> but possibly multiple modifiers PFM_ATTR_MOD_*. I would assume
> those are all compatible and are meant to be used together, i.e., that
> is what the library would do internally.
Yes, it would be the same set of consistent defaults that the library would use
internally. In fact, I had in mind that the library could actually use this
function internally to set the defaults, instead of coding them "ad hoc" for
each processor.
>
>> I think this may be important when the user looks at his results and
>> scratches his head... "Why am I getting this count that is obviously not
>> what I wanted to count? Is the tool counting cycles asserted instead of
>> edges?" etc. Then he has to go find the attributes, and try forcing them to
>> what he thinks they should be to see if it makes a difference.
>>
>> One other issue is the notion of mandatory attributes. For some events, we
>> are going to need the user to specify a couple of attributes which supply a
>> bit mask. These masks would have no default value. It would be nice for
>> the tool to have some way of sensing required attributes.
>>
> There is already that kind of problems on X86 which some events which do
> requires some unit masks modifiers. For instance, the cache events needs
> MESI modifiers. The library currently checks if some MESI bits are specified
> by user, if not it sets them all, i.e., any cache line state. That seems like
> a
> reasonable default.
>
>> So maybe the interface could be generalized a bit more so that instead of
>> returning separate arrays, we return a data structure with all of the
>> interesting details about the attribute:
>>
>> struct pfm_attr_info_t {
>> char *name;
>
> Why not just return the attr index, and then the user can use
> pfm_get_event_attr(), pfm_get_event_attr_desc().
>
>> char *desc;
>
> May be overkill.
>
>> pfm_attr_t type;
>
>> int has_default; /* boolean */
>
> I don't get has_default?
Just means, this attribute as a default value. In other words, if this boolean
is false, you must explicitly supply the attribute, e.g. ':e=0'. If it is
true,
the following union contains the default value.
>
>> union {
>> const char *dfl_str;
>> int dfl_bool;
>> int dfl_int;
>> };
>> int mandatory; /* 1 = mandatory, 0 = optional */
On second thought, when has_default == 0, it's the same as setting mandatory to
1. I think mandatory is not needed.
>> }
>>
>> It would be nice to supply information about the allowed values of the
>> attribute, but I don't want to get too carried away here.
>
> That would be complicated. This could be part of the description to guide the
> user. There are already some sanity checks on boolean.
That's a good point.
>> So this would be the proposed api call:
>>
>> pfm_get_event_attr_info(int idx, int attr_idx, pfm_attr_info_t *attr_infos,
>> int *n_attrs);
>>
>> I agree, that's certainly one easy-to-use solution to the plm problem. I
>> was just thinking if we had a more general-purpose mechanism, that could be
>> used for all attributes instead of just plm, that we could kill two birds
>> with one stone.
>>
> understood.
--
Regards,
- Corey
Corey Ashford
Software Engineer
IBM Linux Technology Center, Linux Toolchain
Beaverton, OR
503-578-3507
cjash...@us.ibm.com
------------------------------------------------------------------------------
Join us December 9, 2009 for the Red Hat Virtual Experience,
a free event focused on virtualization and cloud computing.
Attend in-depth sessions from your desk. Your couch. Anywhere.
http://p.sf.net/sfu/redhat-sfdev2dev
_______________________________________________
perfmon2-devel mailing list
perfmon2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/perfmon2-devel
