Shailabh Nagar <[email protected]> wrote:
>
> delayacct-setup.patch
>
> Initialization code related to collection of per-task "delay"
> statistics which measure how long it had to wait for cpu,
> sync block io, swapping etc. The collection of statistics and
> the interface are in other patches. This patch sets up the data
> structures and allows the statistics collection to be disabled
> through a kernel boot paramater.
>
> ...
>
> + delayacct [KNL] Enable per-task delay accounting
> +
Why does this boot parameter exist?
The code is neat-looking.
> --- /dev/null 1970-01-01 00:00:00.000000000 +0000
> +++ linux-2.6.16/kernel/delayacct.c 2006-03-29 18:12:57.000000000 -0500
> @@ -0,0 +1,92 @@
> +/* delayacct.c - per-task delay accounting
> + *
> + * Copyright (C) Shailabh Nagar, IBM Corp. 2006
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it would be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See
> + * the GNU General Public License for more details.
> + */
> +
> +#include <linux/sched.h>
> +#include <linux/slab.h>
> +#include <linux/time.h>
> +#include <linux/sysctl.h>
> +#include <linux/delayacct.h>
> +
> +int delayacct_on __read_mostly = 0; /* Delay accounting turned on/off */
Yes, it should be __read_mostly. But it shouldn't be initialised to zero.
> +void __delayacct_tsk_init(struct task_struct *tsk)
> +{
> + tsk->delays = kmem_cache_alloc(delayacct_cache, SLAB_KERNEL);
> + if (tsk->delays) {
> + memset(tsk->delays, 0, sizeof(*tsk->delays));
> + spin_lock_init(&tsk->delays->lock);
> + }
> +}
We have kmem_cache_zalloc() now.
> +void __delayacct_tsk_exit(struct task_struct *tsk)
> +{
> + if (tsk->delays) {
> + kmem_cache_free(delayacct_cache, tsk->delays);
> + tsk->delays = NULL;
> + }
> +}
delayacct_tsk_exit() already checked tsk->delays.
> +/*
> + * Finish delay accounting for a statistic using
> + * its timestamps (@start, @end), accumalator (@total) and @count
> + */
> +
> +static inline void delayacct_end(struct timespec *start, struct timespec *end,
> + u64 *total, u32 *count)
> +{
> + struct timespec ts;
> + nsec_t ns;
> +
> + do_posix_clock_monotonic_gettime(end);
> + ts.tv_sec = end->tv_sec - start->tv_sec;
> + ts.tv_nsec = end->tv_nsec - start->tv_nsec;
> + ns = timespec_to_ns(&ts);
> + if (ns < 0)
> + return;
> +
> + spin_lock(¤t->delays->lock);
> + *total += ns;
> + (*count)++;
> + spin_unlock(¤t->delays->lock);
> +}
It's strange to have a static inline function at the end of a .c file. I
guess this gets used in later patches.
It looks rather too big to be inlined.
I'm surprised that we don't already have a timeval_sub() function
somewhere.
The code you have there will cause ts.tv_nsec to go negative half the time.
It looks like timespec_to_ns() will happily fix that up for us, but it's
all a bit fragile.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
