Messages in this thread | | | Date | Thu, 21 Oct 2004 19:16:08 -0700 | From | Andrew Morton <> | Subject | Re: [Lse-tech] [PATCH 2.6.9 1/2] enhanced accounting data collection |
| |
Jay Lan <jlan@engr.sgi.com> wrote: > > 1/2: acct_io > > Enahanced I/O accounting data collection. >
It's nice to use `diff -p' so people can see what functions you're hitting.
> + current->syscr++;
Should these metrics be per-thread or per-heavyweight process?
> + if (ret > 0) { > + current->rchar += ret; > + }
It's conventional to omit the braces if there is only one statement in the block.
> =================================================================== > --- linux.orig/include/linux/sched.h 2004-10-01 17:01:21.412848229 -0700 > +++ linux/include/linux/sched.h 2004-10-01 17:09:42.723482260 -0700 > @@ -591,6 +591,9 @@ > struct rw_semaphore pagg_sem;
There is no `pagg_sem' in the kernel, so this will spit a reject.
> #endif > > +/* i/o counters(bytes read/written, #syscalls */ > + unsigned long rchar, wchar, syscr, syscw; > +
These will overflow super-quick. Shouldn't they be 64-bit?
> --- linux.orig/kernel/fork.c 2004-10-01 17:01:21.432379595 -0700 > +++ linux/kernel/fork.c 2004-10-01 17:09:42.732271376 -0700 > @@ -995,6 +995,7 @@ > p->real_timer.data = (unsigned long) p; > > p->utime = p->stime = 0; > + p->rchar = p->wchar = p->syscr = p->syscw = 0;
We generally prefer
p->rchar = 0; p->wchar = 0; etc.
yes, the code which is there has already sinned - feel free to clean it up while you're there ;)
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |