[Patch 0/3] delay accounting fixes

[Patch 0/3] delay accounting fixes

[Shailabh Nagar]

Andrew,

Following are some fixes to the delay accounting & 
taskstats patches, against 2.6.18-rc2.

The most significant change is the third patch in the
series which turns on delay accounting by default. This 
is a temporary measure to ensure that the features get
coverage testing in 2.6.18-rc kernels without requiring
kernel options to be turned on. Late in the -rc
series, it can be reversed (will send a patch to do that)

--Shailabh

[Patch 1/3] make taskstats sending completely independent of delay accounting on/off status

[Shailabh Nagar]

Complete the separation of delay accounting and taskstats by ignoring the
return value of delay accounting functions that fill in parts of taskstats
before it is sent out (either in response to a command or as part of a task
exit).

Also make delayacct_add_tsk return silently when delay accounting is turned
off rather than treat it as an error.

Signed-Off-By: Shailabh Nagar <nagar@watson.ibm.com>

 include/linux/delayacct.h |    4 +---
 kernel/taskstats.c        |    8 +++-----
 2 files changed, 4 insertions(+), 8 deletions(-)

Index: linux-2.6.18-rc2/kernel/taskstats.c
===================================================================
--- linux-2.6.18-rc2.orig/kernel/taskstats.c	2006-07-17 17:01:51.000000000 -0400
+++ linux-2.6.18-rc2/kernel/taskstats.c	2006-07-17 17:03:41.000000000 -0400
@@ -177,7 +177,7 @@ static int send_cpu_listeners(struct sk_
 static int fill_pid(pid_t pid, struct task_struct *pidtsk,
 		struct taskstats *stats)
 {
-	int rc;
+	int rc = 0;
 	struct task_struct *tsk = pidtsk;
 
 	if (!pidtsk) {
@@ -196,12 +196,10 @@ static int fill_pid(pid_t pid, struct ta
 	 * Each accounting subsystem adds calls to its functions to
 	 * fill in relevant parts of struct taskstsats as follows
 	 *
-	 *	rc = per-task-foo(stats, tsk);
-	 *	if (rc)
-	 *		goto err;
+	 *	per-task-foo(stats, tsk);
 	 */
 
-	rc = delayacct_add_tsk(stats, tsk);
+	delayacct_add_tsk(stats, tsk);
 	stats->version = TASKSTATS_VERSION;
 
 	/* Define err: label here if needed */
Index: linux-2.6.18-rc2/include/linux/delayacct.h
===================================================================
--- linux-2.6.18-rc2.orig/include/linux/delayacct.h	2006-07-17 17:01:51.000000000 -0400
+++ linux-2.6.18-rc2/include/linux/delayacct.h	2006-07-17 17:03:41.000000000 -0400
@@ -80,9 +80,7 @@ static inline void delayacct_blkio_end(v
 static inline int delayacct_add_tsk(struct taskstats *d,
 					struct task_struct *tsk)
 {
-	if (likely(!delayacct_on))
-		return -EINVAL;
-	if (!tsk->delays)
+	if (likely(!delayacct_on) || !tsk->delays)
 		return 0;
 	return __delayacct_add_tsk(d, tsk);
 }

[Patch 2/3] taskstats: free skb, avoid returns in send_cpu_listeners

[Shailabh Nagar]

Add a missing freeing of skb in the case there are no listeners at all.
Also remove the returning of error values by the function as it is unused by
the sole caller.

Signed-Off-By: Shailabh Nagar <nagar@watson.ibm.com>
Signed-Off-By: Chandra Seetharaman <sekharan@us.ibm.com>

 kernel/taskstats.c |   24 +++++++++++-------------
 1 files changed, 11 insertions(+), 13 deletions(-)

Index: linux-2.6.18-rc2/kernel/taskstats.c
===================================================================
--- linux-2.6.18-rc2.orig/kernel/taskstats.c	2006-07-17 17:03:41.000000000 -0400
+++ linux-2.6.18-rc2/kernel/taskstats.c	2006-07-17 17:06:52.000000000 -0400
@@ -121,46 +121,45 @@ static int send_reply(struct sk_buff *sk
 /*
  * Send taskstats data in @skb to listeners registered for @cpu's exit data
  */
-static int send_cpu_listeners(struct sk_buff *skb, unsigned int cpu)
+static void send_cpu_listeners(struct sk_buff *skb, unsigned int cpu)
 {
 	struct genlmsghdr *genlhdr = nlmsg_data((struct nlmsghdr *)skb->data);
 	struct listener_list *listeners;
 	struct listener *s, *tmp;
 	struct sk_buff *skb_next, *skb_cur = skb;
 	void *reply = genlmsg_data(genlhdr);
-	int rc, ret, delcount = 0;
+	int rc, delcount = 0;
 
 	rc = genlmsg_end(skb, reply);
 	if (rc < 0) {
 		nlmsg_free(skb);
-		return rc;
+		return;
 	}
 
 	rc = 0;
 	listeners = &per_cpu(listener_array, cpu);
 	down_read(&listeners->sem);
-	list_for_each_entry_safe(s, tmp, &listeners->list, list) {
+	list_for_each_entry(s, &listeners->list, list) {
 		skb_next = NULL;
 		if (!list_is_last(&s->list, &listeners->list)) {
 			skb_next = skb_clone(skb_cur, GFP_KERNEL);
-			if (!skb_next) {
-				nlmsg_free(skb_cur);
-				rc = -ENOMEM;
+			if (!skb_next)
 				break;
-			}
 		}
-		ret = genlmsg_unicast(skb_cur, s->pid);
-		if (ret == -ECONNREFUSED) {
+		rc = genlmsg_unicast(skb_cur, s->pid);
+		if (rc == -ECONNREFUSED) {
 			s->valid = 0;
 			delcount++;
-			rc = ret;
 		}
 		skb_cur = skb_next;
 	}
 	up_read(&listeners->sem);
 
+	if (skb_cur)
+		nlmsg_free(skb_cur);
+
 	if (!delcount)
-		return rc;
+		return;
 
 	/* Delete invalidated entries */
 	down_write(&listeners->sem);
@@ -171,7 +170,6 @@ static int send_cpu_listeners(struct sk_
 		}
 	}
 	up_write(&listeners->sem);
-	return rc;
 }
 
 static int fill_pid(pid_t pid, struct task_struct *pidtsk,

[Patch 3/3] delay accounting: temporarily enable by default

[Shailabh Nagar]

Enable delay accounting by default so that feature gets coverage testing
without requiring special measures/

Earlier, it was off by default and had to be enabled via a boot time param.
This patch reverses the default behaviour to improve coverage testing. It
can be removed late in the kernel development cycle if its believed users
shouldn't have to incur any cost if they don't want delay accounting. Or it
can be retained forever if the utility of the stats is deemed common enough
to warrant keeping the feature on.

Signed-Off-By: Shailabh Nagar <nagar@watson.ibm.com>

 Documentation/accounting/delay-accounting.txt |   10 ++++++----
 Documentation/kernel-parameters.txt           |    4 ++--
 include/linux/delayacct.h                     |    4 ++--
 kernel/delayacct.c                            |    8 ++++----
 4 files changed, 14 insertions(+), 12 deletions(-)

Index: linux-2.6.18-rc2/Documentation/accounting/delay-accounting.txt
===================================================================
--- linux-2.6.18-rc2.orig/Documentation/accounting/delay-accounting.txt	2006-07-17 17:01:41.000000000 -0400
+++ linux-2.6.18-rc2/Documentation/accounting/delay-accounting.txt	2006-07-17 17:07:54.000000000 -0400
@@ -64,11 +64,13 @@ Compile the kernel with
 	CONFIG_TASK_DELAY_ACCT=y
 	CONFIG_TASKSTATS=y
 
-Enable the accounting at boot time by adding
-the following to the kernel boot options
-	delayacct
+Delay accounting is enabled by default at boot up.
+To disable, add
+   nodelayacct
+to the kernel boot options. The rest of the instructions
+below assume this has not been done.
 
-and after the system has booted up, use a utility
+After the system has booted up, use a utility
 similar to  getdelays.c to access the delays
 seen by a given task or a task group (tgid).
 The utility also allows a given command to be
Index: linux-2.6.18-rc2/kernel/delayacct.c
===================================================================
--- linux-2.6.18-rc2.orig/kernel/delayacct.c	2006-07-17 17:01:51.000000000 -0400
+++ linux-2.6.18-rc2/kernel/delayacct.c	2006-07-17 17:07:54.000000000 -0400
@@ -19,15 +19,15 @@
 #include <linux/sysctl.h>
 #include <linux/delayacct.h>
 
-int delayacct_on __read_mostly;	/* Delay accounting turned on/off */
+int delayacct_on __read_mostly = 1;	/* Delay accounting turned on/off */
 kmem_cache_t *delayacct_cache;
 
-static int __init delayacct_setup_enable(char *str)
+static int __init delayacct_setup_disable(char *str)
 {
-	delayacct_on = 1;
+	delayacct_on = 0;
 	return 1;
 }
-__setup("delayacct", delayacct_setup_enable);
+__setup("nodelayacct", delayacct_setup_disable);
 
 void delayacct_init(void)
 {
Index: linux-2.6.18-rc2/Documentation/kernel-parameters.txt
===================================================================
--- linux-2.6.18-rc2.orig/Documentation/kernel-parameters.txt	2006-07-17 17:01:41.000000000 -0400
+++ linux-2.6.18-rc2/Documentation/kernel-parameters.txt	2006-07-17 17:07:54.000000000 -0400
@@ -448,8 +448,6 @@ running once the system is up.
 			Format: <area>[,<node>]
 			See also Documentation/networking/decnet.txt.
 
-	delayacct	[KNL] Enable per-task delay accounting
-
 	dhash_entries=	[KNL]
 			Set number of hash buckets for dentry cache.
 
@@ -1031,6 +1029,8 @@ running once the system is up.
 
 	nocache		[ARM]
 
+	nodelayacct	[KNL] Disable per-task delay accounting
+
 	nodisconnect	[HW,SCSI,M68K] Disables SCSI disconnects.
 
 	noexec		[IA-64]
Index: linux-2.6.18-rc2/include/linux/delayacct.h
===================================================================
--- linux-2.6.18-rc2.orig/include/linux/delayacct.h	2006-07-17 17:03:41.000000000 -0400
+++ linux-2.6.18-rc2/include/linux/delayacct.h	2006-07-17 17:07:54.000000000 -0400
@@ -55,7 +55,7 @@ static inline void delayacct_tsk_init(st
 {
 	/* reinitialize in case parent's non-null pointer was dup'ed*/
 	tsk->delays = NULL;
-	if (unlikely(delayacct_on))
+	if (delayacct_on)
 		__delayacct_tsk_init(tsk);
 }
 
@@ -80,7 +80,7 @@ static inline void delayacct_blkio_end(v
 static inline int delayacct_add_tsk(struct taskstats *d,
 					struct task_struct *tsk)
 {
-	if (likely(!delayacct_on) || !tsk->delays)
+	if (!delayacct_on || !tsk->delays)
 		return 0;
 	return __delayacct_add_tsk(d, tsk);
 }