On Tue, Oct 07, 2008 at 11:46:53AM +0200, stephane eranian wrote: > David, > > On Tue, Oct 7, 2008 at 5:56 AM, David Gibson > <[EMAIL PROTECTED]> wrote: > > On Sun, Oct 05, 2008 at 11:23:15PM +0200, stephane eranian wrote: > >> I see, so you were just decoupling to double the number of available bits. > >> I guess we have a minimum of 32 bits. It seems overkill to support up to > >> 32 'types'. We could force flags to uint64_t. But then we would have to do > >> the same for all other syscalls to be consistent. Defining flags as long > >> won't work because of 32-bit vs. 64-bit ABI compatibility issues. > > > > I guess so. I guess there's also the possibility that you might later > > have some flags that can apply to many of the syscalls, and it would > > be nice to have them in a standard bit position for all calls. That > > might make bit allocation a bit interesting if you're squeezing type > > into the same field. > > > I can see that for subsets of the calls, e.g., read and write of registers. > It is not clear to me what kind of 'service' would apply to all calls. > > As Corey suggested yesteday, the flags for read and write operations > are not just single bit. A portion of the 64 bits, 8 in his proposal, are used > to encode the 'type' of register (PMD, PMC, PMD_ATTR). That simplifies > checking for valid bits in this portion of flags.
Yes, that was always what I had in mind if flags and type were combined. > OTOH, if I go back to your original proposal: > int pfm_write_pmrs(int fd, int flags, int type, void *v, size_t); > > The 'type' field would have the same encoding as proposed by Corey. > 32 bits would be plentyful for types. Flags could remain at 32-bits > across the board, thereby avoiding the problems outlined by Arnd > in his follow-up message. Separating the type also makes it look > less like an ioctl(). So I think we should go with that proposal. Works for me. > [snip] > >> > >> If I summarize our discussion. It seems we can define the API as follows: > >> > >> int pfm_create_session(int fd, uint64_t flags, pfarg_sinfo_t *sif, > >> [ char *smpl_name, void *smpl_arg, size_t arg_size]); > >> int pfm_read_pmrs(int fd, uint64_t flags, void *tab, size_t sz); > >> int pfm_write_pmrs(int fd, uint64_t flags, void *tab, size_t sz); > >> int pfm_attach_session(int fd, uint64_t flags, int target); /* > >> attach, detach with target=-1 */ > >> int pfm_control_session(int fd, uint64_t flags); /* for start/stop */ > > > > Unless you have other functions in mind for this, I'd suggest a name > > that's a little less generic here. pfm_set_running() maybe. > > > What about pfm_set_state() or pfm_set_session_state()? I guess so. > >> int pfm_control_sets(int fd, uint64_t flags, void *sets, size_t > >> sz); > > > > Hrm. This now has identical signature to the {read,write}_pmrs. I > > wonder if these could be sensibly combined. Maybe have > > pfm_get_info(), pfm_set_info() which can get/set control structures > > for several different namespaces: PMCs PMDs and now event sets. > > > I think this would be too generic as it would be mixing very different > data types. This is not the case for pfm_write_pmrs() and pfm_read_pmrs() > because you are always passing 'register descriptions'. > > There is a fundamental difference between pfm_control_sets() and > pfm_write_pmrs()/pfm_read_pmrs(). For the latter, the 'action' is hardcoded > in the name of the syscall, i.e., read or write, what varies is whether or not > we are passing PMD or PMC. For the former, the 'action', i.e., create new > set or get info about a set, is hardcoded in the flags, and it determines the Uh.. I was assuming if combined we'd drop the selecting flag from control_sets() and instead make the pfm_read_stuff() call be the get_info() equivalent when used on the eventsets space and pfm_write_stuff() be the create/modify sets call. The fact that the structure is different for read/write in this case is a bit funny. In fact it would probably be best to have different 'type' fields for read and write too (EVTSET_INFOm only usable for reads , and EVTSET_CONTROL only for writes, maybe). I'm not dead-set on combining these calls, but it seems to me we've already just about made the read/write calls into a read/write array of control structures in some namespace call, which pretty much is what the event set call is too. -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson ------------------------------------------------------------------------- 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