Re: [PATCH 1/6] 2.6.16-rc1 perfmon2 patch for review

["Bryan O'Sullivan" <bos@serpentine.com>]

On Mon, 2006-01-23 at 06:23 -0800, Stephane Eranian wrote:

> The Carta algorithm is very simple and provides good enough
> randomization.

Fair enough.

> I did some search and could not find the message thread you are referring
> to. Could you provide some URL?

http://marc.theaimsgroup.com/?l=linux-kernel&m=113709100114894&w=2

> The data structure is indeed shared with user space. I do not reuse
> the same perfmon.h for user applications. Instead, I have a "user"
> perfmon.h (that would eventually migrate to libc). It defines the field
> using the uint32_t, uint64_t notation instead.

It would probably be helpful to annotate those types with the __u32 and
so on forms, to make it clear that userspace will be using them.

> We had a long discussion with David Gibson about the bitmask reg_*pmds[]
> in the context of ABI issues on platforms running a 64-bit OS but with
> both a 32-bit and 64-bit  ABIs. The number of bits in the bitmask is fixed.
> When using unsigned long the number of elements in the reg_*_pmds[] array
> differ between 32-bit and 64-bit but it does work.

How does it work?  Is the value of PFM_PMD_BV different depending on
whether it's a 32-bit or 64-bit arch?  That makes me pretty
uncomfortable.

Also, you should drop the typedef of pfarg_pmd_t and use "struct
pfarg_pmd" instead.  Likewise for all structs that are not intended to
be opaque.

> I must admit I don't quite follow the arguments about/sys vs. /proc if
> that is what you are asking for. I'd like to understand better why 
> put entries in one vs. the other.

Please see my prior message.

> > > +/* use for IA-64 only */
> > > +#ifdef __ia64__
> > > +#define pfm_release_dbregs(_t) 		do { } while (0)
> > > +#define pfm_use_dbregs(_t)     		(0)
> > > +#endif
> > 
> > Please move this to asm-ia64/perfmon.h, then.

> Well the issue is that this part is used whenever CONFIG_PERFMON is not
> selected. perfmon.h includes <asm/perfmon.h> but only when CONFIG_PERFMON
> is selected. I would have to move asm/perfmon.h at the top of perfmon.h
> but it depends on some definitions in perfmon.h.

Ugh.

> But perfmon2 also provides kernel level sampling buffer
> for per-thread sampling. There the buffer is attached to the thread not the processor
> it is running on. That's where this becomes very complicated with relayfs.

I don't see the difficulty.  Sure, you end up having to look at each
relayfs buffer in userspace instead of a single one, but can't libpfm
hide that from its consumers?  Surely having a finer granularity of data
like this ("I spent 75% of my time on this CPU") ends up being more
useful, no?

	<b