Re: [patch 00/19] perfmon2 minimal v3: introduction

[Stephen Rothwell]

Hi Stephane,

On Mon, 30 Jun 2008 06:13:26 -0700 (PDT) eranian@xxxxxxxxxxxxxx wrote:
>
> The following patches implement a minimal perfmon2 subsystem which provides
> access to the hardware performance counters of modern processors.

First up, I cannot speak to the actual function of the patches, but just
to the mechanics of getting them into the kernel.  One good thing is that
with CONFIG_PERFMON turned off, it looks like the impact is very close to
zero. Two obvious things that would help these being accepted are:

	ordering - in the current incarnation, the patches produce places
where the kernel will not build.  We like things to be ordered so that
git bisection does not fail if at all possible - that means that an
allmod/yesconfig should build after the application of each patch.
Currently this is not so.  It could be mode to do so trivially by
delaying the "global" Kconfig/Makefile updates until later in the
sequence.  And you really should introduce the system calls *before*
wiring them up.

	quite a few of the macros (especially for the dummy (non
CONFIG_PERFMON) versions of functions) should be "static inline"
functions unless they absolutely must be macros.  Andrew Morton said it
best - "write in C not C preprocessor".

I can see that this infrastructure could be very useful and I appreciate
that it has been cut down for this initial merge.  These patches (even as
they are) compare favourably to some other new infrastructure that has
been introduced.

-- 
Cheers,
Stephen Rothwell                    sfr@xxxxxxxxxxxxxxxx
http://www.canb.auug.org.au/~sfr/

Attachment: pgp00000.pgp
Description: PGP signature