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

Stephen Rothwell
2008-07-28T15:47:47Z

[-- Attachment #1: Type: text/plain, Size: 1624 bytes --]

Hi Stephane,

On Mon, 30 Jun 2008 06:13:26 -0700 (PDT) eranian@googlemail.com 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@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]