Message-ID: <20081005055339.GC469@yookeroo.seuss>
Date:	Sun, 5 Oct 2008 16:53:39 +1100
From:	David Gibson <david@...son.dropbear.id.au>
To:	eranian@...il.com
Cc:	linux-kernel@...r.kernel.org,
	perfmon2-devel <perfmon2-devel@...ts.sourceforge.net>
Subject: Re: perfmon3 interface overview

On Fri, Oct 03, 2008 at 12:58:12PM +0200, stephane eranian wrote:
> David,
> 
> On Fri, Oct 3, 2008 at 8:17 AM, David Gibson
> <david@...son.dropbear.id.au> 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.

[snip]
>  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);

Uh.. maybe.. there are actually several possible variants all of which
would meet the general idea I had in mind.

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

Combining the type into the flags is certainly a possibility.  I was
just a bit worried that if you eventually have enough actual flag
flags, in addition to the type values, you might start running out of
bits.

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

Yeah; this is an interesting tradeoff of flexibility versus
predictability.  Actually, what I originally had in mind was
seperately passing both 'n' and a per-element size.  That provides a
bit more defined structure to the void * - it must be an array of n
identical, fixed-size elements.  The internal structure of each
element is defined only by type, but it is assumed to contain no
pointers to further chained structures (i.e. its safe for wrapper code
to do shallow copies of the array).

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

Yeah.  As I said I was originally thinking of keeping the 'n'
parameter, and adding an element size parameter.  But just having an
overall size is also a possibility - it gives less definition to the
internal structure but at least things can marshal or copy the whole
array parameter without having to know its internals.

[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.

Sorry, I didn't express myself well.  I realise it's an integer, and
didn't mean an actual NULL, but as you say a special integer value
with a function similar to NULL.  -1 is the most obvious choice.

-- 
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists