Messages in this thread | | | Date | Mon, 31 Jul 2006 16:23:31 -0400 | From | Shailabh Nagar <> | Subject | Re: [patch 1/3] add basic accounting fields to taskstats |
| |
Jay Lan wrote: > This patch adds a few basic accounting fields to the taskstat > struct and a bacct_add_tsk() routine to fill the data on > exit. > > > Signed-off-by: Jay Lan <jlan@sgi.com> > > > ------------------------------------------------------------------------ > > Index: linux/include/linux/taskstats.h > =================================================================== > --- linux.orig/include/linux/taskstats.h 2006-07-31 11:38:54.132326042 -0700 > +++ linux/include/linux/taskstats.h 2006-07-31 11:42:10.634609610 -0700 > @@ -2,6 +2,7 @@ > * > * Copyright (C) Shailabh Nagar, IBM Corp. 2006 > * (C) Balbir Singh, IBM Corp. 2006 > + * (C) Jay Lan, SGI, 2006 > * > * This program is free software; you can redistribute it and/or modify it > * under the terms of version 2.1 of the GNU Lesser General Public License > @@ -29,13 +30,18 @@ > * c) add new fields after version comment; maintain 64-bit alignment > */ > > -#define TASKSTATS_VERSION 1 > +#define TASKSTATS_VERSION 2 > +#define TASK_COMM_LEN 16 > > struct taskstats { > > /* Version 1 */ > __u16 version; > - __u16 padding[3]; /* Userspace should not interpret the padding > + __u8 ac_flag; /* Record flags */ > + __u8 ac_nice; /* task_nice */ > + __u8 ac_sched; /* Scheduling discipline */ > + __u8 ac_pad;
[Minor comment] The choice of fields to fill the padding is visually separate from the delay accounting fields which follow. Can a single field of the same length, say ac_uid, be put here instead ?
Also, is the ac_ prefix needed for the common fields like comm, uid, gid, pid that are pretty much coming straight out of task_struct ?
> + __u16 padding; /* Userspace should not interpret the padding
Combine the padding's ?
> * field which can be replaced by useful > * fields if struct taskstats is extended. > */ > @@ -88,6 +94,19 @@ struct taskstats { > __u64 cpu_run_virtual_total; > /* Delay accounting fields end */ > /* version 1 ends here */ > + > + /* Basic Accounting Fields start */ > + char ac_comm[TASK_COMM_LEN]; /* Command name */ > + __u32 ac_exitcode; /* Exit status */ > + __u32 ac_uid; /* User ID */ > + __u32 ac_gid; /* Group ID */ > + __u32 ac_pid; /* Process ID */ > + __u32 ac_ppid; /* Parent process ID */ > + __u32 ac_btime; /* Begin time [sec sinec 1970] */
s/sinec/since
> + __u64 ac_etime; /* Elapsed time [usec] */ > + __u64 ac_utime; /* User CPU time [usec] */ > + __u64 ac_stime; /* SYstem CPU time [usec] */ > + /* Basic Accounting Fields end */ > }; > > > Index: linux/kernel/taskstats.c > =================================================================== > --- linux.orig/kernel/taskstats.c 2006-07-31 11:38:54.160326367 -0700 > +++ linux/kernel/taskstats.c 2006-07-31 11:44:54.952523699 -0700 > @@ -3,6 +3,7 @@ > * > * Copyright (C) Shailabh Nagar, IBM Corp. 2006 > * (C) Balbir Singh, IBM Corp. 2006 > + * (C) Jay Lan, SGI, 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 > @@ -18,6 +19,7 @@ > > #include <linux/kernel.h> > #include <linux/taskstats_kern.h> > +#include <linux/acct.h> > #include <linux/delayacct.h> > #include <linux/cpumask.h> > #include <linux/percpu.h> > @@ -172,6 +174,53 @@ static void send_cpu_listeners(struct sk > up_write(&listeners->sem); > } > > + > +#define USEC_PER_TICK (USEC_PER_SEC/HZ) > +/* > + * fill in basic accounting fields > + */ > +static void bacct_add_tsk(struct taskstats *stats, struct task_struct *tsk) > +{ > + u64 run_time; > + struct timespec uptime; > + > + /* calculate run_time in nsec */ > + do_posix_clock_monotonic_gettime(&uptime);
> + run_time = (u64)uptime.tv_sec*NSEC_PER_SEC + uptime.tv_nsec; > + run_time -= (u64)current->group_leader->start_time.tv_sec * NSEC_PER_SEC > + + current->group_leader->start_time.tv_nsec; > + do_div(run_time, NSEC_PER_USEC); /* rebase run_time to usec */ > + stats->ac_etime = run_time; > + do_div(run_time, USEC_PER_SEC); /* rebase run_time to sec */ > + stats->ac_btime = xtime.tv_sec - run_time;
Above chunk can be written more succintly using the new timespec_sub function added.
ts = timespec_sub(¤t->group_leader->start_time, &uptime); stats->ac_etime = timespec_to_ns(&ts); stats->ac_btime = xtime.tv_sec - ts.tv_sec;
Also, if there's any chance of run_time going negative due to overflow, that needs to be handled in both original and succinct code (in latter you can check for ts->tv_sec being negative).
> + if (thread_group_leader(tsk)) { > + stats->ac_exitcode = tsk->exit_code; > + if (tsk->flags & PF_FORKNOEXEC) > + stats->ac_flag |= AFORK; > + } > + if (tsk->flags & PF_SUPERPRIV) > + stats->ac_flag |= ASU; > + if (tsk->flags & PF_DUMPCORE) > + stats->ac_flag |= ACORE; > + if (tsk->flags & PF_SIGNALED) > + stats->ac_flag |= AXSIG; > + stats->ac_nice = task_nice(tsk); > + stats->ac_sched = tsk->policy; > + stats->ac_uid = tsk->uid; > + stats->ac_gid = tsk->gid; > + stats->ac_pid = tsk->pid; > + stats->ac_ppid = (tsk->parent) ? tsk->parent->pid : 0; > + stats->ac_utime = tsk->utime * USEC_PER_TICK; > + stats->ac_stime = tsk->stime * USEC_PER_TICK; > + /* Each process gets a minimum of a half tick cpu time */ > + if ((stats->ac_utime == 0) && (stats->ac_stime == 0)) { > + stats->ac_stime = USEC_PER_TICK/2; > + } > + > + strncpy(stats->ac_comm, tsk->comm, sizeof(stats->ac_comm)); > +} > + > + > static int fill_pid(pid_t pid, struct task_struct *pidtsk, > struct taskstats *stats) > { > @@ -200,6 +249,9 @@ static int fill_pid(pid_t pid, struct ta > delayacct_add_tsk(stats, tsk); > stats->version = TASKSTATS_VERSION; > > + /* fill in basic acct fields */ > + bacct_add_tsk(stats, tsk); > +
Should be added before stats->version is filled since that is a generic field, not delay accounting specific.
> /* Define err: label here if needed */ > put_task_struct(tsk); > return rc; >
- 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/
| |