Messages in this thread | | | Date | Tue, 29 Jul 2008 01:47:30 +1000 | From | Stephen Rothwell <> | Subject | Re: [patch 00/19] perfmon2 minimal v3: introduction |
| |
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/ [unhandled content-type:application/pgp-signature] | |