[PATCH 1/8] taskstats: fix indentation of long argument lists

[PATCH 1/8] taskstats: fix indentation of long argument lists

[Guillaume Chazarain]

Align with the opening parenthesis.

Changelog since V1 (http://lkml.org/lkml/2007/9/21/527):
- renamed fill_threadgroup() and add_tsk() to respectively
fill_threadgroup_stats() and add_tsk_stats() as suggested by Balbir Singh.
- added braces around do/while.
- added patch to unbreak binary compatibility between taskstats v5/v6.
- split further by preparing the bacct/xacct before the main patch.
- some indentation fixes.

Signed-off-by: Guillaume Chazarain <guichaz@yahoo.fr>
Cc: Balbir Singh <balbir@in.ibm.com>
Cc: Jay Lan <jlan@engr.sgi.com>
Cc: Jonathan Lim <jlim@sgi.com>
Cc: Oleg Nesterov <oleg@tv-sign.ru>
---

 kernel/taskstats.c |    9 ++++-----
 1 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/kernel/taskstats.c b/kernel/taskstats.c
index 059431e..a42b6c0 100644
--- a/kernel/taskstats.c
+++ b/kernel/taskstats.c
@@ -69,7 +69,7 @@ enum actions {
 };
 
 static int prepare_reply(struct genl_info *info, u8 cmd, struct sk_buff **skbp,
-				size_t size)
+			 size_t size)
 {
 	struct sk_buff *skb;
 	void *reply;
@@ -119,7 +119,7 @@ static int send_reply(struct sk_buff *skb, pid_t pid)
  * Send taskstats data in @skb to listeners registered for @cpu's exit data
  */
 static void send_cpu_listeners(struct sk_buff *skb,
-					struct listener_list *listeners)
+			       struct listener_list *listeners)
 {
 	struct genlmsghdr *genlhdr = nlmsg_data(nlmsg_hdr(skb));
 	struct listener *s, *tmp;
@@ -168,8 +168,7 @@ static void send_cpu_listeners(struct sk_buff *skb,
 	up_write(&listeners->sem);
 }
 
-static int fill_pid(pid_t pid, struct task_struct *tsk,
-		struct taskstats *stats)
+static int fill_pid(pid_t pid, struct task_struct *tsk, struct taskstats *stats)
 {
 	int rc = 0;
 
@@ -210,7 +209,7 @@ static int fill_pid(pid_t pid, struct task_struct *tsk,
 }
 
 static int fill_tgid(pid_t tgid, struct task_struct *first,
-		struct taskstats *stats)
+		     struct taskstats *stats)
 {
 	struct task_struct *tsk;
 	unsigned long flags;

[PATCH 2/8] taskstats: split the basic accounting fields

[Guillaume Chazarain]

Split the basic accounting taskstats fields into the threadgroup specific ones
and the thread specific ones. This should have no effect on the execution.

Signed-off-by: Guillaume Chazarain <guichaz@yahoo.fr>
Cc: Balbir Singh <balbir@in.ibm.com>
Cc: Jay Lan <jlan@engr.sgi.com>
Cc: Jonathan Lim <jlim@sgi.com>
Cc: Oleg Nesterov <oleg@tv-sign.ru>
---

 kernel/tsacct.c |   53 +++++++++++++++++++++++++++++++++++++----------------
 1 files changed, 37 insertions(+), 16 deletions(-)

diff --git a/kernel/tsacct.c b/kernel/tsacct.c
index 4ab1b58..d32378f 100644
--- a/kernel/tsacct.c
+++ b/kernel/tsacct.c
@@ -25,13 +25,11 @@
 /*
  * fill in basic accounting fields
  */
-void bacct_add_tsk(struct taskstats *stats, struct task_struct *tsk)
+static void fill_wall_times(struct taskstats *stats, struct task_struct *tsk)
 {
 	struct timespec uptime, ts;
 	s64 ac_etime;
 
-	BUILD_BUG_ON(TS_COMM_LEN < TASK_COMM_LEN);
-
 	/* calculate task elapsed time in timespec */
 	do_posix_clock_monotonic_gettime(&uptime);
 	ts = timespec_sub(uptime, tsk->start_time);
@@ -40,17 +38,47 @@ void bacct_add_tsk(struct taskstats *stats, struct task_struct *tsk)
 	do_div(ac_etime, NSEC_PER_USEC);
 	stats->ac_etime = ac_etime;
 	stats->ac_btime = get_seconds() - ts.tv_sec;
-	if (thread_group_leader(tsk)) {
-		stats->ac_exitcode = tsk->exit_code;
-		if (tsk->flags & PF_FORKNOEXEC)
-			stats->ac_flag |= AFORK;
-	}
+}
+
+/*
+ * Later on, XXX_add_tsk() will need to be called after XXX_fill_threadgroup(),
+ * so put the functions in this order
+ */
+static void bacct_fill_threadgroup(struct taskstats *stats,
+				   struct task_struct *tsk);
+
+void bacct_add_tsk(struct taskstats *stats, struct task_struct *tsk)
+{
 	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_utime	+= cputime_to_msecs(tsk->utime) * USEC_PER_MSEC;
+	stats->ac_stime += cputime_to_msecs(tsk->stime) * USEC_PER_MSEC;
+	stats->ac_utimescaled	+=
+		cputime_to_msecs(tsk->utimescaled) * USEC_PER_MSEC;
+	stats->ac_stimescaled	+=
+		cputime_to_msecs(tsk->stimescaled) * USEC_PER_MSEC;
+	stats->ac_minflt	+= tsk->min_flt;
+	stats->ac_majflt	+= tsk->maj_flt;
+
+	bacct_fill_threadgroup(stats, tsk);
+}
+
+static void bacct_fill_threadgroup(struct taskstats *stats,
+				   struct task_struct *tsk)
+{
+	fill_wall_times(stats, tsk);
+
+	if (thread_group_leader(tsk)) {
+		stats->ac_exitcode = tsk->exit_code;
+		if (tsk->flags & PF_FORKNOEXEC)
+			stats->ac_flag |= AFORK;
+	}
+
 	stats->ac_nice	 = task_nice(tsk);
 	stats->ac_sched	 = tsk->policy;
 	stats->ac_uid	 = tsk->uid;
@@ -60,15 +88,8 @@ void bacct_add_tsk(struct taskstats *stats, struct task_struct *tsk)
 	stats->ac_ppid	 = pid_alive(tsk) ?
 				rcu_dereference(tsk->real_parent)->tgid : 0;
 	rcu_read_unlock();
-	stats->ac_utime	 = cputime_to_msecs(tsk->utime) * USEC_PER_MSEC;
-	stats->ac_stime	 = cputime_to_msecs(tsk->stime) * USEC_PER_MSEC;
-	stats->ac_utimescaled =
-		cputime_to_msecs(tsk->utimescaled) * USEC_PER_MSEC;
-	stats->ac_stimescaled =
-		cputime_to_msecs(tsk->stimescaled) * USEC_PER_MSEC;
-	stats->ac_minflt = tsk->min_flt;
-	stats->ac_majflt = tsk->maj_flt;
 
+	BUILD_BUG_ON(TS_COMM_LEN < TASK_COMM_LEN);
 	strncpy(stats->ac_comm, tsk->comm, sizeof(stats->ac_comm));
 }
 

[PATCH 3/8] taskstats: split the extended accounting fields

[Guillaume Chazarain]

Split the extended accounting taskstats fields into the threadgroup specific
ones and the thread specific ones. This should have no effect on the execution.

Signed-off-by: Guillaume Chazarain <guichaz@yahoo.fr>
Cc: Balbir Singh <balbir@in.ibm.com>
Cc: Jay Lan <jlan@engr.sgi.com>
Cc: Jonathan Lim <jlim@sgi.com>
Cc: Oleg Nesterov <oleg@tv-sign.ru>
---

 kernel/tsacct.c |   48 ++++++++++++++++++++++++++++--------------------
 1 files changed, 28 insertions(+), 20 deletions(-)

diff --git a/kernel/tsacct.c b/kernel/tsacct.c
index d32378f..e8c25d2 100644
--- a/kernel/tsacct.c
+++ b/kernel/tsacct.c
@@ -101,33 +101,41 @@ static void bacct_fill_threadgroup(struct taskstats *stats,
 /*
  * fill in extended accounting fields
  */
-void xacct_add_tsk(struct taskstats *stats, struct task_struct *p)
+
+static void xacct_fill_threadgroup(struct taskstats *stats,
+				   struct task_struct *tsk);
+
+void xacct_add_tsk(struct taskstats *stats, struct task_struct *tsk)
+{
+	/* convert pages-jiffies to Mbyte-usec */
+	stats->coremem += jiffies_to_usecs(tsk->acct_rss_mem1) * PAGE_SIZE / MB;
+	stats->virtmem += jiffies_to_usecs(tsk->acct_vm_mem1)  * PAGE_SIZE / MB;
+
+	stats->read_char	+= tsk->rchar;
+	stats->write_char	+= tsk->wchar;
+	stats->read_syscalls	+= tsk->syscr;
+	stats->write_syscalls	+= tsk->syscw;
+#ifdef CONFIG_TASK_IO_ACCOUNTING
+	stats->read_bytes	+= tsk->ioac.read_bytes;
+	stats->write_bytes	+= tsk->ioac.write_bytes;
+	stats->cancelled_write_bytes += tsk->ioac.cancelled_write_bytes;
+#endif
+
+	xacct_fill_threadgroup(stats, tsk);
+}
+
+static void xacct_fill_threadgroup(struct taskstats *stats,
+				   struct task_struct *tsk)
 {
 	struct mm_struct *mm;
 
-	/* convert pages-jiffies to Mbyte-usec */
-	stats->coremem = jiffies_to_usecs(p->acct_rss_mem1) * PAGE_SIZE / MB;
-	stats->virtmem = jiffies_to_usecs(p->acct_vm_mem1) * PAGE_SIZE / MB;
-	mm = get_task_mm(p);
+	mm = get_task_mm(tsk);
 	if (mm) {
 		/* adjust to KB unit */
-		stats->hiwater_rss   = mm->hiwater_rss * PAGE_SIZE / KB;
-		stats->hiwater_vm    = mm->hiwater_vm * PAGE_SIZE / KB;
+		stats->hiwater_rss = mm->hiwater_rss * PAGE_SIZE / KB;
+		stats->hiwater_vm  = mm->hiwater_vm  * PAGE_SIZE / KB;
 		mmput(mm);
 	}
-	stats->read_char	= p->rchar;
-	stats->write_char	= p->wchar;
-	stats->read_syscalls	= p->syscr;
-	stats->write_syscalls	= p->syscw;
-#ifdef CONFIG_TASK_IO_ACCOUNTING
-	stats->read_bytes	= p->ioac.read_bytes;
-	stats->write_bytes	= p->ioac.write_bytes;
-	stats->cancelled_write_bytes = p->ioac.cancelled_write_bytes;
-#else
-	stats->read_bytes	= 0;
-	stats->write_bytes	= 0;
-	stats->cancelled_write_bytes = 0;
-#endif
 }
 #undef KB
 #undef MB

[PATCH 4/8] taskstats: separate PID/TGID stats producers to complete the TGID ones

[Guillaume Chazarain]

TASKSTATS_CMD_ATTR_TGID used to return only the delay accounting stats, not
the basic and extended accounting.  With this patch,
TASKSTATS_CMD_ATTR_TGID also aggregates the accounting info for all threads
of a thread group.

TASKSTATS_CMD_ATTR_PID output should be unchanged
TASKSTATS_CMD_ATTR_TGID output should have all fields set, unlike before the
patch where most of the fiels were set to 0.

To this aim, two functions were introduced: fill_threadgroup_stats() and
add_tsk_stats(). These functions are responsible for aggregating the
subsystem specific accounting information. Taskstats requesters (fill_pid(),
fill_tgid() and fill_tgid_exit()) should only call add_tsk_stats() and
fill_threadgroup_stats() to get the stats.

Signed-off-by: Guillaume Chazarain <guichaz@yahoo.fr>
Cc: Balbir Singh <balbir@in.ibm.com>
Cc: Jay Lan <jlan@engr.sgi.com>
Cc: Jonathan Lim <jlim@sgi.com>
Cc: Oleg Nesterov <oleg@tv-sign.ru>
---

 Documentation/accounting/getdelays.c |    2 -
 include/linux/tsacct_kern.h          |   12 +++-
 kernel/taskstats.c                   |  107 +++++++++++++++++++++++-----------
 kernel/tsacct.c                      |   20 +-----
 4 files changed, 85 insertions(+), 56 deletions(-)

diff --git a/Documentation/accounting/getdelays.c b/Documentation/accounting/getdelays.c
index cbee3a2..78773c0 100644
--- a/Documentation/accounting/getdelays.c
+++ b/Documentation/accounting/getdelays.c
@@ -76,7 +76,7 @@ static void usage(void)
 	fprintf(stderr, "getdelays [-dilv] [-w logfile] [-r bufsize] "
 			"[-m cpumask] [-t tgid] [-p pid]\n");
 	fprintf(stderr, "  -d: print delayacct stats\n");
-	fprintf(stderr, "  -i: print IO accounting (works only with -p)\n");
+	fprintf(stderr, "  -i: print IO accounting\n");
 	fprintf(stderr, "  -l: listen forever\n");
 	fprintf(stderr, "  -v: debug on\n");
 }
diff --git a/include/linux/tsacct_kern.h b/include/linux/tsacct_kern.h
index 7e50ac7..1568a3d 100644
--- a/include/linux/tsacct_kern.h
+++ b/include/linux/tsacct_kern.h
@@ -10,19 +10,25 @@
 #include <linux/taskstats.h>
 
 #ifdef CONFIG_TASKSTATS
-extern void bacct_add_tsk(struct taskstats *stats, struct task_struct *tsk);
+void bacct_add_tsk(struct taskstats *stats, struct task_struct *task);
+void bacct_fill_threadgroup(struct taskstats *stats, struct task_struct *task);
 #else
-static inline void bacct_add_tsk(struct taskstats *stats, struct task_struct *tsk)
+static inline void bacct_add_tsk(struct taskstats *stats, struct task_struct *task)
+{}
+static inline void bacct_fill_threadgroup(struct taskstats *stats, struct task_struct *task)
 {}
 #endif /* CONFIG_TASKSTATS */
 
 #ifdef CONFIG_TASK_XACCT
-extern void xacct_add_tsk(struct taskstats *stats, struct task_struct *p);
+void xacct_add_tsk(struct taskstats *stats, struct task_struct *p);
+void xacct_fill_threadgroup(struct taskstats *stats, struct task_struct *task);
 extern void acct_update_integrals(struct task_struct *tsk);
 extern void acct_clear_integrals(struct task_struct *tsk);
 #else
 static inline void xacct_add_tsk(struct taskstats *stats, struct task_struct *p)
 {}
+static inline void xacct_fill_threadgroup(struct taskstats *stats, struct task_struct *task)
+{}
 static inline void acct_update_integrals(struct task_struct *tsk)
 {}
 static inline void acct_clear_integrals(struct task_struct *tsk)
diff --git a/kernel/taskstats.c b/kernel/taskstats.c
index a42b6c0..972bbfa 100644
--- a/kernel/taskstats.c
+++ b/kernel/taskstats.c
@@ -168,6 +168,63 @@ static void send_cpu_listeners(struct sk_buff *skb,
 	up_write(&listeners->sem);
 }
 
+/**
+ * add_tsk_stats - combine some thread specific stats in a taskstats
+ * @stats: the taskstats to write into
+ * @task: the thread to combine
+ *
+ * There are two types of taskstats fields when considering a thread group:
+ *	- those that can be aggregated from each thread in the group (like CPU
+ *	times),
+ *	- those that cannot be aggregated (like UID) or are identical (like
+ *	memory usage), so are taken from the group leader.
+ * XXX_add_tsk() methods deal with the first type while XXX_fill_threadgroup()
+ * with the second one.
+ */
+static void add_tsk_stats(struct taskstats *stats, struct task_struct *task)
+{
+	/*
+	 * Each accounting subsystem adds calls to its functions to combine
+	 * relevant parts of struct taskstsats for a single pid as follows:
+	 *
+	 *	per-task-foo-add_tsk_stats(stats, task);
+	 */
+
+	/* fill in delay acct fields */
+	delayacct_add_tsk(stats, task);
+
+	/* fill in basic acct fields */
+	bacct_add_tsk(stats, task);
+
+	/* fill in extended acct fields */
+	xacct_add_tsk(stats, task);
+}
+
+/**
+ * fill_threadgroup_stats - initialize some common stats for the thread group
+ * @stats: the taskstats to write into
+ * @task: the thread representing the whole group
+ *
+ * Will be called on the thread group leader if requesting stats for the whole
+ * thread group.
+ */
+static void fill_threadgroup_stats(struct taskstats *stats,
+				   struct task_struct *task)
+{
+	/*
+	 * Each accounting subsystem adds calls to its functions to initialize
+	 * relevant parts of struct taskstsats for a single tgid as follows:
+	 *
+	 *	per-task-foo-fill_threadgroup(stats, task);
+	 */
+
+	/* fill in basic acct fields */
+	bacct_fill_threadgroup(stats, task);
+
+	/* fill in extended acct fields */
+	xacct_fill_threadgroup(stats, task);
+}
+
 static int fill_pid(pid_t pid, struct task_struct *tsk, struct taskstats *stats)
 {
 	int rc = 0;
@@ -184,23 +241,11 @@ static int fill_pid(pid_t pid, struct task_struct *tsk, struct taskstats *stats)
 		get_task_struct(tsk);
 
 	memset(stats, 0, sizeof(*stats));
-	/*
-	 * Each accounting subsystem adds calls to its functions to
-	 * fill in relevant parts of struct taskstsats as follows
-	 *
-	 *	per-task-foo(stats, tsk);
-	 */
-
-	delayacct_add_tsk(stats, tsk);
-
-	/* fill in basic acct fields */
 	stats->version = TASKSTATS_VERSION;
 	stats->nvcsw = tsk->nvcsw;
 	stats->nivcsw = tsk->nivcsw;
-	bacct_add_tsk(stats, tsk);
-
-	/* fill in extended acct fields */
-	xacct_add_tsk(stats, tsk);
+	add_tsk_stats(stats, tsk);
+	fill_threadgroup_stats(stats, tsk);
 
 	/* Define err: label here if needed */
 	put_task_struct(tsk);
@@ -236,27 +281,20 @@ static int fill_tgid(pid_t tgid, struct task_struct *first,
 		if (tsk->exit_state)
 			continue;
 		/*
-		 * Accounting subsystem can call its functions here to
-		 * fill in relevant parts of struct taskstsats as follows
-		 *
-		 *	per-task-foo(stats, tsk);
+		 * This check is racy as a thread could exit just right
+		 * now and have its statistics accounted twice.
 		 */
-		delayacct_add_tsk(stats, tsk);
-
+		add_tsk_stats(stats, tsk);
 		stats->nvcsw += tsk->nvcsw;
 		stats->nivcsw += tsk->nivcsw;
 	} while_each_thread(first, tsk);
 
+	fill_threadgroup_stats(stats, first->group_leader);
 	unlock_task_sighand(first, &flags);
 	rc = 0;
 out:
 	rcu_read_unlock();
-
 	stats->version = TASKSTATS_VERSION;
-	/*
-	 * Accounting subsytems can also add calls here to modify
-	 * fields of taskstats.
-	 */
 	return rc;
 }
 
@@ -266,19 +304,14 @@ static void fill_tgid_exit(struct task_struct *tsk)
 	unsigned long flags;
 
 	spin_lock_irqsave(&tsk->sighand->siglock, flags);
-	if (!tsk->signal->stats)
-		goto ret;
 
 	/*
-	 * Each accounting subsystem calls its functions here to
-	 * accumalate its per-task stats for tsk, into the per-tgid structure
-	 *
-	 *	per-task-foo(tsk->signal->stats, tsk);
+	 * The fill_threadgroup_stats() part of the statistics will be added by
+	 * the stats requester, i.e. fill_tgid() or taskstats_exit()
 	 */
-	delayacct_add_tsk(tsk->signal->stats, tsk);
-ret:
+	add_tsk_stats(tsk->signal->stats, tsk);
+
 	spin_unlock_irqrestore(&tsk->sighand->siglock, flags);
-	return;
 }
 
 static int add_del_listener(pid_t pid, cpumask_t *maskp, int isadd)
@@ -507,7 +540,13 @@ void taskstats_exit(struct task_struct *tsk, int group_dead)
 	if (!stats)
 		goto err;
 
+	/*
+	 * Race here: the last thread may have not finished its
+	 * fill_tgid_exit(), leaving an incomplete tsk->signal->stats
+	 */
+
 	memcpy(stats, tsk->signal->stats, sizeof(*stats));
+	fill_threadgroup_stats(stats, tsk->group_leader);
 
 send:
 	send_cpu_listeners(rep_skb, listeners);
diff --git a/kernel/tsacct.c b/kernel/tsacct.c
index e8c25d2..4caea73 100644
--- a/kernel/tsacct.c
+++ b/kernel/tsacct.c
@@ -40,13 +40,6 @@ static void fill_wall_times(struct taskstats *stats, struct task_struct *tsk)
 	stats->ac_btime = get_seconds() - ts.tv_sec;
 }
 
-/*
- * Later on, XXX_add_tsk() will need to be called after XXX_fill_threadgroup(),
- * so put the functions in this order
- */
-static void bacct_fill_threadgroup(struct taskstats *stats,
-				   struct task_struct *tsk);
-
 void bacct_add_tsk(struct taskstats *stats, struct task_struct *tsk)
 {
 	if (tsk->flags & PF_SUPERPRIV)
@@ -64,12 +57,9 @@ void bacct_add_tsk(struct taskstats *stats, struct task_struct *tsk)
 		cputime_to_msecs(tsk->stimescaled) * USEC_PER_MSEC;
 	stats->ac_minflt	+= tsk->min_flt;
 	stats->ac_majflt	+= tsk->maj_flt;
-
-	bacct_fill_threadgroup(stats, tsk);
 }
 
-static void bacct_fill_threadgroup(struct taskstats *stats,
-				   struct task_struct *tsk)
+void bacct_fill_threadgroup(struct taskstats *stats, struct task_struct *tsk)
 {
 	fill_wall_times(stats, tsk);
 
@@ -102,9 +92,6 @@ static void bacct_fill_threadgroup(struct taskstats *stats,
  * fill in extended accounting fields
  */
 
-static void xacct_fill_threadgroup(struct taskstats *stats,
-				   struct task_struct *tsk);
-
 void xacct_add_tsk(struct taskstats *stats, struct task_struct *tsk)
 {
 	/* convert pages-jiffies to Mbyte-usec */
@@ -120,12 +107,9 @@ void xacct_add_tsk(struct taskstats *stats, struct task_struct *tsk)
 	stats->write_bytes	+= tsk->ioac.write_bytes;
 	stats->cancelled_write_bytes += tsk->ioac.cancelled_write_bytes;
 #endif
-
-	xacct_fill_threadgroup(stats, tsk);
 }
 
-static void xacct_fill_threadgroup(struct taskstats *stats,
-				   struct task_struct *tsk)
+void xacct_fill_threadgroup(struct taskstats *stats, struct task_struct *tsk)
 {
 	struct mm_struct *mm;
 

[PATCH 5/8] taskstats: factor out version and context switch accounting

[Guillaume Chazarain]

All the specific taskstats fields should only be manipulated in
{add_tsk,fill_threadgroup}_stats().

Signed-off-by: Guillaume Chazarain <guichaz@yahoo.fr>
Cc: Balbir Singh <balbir@in.ibm.com>
Cc: Jay Lan <jlan@engr.sgi.com>
Cc: Jonathan Lim <jlim@sgi.com>
Cc: Oleg Nesterov <oleg@tv-sign.ru>
---

 kernel/taskstats.c |   21 ++++++++-------------
 kernel/tsacct.c    |    3 +++
 2 files changed, 11 insertions(+), 13 deletions(-)

diff --git a/kernel/taskstats.c b/kernel/taskstats.c
index 972bbfa..f25bf03 100644
--- a/kernel/taskstats.c
+++ b/kernel/taskstats.c
@@ -218,6 +218,8 @@ static void fill_threadgroup_stats(struct taskstats *stats,
 	 *	per-task-foo-fill_threadgroup(stats, task);
 	 */
 
+	stats->version = TASKSTATS_VERSION;
+
 	/* fill in basic acct fields */
 	bacct_fill_threadgroup(stats, task);
 
@@ -241,9 +243,6 @@ static int fill_pid(pid_t pid, struct task_struct *tsk, struct taskstats *stats)
 		get_task_struct(tsk);
 
 	memset(stats, 0, sizeof(*stats));
-	stats->version = TASKSTATS_VERSION;
-	stats->nvcsw = tsk->nvcsw;
-	stats->nivcsw = tsk->nivcsw;
 	add_tsk_stats(stats, tsk);
 	fill_threadgroup_stats(stats, tsk);
 
@@ -278,15 +277,12 @@ static int fill_tgid(pid_t tgid, struct task_struct *first,
 
 	tsk = first;
 	do {
-		if (tsk->exit_state)
-			continue;
-		/*
-		 * This check is racy as a thread could exit just right
-		 * now and have its statistics accounted twice.
-		 */
-		add_tsk_stats(stats, tsk);
-		stats->nvcsw += tsk->nvcsw;
-		stats->nivcsw += tsk->nivcsw;
+		if (!tsk->exit_state)
+			/*
+			 * This check is racy as a thread could exit just right
+			 * now and have its statistics accounted twice.
+			 */
+			add_tsk_stats(stats, tsk);
 	} while_each_thread(first, tsk);
 
 	fill_threadgroup_stats(stats, first->group_leader);
@@ -294,7 +290,6 @@ static int fill_tgid(pid_t tgid, struct task_struct *first,
 	rc = 0;
 out:
 	rcu_read_unlock();
-	stats->version = TASKSTATS_VERSION;
 	return rc;
 }
 
diff --git a/kernel/tsacct.c b/kernel/tsacct.c
index 4caea73..73b4c69 100644
--- a/kernel/tsacct.c
+++ b/kernel/tsacct.c
@@ -57,6 +57,9 @@ void bacct_add_tsk(struct taskstats *stats, struct task_struct *tsk)
 		cputime_to_msecs(tsk->stimescaled) * USEC_PER_MSEC;
 	stats->ac_minflt	+= tsk->min_flt;
 	stats->ac_majflt	+= tsk->maj_flt;
+
+	stats->nvcsw		+= tsk->nvcsw;
+	stats->nivcsw		+= tsk->nivcsw;
 }
 
 void bacct_fill_threadgroup(struct taskstats *stats, struct task_struct *tsk)

[PATCH 6/8] taskstats: tell fill_threadgroup_stats() if it replies with PID or TGID stats

[Guillaume Chazarain]

fill_threadgroup_stats() may want to know if it is filling
TASKSTATS_CMD_ATTR_TGID or TASKSTATS_CMD_ATTR_PID stats, so give it this
information in the tg_stats boolean.

Signed-off-by: Guillaume Chazarain <guichaz@yahoo.fr>
Cc: Balbir Singh <balbir@in.ibm.com>
Cc: Jay Lan <jlan@engr.sgi.com>
Cc: Jonathan Lim <jlim@sgi.com>
Cc: Oleg Nesterov <oleg@tv-sign.ru>
---

 include/linux/tsacct_kern.h |    4 ++--
 kernel/taskstats.c          |   11 ++++++-----
 kernel/tsacct.c             |    3 ++-
 3 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/include/linux/tsacct_kern.h b/include/linux/tsacct_kern.h
index 1568a3d..e8d1ad6 100644
--- a/include/linux/tsacct_kern.h
+++ b/include/linux/tsacct_kern.h
@@ -11,11 +11,11 @@
 
 #ifdef CONFIG_TASKSTATS
 void bacct_add_tsk(struct taskstats *stats, struct task_struct *task);
-void bacct_fill_threadgroup(struct taskstats *stats, struct task_struct *task);
+void bacct_fill_threadgroup(struct taskstats *stats, struct task_struct *task, bool tg_stats);
 #else
 static inline void bacct_add_tsk(struct taskstats *stats, struct task_struct *task)
 {}
-static inline void bacct_fill_threadgroup(struct taskstats *stats, struct task_struct *task)
+static inline void bacct_fill_threadgroup(struct taskstats *stats, struct task_struct *task, bool tg_stats)
 {}
 #endif /* CONFIG_TASKSTATS */
 
diff --git a/kernel/taskstats.c b/kernel/taskstats.c
index f25bf03..59fe1d8 100644
--- a/kernel/taskstats.c
+++ b/kernel/taskstats.c
@@ -204,12 +204,13 @@ static void add_tsk_stats(struct taskstats *stats, struct task_struct *task)
  * fill_threadgroup_stats - initialize some common stats for the thread group
  * @stats: the taskstats to write into
  * @task: the thread representing the whole group
+ * @tg_stats: whether in the end thread group stats are requested
  *
  * Will be called on the thread group leader if requesting stats for the whole
  * thread group.
  */
 static void fill_threadgroup_stats(struct taskstats *stats,
-				   struct task_struct *task)
+				   struct task_struct *task, bool tg_stats)
 {
 	/*
 	 * Each accounting subsystem adds calls to its functions to initialize
@@ -221,7 +222,7 @@ static void fill_threadgroup_stats(struct taskstats *stats,
 	stats->version = TASKSTATS_VERSION;
 
 	/* fill in basic acct fields */
-	bacct_fill_threadgroup(stats, task);
+	bacct_fill_threadgroup(stats, task, tg_stats);
 
 	/* fill in extended acct fields */
 	xacct_fill_threadgroup(stats, task);
@@ -244,7 +245,7 @@ static int fill_pid(pid_t pid, struct task_struct *tsk, struct taskstats *stats)
 
 	memset(stats, 0, sizeof(*stats));
 	add_tsk_stats(stats, tsk);
-	fill_threadgroup_stats(stats, tsk);
+	fill_threadgroup_stats(stats, tsk, false);
 
 	/* Define err: label here if needed */
 	put_task_struct(tsk);
@@ -285,7 +286,7 @@ static int fill_tgid(pid_t tgid, struct task_struct *first,
 			add_tsk_stats(stats, tsk);
 	} while_each_thread(first, tsk);
 
-	fill_threadgroup_stats(stats, first->group_leader);
+	fill_threadgroup_stats(stats, first->group_leader, true);
 	unlock_task_sighand(first, &flags);
 	rc = 0;
 out:
@@ -541,7 +542,7 @@ void taskstats_exit(struct task_struct *tsk, int group_dead)
 	 */
 
 	memcpy(stats, tsk->signal->stats, sizeof(*stats));
-	fill_threadgroup_stats(stats, tsk->group_leader);
+	fill_threadgroup_stats(stats, tsk->group_leader, true);
 
 send:
 	send_cpu_listeners(rep_skb, listeners);
diff --git a/kernel/tsacct.c b/kernel/tsacct.c
index 73b4c69..8c2f470 100644
--- a/kernel/tsacct.c
+++ b/kernel/tsacct.c
@@ -62,7 +62,8 @@ void bacct_add_tsk(struct taskstats *stats, struct task_struct *tsk)
 	stats->nivcsw		+= tsk->nivcsw;
 }
 
-void bacct_fill_threadgroup(struct taskstats *stats, struct task_struct *tsk)
+void bacct_fill_threadgroup(struct taskstats *stats, struct task_struct *tsk,
+			    bool tg_stats)
 {
 	fill_wall_times(stats, tsk);
 

[PATCH 7/8] taskstats: fix stats->ac_exitcode to work on threads and use group_exit_code

[Guillaume Chazarain]

Threads also have an exit code on their own, so report it in
TASKSTATS_CMD_ATTR_PID.

For TASKSTATS_CMD_ATTR_TGID, instead of relying only on the exit code of the
leader, we use task->signal->group_exit_code if not null as suggested by
Oleg Nesterov.

Also, document that as of this patch, fill_threadgroup_stats() must be called
after add_tsk_stats() as it may overwrite some stats.

Signed-off-by: Guillaume Chazarain <guichaz@yahoo.fr>
Cc: Balbir Singh <balbir@in.ibm.com>
Cc: Jay Lan <jlan@engr.sgi.com>
Cc: Jonathan Lim <jlim@sgi.com>
Cc: Oleg Nesterov <oleg@tv-sign.ru>
---

 kernel/taskstats.c |    2 ++
 kernel/tsacct.c    |   12 +++++++-----
 2 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/kernel/taskstats.c b/kernel/taskstats.c
index 59fe1d8..938c0b0 100644
--- a/kernel/taskstats.c
+++ b/kernel/taskstats.c
@@ -208,6 +208,8 @@ static void add_tsk_stats(struct taskstats *stats, struct task_struct *task)
  *
  * Will be called on the thread group leader if requesting stats for the whole
  * thread group.
+ * fill_threadgroup_stats() may overwrite stats from add_tsk_stats(), so it
+ * must be called after add_tsk_stats().
  */
 static void fill_threadgroup_stats(struct taskstats *stats,
 				   struct task_struct *task, bool tg_stats)
diff --git a/kernel/tsacct.c b/kernel/tsacct.c
index 8c2f470..e83f53b 100644
--- a/kernel/tsacct.c
+++ b/kernel/tsacct.c
@@ -65,13 +65,15 @@ void bacct_add_tsk(struct taskstats *stats, struct task_struct *tsk)
 void bacct_fill_threadgroup(struct taskstats *stats, struct task_struct *tsk,
 			    bool tg_stats)
 {
+	int group_exit_code;
+
 	fill_wall_times(stats, tsk);
 
-	if (thread_group_leader(tsk)) {
-		stats->ac_exitcode = tsk->exit_code;
-		if (tsk->flags & PF_FORKNOEXEC)
-			stats->ac_flag |= AFORK;
-	}
+	if (thread_group_leader(tsk) && ((tsk->flags & PF_FORKNOEXEC)))
+		stats->ac_flag |= AFORK;
+
+	group_exit_code = tg_stats ? tsk->signal->group_exit_code : 0;
+	stats->ac_exitcode = group_exit_code ? : tsk->exit_code;
 
 	stats->ac_nice	 = task_nice(tsk);
 	stats->ac_sched	 = tsk->policy;

[PATCH 8/8] taskstats: avoid breaking binary compatibility between taskstats v6 and before

[Guillaume Chazarain]

Place fields added in v6 at the end of the struct.

Signed-off-by: Guillaume Chazarain <guichaz@yahoo.fr>
Cc: Michael Neuling <mikey@neuling.org>
Cc: Balbir Singh <balbir@in.ibm.com>
Cc: Jay Lan <jlan@engr.sgi.com>
Cc: Jonathan Lim <jlim@sgi.com>
Cc: Oleg Nesterov <oleg@tv-sign.ru>
---

 include/linux/taskstats.h |   18 +++++++++++-------
 1 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/include/linux/taskstats.h b/include/linux/taskstats.h
index 92bfd1c..dc6850c 100644
--- a/include/linux/taskstats.h
+++ b/include/linux/taskstats.h
@@ -85,12 +85,9 @@ struct taskstats {
 	 * On some architectures, value will adjust for cpu time stolen
 	 * from the kernel in involuntary waits due to virtualization.
 	 * Value is cumulative, in nanoseconds, without a corresponding count
-	 * and wraps around to zero silently on overflow.  The
-	 * _scaled_ version accounts for cpus which can scale the
-	 * number of instructions executed each cycle.
+	 * and wraps around to zero silently on overflow.
 	 */
 	__u64	cpu_run_real_total;
-	__u64	cpu_scaled_run_real_total;
 
 	/* cpu "virtual" running time
 	 * Uses time intervals seen by the kernel i.e. no adjustment
@@ -146,9 +143,6 @@ struct taskstats {
 	__u64	read_syscalls;		/* read syscalls */
 	__u64	write_syscalls;		/* write syscalls */
 
-	/* time accounting for SMT machines */
-	__u64	ac_utimescaled;		/* utime scaled on frequency etc */
-	__u64	ac_stimescaled;		/* stime scaled on frequency etc */
 	/* Extended accounting fields end */
 
 #define TASKSTATS_HAS_IO_ACCOUNTING
@@ -159,6 +153,16 @@ struct taskstats {
 
 	__u64  nvcsw;			/* voluntary_ctxt_switches */
 	__u64  nivcsw;			/* nonvoluntary_ctxt_switches */
+
+	/*
+	 * Same as cpu_run_real_total but accounts for cpus which can scale the
+	 * number of instructions executed each cycle.
+	 */
+	__u64	cpu_scaled_run_real_total;
+
+	/* time accounting for SMT machines */
+	__u64	ac_utimescaled;		/* utime scaled on frequency etc */
+	__u64	ac_stimescaled;		/* stime scaled on frequency etc */
 };
 
 

[PATCH 7/8] taskstats: fix stats->ac_exitcode to work on threads and use group_exit_code

[Guillaume Chazarain]

Threads also have an exit code on their own, so report it in
TASKSTATS_CMD_ATTR_PID.

For TASKSTATS_CMD_ATTR_TGID, instead of relying only on the exit code of the
leader, we use task->signal->group_exit_code if not null as suggested by
Oleg Nesterov.

Also, document that as of this patch, fill_threadgroup_stats() must be called
after add_tsk_stats() as it may overwrite some stats.

Signed-off-by: Guillaume Chazarain <guichaz@yahoo.fr>
Cc: Balbir Singh <balbir@in.ibm.com>
Cc: Jay Lan <jlan@engr.sgi.com>
Cc: Jonathan Lim <jlim@sgi.com>
Cc: Oleg Nesterov <oleg@tv-sign.ru>
---

 kernel/taskstats.c |    2 ++
 kernel/tsacct.c    |   12 +++++++-----
 2 files changed, 9 insertions(+), 5 deletions(-)


diff --git a/kernel/taskstats.c b/kernel/taskstats.c
index 59fe1d8..938c0b0 100644
--- a/kernel/taskstats.c
+++ b/kernel/taskstats.c
@@ -208,6 +208,8 @@ static void add_tsk_stats(struct taskstats *stats, struct task_struct *task)
  *
  * Will be called on the thread group leader if requesting stats for the whole
  * thread group.
+ * fill_threadgroup_stats() may overwrite stats from add_tsk_stats(), so it
+ * must be called after add_tsk_stats().
  */
 static void fill_threadgroup_stats(struct taskstats *stats,
 				   struct task_struct *task, bool tg_stats)
diff --git a/kernel/tsacct.c b/kernel/tsacct.c
index 8c2f470..e83f53b 100644
--- a/kernel/tsacct.c
+++ b/kernel/tsacct.c
@@ -65,13 +65,15 @@ void bacct_add_tsk(struct taskstats *stats, struct task_struct *tsk)
 void bacct_fill_threadgroup(struct taskstats *stats, struct task_struct *tsk,
 			    bool tg_stats)
 {
+	int group_exit_code;
+
 	fill_wall_times(stats, tsk);
 
-	if (thread_group_leader(tsk)) {
-		stats->ac_exitcode = tsk->exit_code;
-		if (tsk->flags & PF_FORKNOEXEC)
-			stats->ac_flag |= AFORK;
-	}
+	if (thread_group_leader(tsk) && ((tsk->flags & PF_FORKNOEXEC)))
+		stats->ac_flag |= AFORK;
+
+	group_exit_code = tg_stats ? tsk->signal->group_exit_code : 0;
+	stats->ac_exitcode = group_exit_code ? : tsk->exit_code;
 
 	stats->ac_nice	 = task_nice(tsk);
 	stats->ac_sched	 = tsk->policy;

Re: [PATCH 7/8] taskstats: fix stats->ac_exitcode to work on threads and use group_exit_code

[roel]

Guillaume Chazarain wrote:

[...]

> @@ -65,13 +65,15 @@ void bacct_add_tsk(struct taskstats *stats, struct task_struct *tsk)
>  void bacct_fill_threadgroup(struct taskstats *stats, struct task_struct *tsk,
>  			    bool tg_stats)
>  {
> +	int group_exit_code;
> +
>  	fill_wall_times(stats, tsk);
>  
> -	if (thread_group_leader(tsk)) {
> -		stats->ac_exitcode = tsk->exit_code;
> -		if (tsk->flags & PF_FORKNOEXEC)
> -			stats->ac_flag |= AFORK;
> -	}
> +	if (thread_group_leader(tsk) && ((tsk->flags & PF_FORKNOEXEC)))

	if (thread_group_leader(tsk) && (tsk->flags & PF_FORKNOEXEC))

> +		stats->ac_flag |= AFORK;
> +
> +	group_exit_code = tg_stats ? tsk->signal->group_exit_code : 0;
> +	stats->ac_exitcode = group_exit_code ? : tsk->exit_code;

Isn't this just confusing? why not

	if (tg_stats) {
		group_exit_code = tsk->signal->group_exit_code;
		stats->ac_exitcode = group_exit_code;
		
	} else {
		group_exit_code = 0;
		stats->ac_exitcode = tsk->exit_code;
	}

Re: [PATCH 7/8] taskstats: fix stats->ac_exitcode to work on threads and use group_exit_code

[Guillaume Chazarain]

Le Wed, 26 Sep 2007 22:47:54 +0200,
roel <12o3l@tiscali.nl> a écrit :

> > +	if (thread_group_leader(tsk) && ((tsk->flags & PF_FORKNOEXEC)))
> 
> 	if (thread_group_leader(tsk) && (tsk->flags & PF_FORKNOEXEC))

Yeah, right, good catch.

> > +	group_exit_code = tg_stats ? tsk->signal->group_exit_code : 0;
> > +	stats->ac_exitcode = group_exit_code ? : tsk->exit_code;
> 
> Isn't this just confusing? why not
> 
> 	if (tg_stats) {
> 		group_exit_code = tsk->signal->group_exit_code;
> 		stats->ac_exitcode = group_exit_code;

Because in this case if group_exit_code is null, we want
tsk->exit_code, not 0.

> 		
> 	} else {
> 		group_exit_code = 0;
> 		stats->ac_exitcode = tsk->exit_code;
> 	}

Andrew is not interested at the moment in this series (that replaces
all my previous patches on taskstats, for info), but thank you for the
review.


-- 
Guillaume