Corey, On Mon, Oct 6, 2008 at 8:48 PM, Corey J Ashford <[EMAIL PROTECTED]> wrote: > Thanks for the reply, Stephane. I have a follow-up comment: > > "stephane eranian" <[EMAIL PROTECTED]> wrote on 10/05/2008 04:15:39 > PM: > >> Corey, >> >> On Mon, Oct 6, 2008 at 12:31 AM, Corey J Ashford <[EMAIL PROTECTED]> > wrote: >> > "stephane eranian" <[EMAIL PROTECTED]> wrote on 10/05/2008 > 02:23:15 >> > PM: >> > [snip] >> > Just as a general comment to this revision, I think this has gone too > far >> > toward an ioctl call; I prefer stronger type checking at the API. This >> > has almost none. Also, it's more self-documenting if the type that > you >> > need to pass is explicit in the prototype. >> > >> I understand your concerns, I have some worries about this also. >> >> You cannot get the best of both worlds: strong checking vs. >> flexibility & extensibility without new syscalls >> >> I think the pfm_control_sets() may be the one where I went quite far. >> >> One thing suggested by David and which is part of this new version >> is the duplication between pfarg_pmr_t and pfarg_pmd_attr. I think >> passing only one of the two instead of two like me original proposal >> makes it easier to manage in your app and also in the kernel. Yet >> it maintains the same level of flexibility. You simply count, you can >> use the minimalistic pfarg_pmr_t. You are sampling using kernel >> support, then you pass pfarg_pmd_attr_t for PMDs. Doing this >> unfortunately has the side effect of introducing a void *, which >> inevitably weakens type checking. > > Yes, I see the trade-off. Flexibility long-term is important if you > expect to be having a lot of new capability and supporting new (perhaps > bizarro) architectures down the road. > > I had one other thought on the way in this morning. For pfm_write_pmrs, > for example, the flags parameter currently specifies a single bit per > type, is that correct? It occurred to me that this is probably not a good > use of a bit vector, because it doesn't make sense to have more than one > bit on at a time (if I interpret the proposal correctly). It would also > mean that you'd have to have some code to check for that error case > (multiple type bits on). So instead of using a bit vector, how about a > type code? That way, the user cannot specify more than one type. Plus, > it would require far fewer bits, and provide lots of room for later > expansion. For example, you could use just 8 bits (say) for the type code > and leave the other 24 bits reserved. > > What do you think? > This is what I have implemented in the code last week (following the discussion with David Gibson). I ran into the same issues you outline above: waste of bits, harder to check. So I reserved 4 bits for now, but 8 seems more reasonable.
I have also extended flags to be 64 bit wide, instead of 32, so here is ample room for extensions. So I think we agree on that point. ------------------------------------------------------------------------- This SF.Net email is sponsored by the Moblin Your Move Developer's challenge Build the coolest Linux based applications with Moblin SDK & win great prizes Grand prize is a trip for two to an Open Source event anywhere in the world http://moblin-contest.org/redirect.php?banner_id=100&url=/ _______________________________________________ perfmon2-devel mailing list perfmon2-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/perfmon2-devel
