[lkml]   [2008]   [Oct]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
SubjectRe: perfmon3 interface overview

On Fri, Oct 3, 2008 at 8:17 AM, David Gibson
<> wrote:
> > The new API now has 8 system calls in its fully-featured
> > version. Many data structures shared with user level have been
> > abandoned in favor of explicit syscall parameters. Each syscall has
> Ah, excellent, big structures passed through an interface are often a
> pain to deal with, so I'm glad these are going.


> I, at least, like the changes that have been made here. I do have a
> few suggestions for further simplifications along the same lines
> though, see below.
> [snip]
> > II) programming the registers
> >
> > With v2.81:
> > int pfm_write_pmcs(int fd, pfarg_pmc_t *pmds, int n);
> > int pfm_write_pmds(int fd, pfarg_pmd_t *pmcs, int n);
> > int pfm_read_pmds(int fd, parg_pmd_t *pmds, int n);
> >
> > With v3.0:
> > int pfm_write_pmrs(int fd, int flags, pfarg_pmr_t *pmrs, int n);
> > int pfm_write_pmrs(int fd, int flags, pfarg_pmr_t *pmrs, int n,
> > pfarg_pmd_attr_t *pmas);
> >
> > int pfm_read_pmrs(int fd, int flags, pfarg_pmr_t *pmrs, int n);
> > int pfm_read_pmrs(int fd, int flags, parg_pmr_t *pmrs, int n,
> > pfarg_pmd_attr_t *pmas);
> I would suggest adding 'type' and 'size' (int) parameters, and folding
> the pmrs and pmas arrays into one freeform array. 'size' gives the
> size in bytes of the array elements, 'type' selects the structure of
> each array element. This means:
> - if you ever have some sort of registers to access that are
> neither PMCs nor PMDs, you can access those two, using a new type
> value.
> - if a new method of operation wants to supply different
> information for PMCs or PMDs, you can define a new structure and type
> value (one obvious example would be a minimal (num,value) only
> structure for minimum latency on simple applications).
> - an explicit size value means things can marshal / copy /
> whatnot the parameters without having to know what size each type
> implies - the code will stay valid when new types / structures are
> defined.

So you are suggesting something along the lines:

int pfm_read_pmrs(int fd, int flags, int type, void *tab, size_t sz);
int pfm_write_pmrs(int fd, int flags, int type, void *tab, size_t sz);

I have already introduced a type flag (PFM_RWFL_PMD, PFM_RWFL_PMC).
Why separate the type into its own parameter?

As for the freeform array, isn't that what people do not like because
of void *
and thus weak type checking?

I will look at switching to size instead of count. I think it does make sense.

> [snip]
> > III) attaching and detaching
> >
> > With v2.81:
> > int pfm_load_context(int fd, pfarg_load_t *load);
> > int pfm_unload_context(int fd);
> >
> > With v3.0:
> > int pfm_attach_session(int fd, int flags, int target);
> > int pfm_detach_session(int fd, int flags);
> Couldn't you get rid of one more syscall here by making detach a
> special case of attach with a special "null" value for target, or a
> special flag?

We could combine the two and use the flag field to indicate attach/detach.
The target is not a pointer but an int. Some people suggested I use an
unsigned long instead. In anycase, we could not use 0 to indicate "detach"
because CPU0 is a valid choice for system-wide. Thus we would have to
pick another value to mean "nothing", e.g, -1.

> IV) starting and stopping
> With v2.81:
> int pfm_start(int fd, pfarg_start_t *st);
> int pfm_stop(int fd);
> int pfm_restart(int fd);
> With v3.0:
> int pfm_start_session(int fd, int flags);
> int pfm_stop_session(int fd, int flags);

> Likewise, couldn't you cut this down by one more syscall by making it
> int pfm_set_session_state(int fd, int flags);
> and having a 'RUNNING' flag, which selects start or stop behaviour?

That one we can certainly do. That's a good idea.

Thanks for your feedback, keep it coming.

 \ /
  Last update: 2008-10-03 13:03    [W:0.124 / U:2.000 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site