Re: Performance Stats: Kernel patch

From: Jesper Juhl
Date: Wed Apr 04 2007 - 09:59:27 EST


On 04/04/07, Maxim Uvarov <muvarov@xxxxxxxxxxxxx> wrote:
Hello again,

[snip]
New version of this patch. Please flay it.


A few small comments below.


+config THREAD_PERF_STAT
+ bool "Per-process (thread) performance statistics"
+ depends on (X86 || PPC || MIPS)
+ help
+ Make available to the user the following new per-process
(thread) performance statistics:

Get rid of the word "new" - it won't continue to be a "new" feature forever :-)

+ * Involuntary Context Switches
+ * Voluntary Context Switches
+ * Number of system calls (option)

In the first two lines you Capitalize Each Word, but in the last line
you don't - why?
Shouldn't the first two have "Number of" prepended?
Perhaps it's just me, but I would write "(optional)" instead of "(option)".

In short, I'd suggest this :

* Number of involuntary context switches
* Number of voluntary context switches
* Number of system calls (optional)

+ This information is available via /proc/PID/status.

What about tools that currently parse /proc/PID/status ? Don't you
risk breaking userland stuff by changing this file? Wouldn't it be
better to use a new file?

+
+config THREAD_PERF_STAT_SYSC
+ bool "enable syscall counter"

Capitalize the first word; "Enable syscall counter" .

+ depends on THREAD_PERF_STAT
+ help
+ This option add syscalls counter to /proc/PID/status.

I'd probably have written "This option adds a syscall counter to
/proc/PID/status."


--
Jesper Juhl <jesper.juhl@xxxxxxxxx>
Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please http://www.expita.com/nomime.html
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/