On Wed, 04 Apr 2007 17:15:43 +0400, Maxim Uvarov said: > New version of this patch. Please flay it. > > > Signed-off-by: Max Uvarov <[EMAIL PROTECTED]>
> Index: linux-2.6.18/fs/proc/array.c
> ===================================================================
> --- linux-2.6.18.orig/fs/proc/array.c
> +++ linux-2.6.18/fs/proc/array.c
> @@ -295,6 +295,20 @@ static inline char *task_cap(struct task
> cap_t(p->cap_effective));
> }
>
> +#ifdef CONFIG_THREAD_PERF_STAT
> +static inline char *task_perf(struct task_struct *p, char *buffer)
> +{
> +#ifdef THREAD_PERF_STAT_SYSC
Missed the CONFIG_ here.
> + return buffer + sprintf(buffer, "Nvcsw:\t%lu\n"
> + "Nivcsw:\t%lu\n",
> + cap_t(p->nvcsw),
> + cap_t(p->nivcsw));
cap_t()??!? That's from include/linux/capability.h, and looks something like:
#ifdef STRICT_CAP_T_TYPECHECKS
#define cap_t(x) (x).cap
#else
#define cap_t(x) (x)
#endif
and you're probably picking up the second part, making it a no-op. And if you
ever hit the first part of that ifdef, you'll throw a compile error.
Somebody else can comment on the use of #ifdef - we tend to frown on it inside
open C code, but I'm not seeing a really brilliant way to avoid them entirely
(the 'static inline task_perf' can probably move to a .h, but it's hard to
find a clean way to avoid the ifdefs given that we have Kconfig variables
to select it. array.c already has a CONFIG_S390 in it, anyhow. :)
There's a mostly-hypothetical race between inc_syscall() and the places that
increment the context switch counters, and where we read the values - but at
worst, we'll output a stale off-by-one-ish value. Certainly not worth
grabbing a lock on the task struct for *this* usage, but the sort of thing you
want to keep in mind as you write other code.
Other random comments:
1) You probably want to rebase against something more recent (2.6.21-rc<mumble>
or the final .21 when it's released).
2) It arrived here with some line-wrapping damage, most likely to the fact
that you posted it with Thunderbird. There's a mystic Thunderbird incantation
to make it not do that, but I have no idea what it is - it's in the list
archives someplace.
pgprOong3K6a3.pgp
Description: PGP signature
