Skip to contentSkip to conversation

basic accounting over taskstats

PATCH In review Jay Lan 4 participants 5 messages Aug 3, 2006 Taskstats statistics interface

lore
Share & tools
lore

Jay Lan

author
#0 lore raw

PATCH 1/3 basic accounting over taskstats

This patch is to replace the "[patch 1/3] add basic accounting fields to taskstats" posted on 7/31.

This patch adds some basic accounting fields to the taskstats struct, add a new kernel/tsacct.c to handle basic accounting data handling upon exit. A handle is added to taskstats.c to invoke the basic accounting data handling.

include/linux/taskstats.h +24 -5
Index: linux/include/linux/taskstats.h
===================================================================
@@ -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,16 +30,18 @@
  *	c) add new fields after version comment; maintain 64-bit alignment
  */
 
-#define TASKSTATS_VERSION	1
+
+#define TASKSTATS_VERSION	2
+#define TS_COMM_LEN		16	/* should sync up with TASK_COMM_LEN
+					 * in linux/sched.h */
 
 struct taskstats {
 
 	/* Version 1 */
 	__u16	version;
-	__u16	padding[3];	/* Userspace should not interpret the padding
-				 * field which can be replaced by useful
-				 * fields if struct taskstats is extended.
-				 */
+	__u32	ac_exitcode;	/* Exit status */
+	__u8	ac_flag;		/* Record flags */
+	__u8	ac_nice;		/* task_nice */
 
 	/* Delay accounting fields start
 	 *
@@ -88,6 +91,22 @@ struct taskstats {
 	__u64	cpu_run_virtual_total;
 	/* Delay accounting fields end */
 	/* version 1 ends here */
+
+	/* Basic Accounting Fields start */
+	char	ac_comm[TS_COMM_LEN];	/* Command name */
+	__u8	ac_sched;		/* Scheduling discipline */
+	__u8	ac_pad[3];
+	__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 since 1970] */
+	__u64	ac_etime;		/* Elapsed time [usec] */
+	__u64	ac_utime;		/* User CPU time [usec] */
+	__u64	ac_stime;		/* SYstem CPU time [usec] */
+	__u64	ac_minflt;		/* Minor Page Fault */
+	__u64	ac_majflt;		/* Major Page Fault */
+	/* Basic Accounting Fields end */
 };
 
 
kernel/taskstats.c +4 -0
Index: linux/kernel/taskstats.c
===================================================================
@@ -18,6 +18,7 @@
 
 #include <linux/kernel.h>
 #include <linux/taskstats_kern.h>
+#include <linux/tsacct_kern.h>
 #include <linux/delayacct.h>
 #include <linux/cpumask.h>
 #include <linux/percpu.h>
@@ -198,7 +199,10 @@ static int fill_pid(pid_t pid, struct ta
 	 */
 
 	delayacct_add_tsk(stats, tsk);
+
+	/* fill in basic acct fields */
 	stats->version = TASKSTATS_VERSION;
+	bacct_add_tsk(stats, tsk);
 
 	/* Define err: label here if needed */
 	put_task_struct(tsk);
kernel/Makefile +1 -1
Index: linux/kernel/Makefile
===================================================================
@@ -49,7 +49,7 @@ obj-$(CONFIG_SECCOMP) += seccomp.o
 obj-$(CONFIG_RCU_TORTURE_TEST) += rcutorture.o
 obj-$(CONFIG_RELAY) += relay.o
 obj-$(CONFIG_TASK_DELAY_ACCT) += delayacct.o
-obj-$(CONFIG_TASKSTATS) += taskstats.o
+obj-$(CONFIG_TASKSTATS) += taskstats.o tsacct.o
 
 ifneq ($(CONFIG_SCHED_NO_NO_OMIT_FRAME_POINTER),y)
 # According to Alan Modra <[email protected]>, the -fno-omit-frame-pointer is
kernel/tsacct.c new file +69 -0

69 lines not on this page · raw message

include/linux/tsacct_kern.h new file +19 -0
Index: linux/include/linux/tsacct_kern.h
===================================================================
@@ -0,0 +1,19 @@
+/*
+ * tsacct_kern.h - kernel header for system accounting over taskstats interface
+ *
+ * Copyright (C) Jay Lan	SGI
+ */
+
+#ifndef _LINUX_TSACCT_KERN_H
+#define _LINUX_TSACCT_KERN_H
+
+#include <linux/taskstats.h>
+
+#ifdef CONFIG_TASKSTATS
+extern void bacct_add_tsk(struct taskstats *stats, struct task_struct *tsk);
+#else
+static inline void bacct_add_tsk(struct taskstats *stats, struct task_struct *tsk)
+{}
+#endif /* CONFIG_TASKSTATS */
+
+#endif
#1 lore raw

include/linux/taskstats.h line 36

Quoted 11 lines · On Wed, 02 Aug 2006 21:20:53 -0700 Jay Lan <[email protected]> wrote:

This patch is to replace the "[patch 1/3] add basic accounting fields to taskstats" posted on 7/31.

This patch adds some basic accounting fields to the taskstats struct, add a new kernel/tsacct.c to handle basic accounting data handling upon exit. A handle is added to taskstats.c to invoke the basic accounting data handling.

include/linux/taskstats.h +2 -0
+#define TS_COMM_LEN		16	/* should sync up with TASK_COMM_LEN
+					 * in linux/sched.h */

There was a proposal recently to increase TASK_COMM_LENGTH from 16 to 20 so that it was long enough to hold an entire IEEE(?) UUID so that the operator can easily match up a kernel thread with the storage device which it manages.

I don't know if/when that change will happen, but the message is that TASK_COMM_LENGTH may increase.

kernel/tsacct.c line 33

Quoted 1 line
kernel/tsacct.c new file +1 -0
+	BUILD_BUG_ON(TS_COMM_LEN < TASK_COMM_LEN);

And if it does, we'll need to increase TS_COMM_LEN as well. That will amount to an non-compatible change to the interface which you are
proposing. We want to avoid that.

Hence I'd propose that you increase TS_COMM_LEN to 32 or something and if TASK_COMM_LEN later becomes really big, we might just choose to truncate it.

Or we remove this field altogether, perhaps. The same info is available from /proc/pid/stat anyway. Is it really needed?

#2 lore raw
Quoted 4 lines · Andrew Morton

Or we remove this field altogether, perhaps. The same info is available from /proc/pid/stat anyway. Is it really needed?

Gathering this data in userspace from /proc might be difficult esp. for short-lived tasks.

Also, /proc may not be mounted ? I'd heard somewhere that some sysadmins don't install /proc for security reasons. Don't know how far thats true.

Several other fields, totalling 58 bytes, added by the CSA patches are also duplicated in /proc/pid/stat. But all of them could change in value during the lifetime of a task so I'm guessing its not useful to get them from /proc
even if some kind of userspace polling of the value was possible.

But if there is a way, it would sure save a lot of payload sent over taskstats !

"duplicate" fields from CSA:
+ __u8 ac_nice; /* task_nice */
+ char ac_comm[TS_COMM_LEN]; /* Command name */
+ __u8 ac_sched; /* Scheduling discipline */
+ __u32 ac_pid; /* Process ID */
+ __u32 ac_ppid; /* Parent process ID */
+ __u64 ac_utime; /* User CPU time [usec] */
+ __u64 ac_stime; /* SYstem CPU time [usec] */
+ __u64 ac_minflt; /* Minor Page Fault */
+ __u64 ac_majflt; /* Major Page Fault */

#3 lore raw
Quoted 9 lines · Shailabh Nagar
Quoted 4 lines · Andrew Morton

Or we remove this field altogether, perhaps. The same info is available from /proc/pid/stat anyway. Is it really needed?

Gathering this data in userspace from /proc might be difficult esp. for short-lived tasks.

This is a serious concern. I think increasing TS_COMM_LEN to 32 would be a good solution.

Quoted 11 lines

Also, /proc may not be mounted ? I'd heard somewhere that some sysadmins don't install /proc for security reasons. Don't know how far thats true.

Several other fields, totalling 58 bytes, added by the CSA patches are also duplicated in /proc/pid/stat. But all of them could change in value during the lifetime of a task so I'm guessing its not useful to get them from /proc
even if some kind of userspace polling of the value was possible.

The same concern above applies to here, doesn't it?

Regards,
- jay

Quoted 14 lines

But if there is a way, it would sure save a lot of payload sent over taskstats !

"duplicate" fields from CSA:

include/linux/taskstats.h +9 -0
+    __u8    ac_nice;        /* task_nice */
+    char    ac_comm[TS_COMM_LEN];    /* Command name */
+    __u8    ac_sched;        /* Scheduling discipline */
+    __u32    ac_pid;            /* Process ID */
+    __u32    ac_ppid;        /* Parent process ID */
+    __u64    ac_utime;        /* User CPU time [usec] */
+    __u64    ac_stime;        /* SYstem CPU time [usec] */
+    __u64    ac_minflt;        /* Minor Page Fault */
+    __u64    ac_majflt;        /* Major Page Fault */
#4 lore raw
Quoted 29 lines · Jay Lan
Quoted 9 lines · Shailabh Nagar
Quoted 4 lines · Andrew Morton

Or we remove this field altogether, perhaps. The same info is available from /proc/pid/stat anyway. Is it really needed?

Gathering this data in userspace from /proc might be difficult esp. for short-lived tasks.

This is a serious concern. I think increasing TS_COMM_LEN to 32 would be a good solution.

Quoted 11 lines

Also, /proc may not be mounted ? I'd heard somewhere that some sysadmins don't install /proc for security reasons. Don't know how far thats true.

Several other fields, totalling 58 bytes, added by the CSA patches are also duplicated in /proc/pid/stat. But all of them could change in value during the lifetime of a task so I'm guessing its not useful to get them from /proc
even if some kind of userspace polling of the value was possible.

Quoted 1 line

The same concern above applies to here, doesn't it?

Yes. For some of the fields, pid/ppid/nice/sched you may not really care about whether you get the value present sometime during the lifetime vs. value at the time task exited, but for others like utime/stime/minflt/majflt, getting the last value is likely to matter. Either way, since there's no guarantee that you can poll a short-lived task fast enough to get any value, exporting the value from the kernel at exit seems to be the only safe way.

--Shailabh

Quoted 21 lines

Regards,
- jay

Quoted 14 lines

But if there is a way, it would sure save a lot of payload sent over taskstats !

"duplicate" fields from CSA:

include/linux/taskstats.h +9 -0
+    __u8    ac_nice;        /* task_nice */
+    char    ac_comm[TS_COMM_LEN];    /* Command name */
+    __u8    ac_sched;        /* Scheduling discipline */
+    __u32    ac_pid;            /* Process ID */
+    __u32    ac_ppid;        /* Parent process ID */
+    __u64    ac_utime;        /* User CPU time [usec] */
+    __u64    ac_stime;        /* SYstem CPU time [usec] */
+    __u64    ac_minflt;        /* Minor Page Fault */
+    __u64    ac_majflt;        /* Major Page Fault */