[PATCH 1/3] taskstats-fork: Add a new taskstats command to get notification on fork/clone

[PATCH 1/3] taskstats-fork: Add a new taskstats command to get notification on fork/clone

[Nikanth Karthikesan]

Add a new taskstats command to register for notification, whenever a new task
forks in the cpumask specified.

Signed-off-by: Nikanth Karthikesan <knikanth@suse.de>

---

diff --git a/include/linux/taskstats.h b/include/linux/taskstats.h
index 341dddb..1623a1f 100644
--- a/include/linux/taskstats.h
+++ b/include/linux/taskstats.h
@@ -188,6 +188,7 @@ enum {
 	TASKSTATS_TYPE_STATS,		/* taskstats structure */
 	TASKSTATS_TYPE_AGGR_PID,	/* contains pid + stats */
 	TASKSTATS_TYPE_AGGR_TGID,	/* contains tgid + stats */
+	TASKSTATS_TYPE_PID_TGID,	/* contains pid + tgid */
 	__TASKSTATS_TYPE_MAX,
 };
 
@@ -199,6 +200,8 @@ enum {
 	TASKSTATS_CMD_ATTR_TGID,
 	TASKSTATS_CMD_ATTR_REGISTER_CPUMASK,
 	TASKSTATS_CMD_ATTR_DEREGISTER_CPUMASK,
+	TASKSTATS_CMD_ATTR_REGISTER_FORK_CPUMASK,
+	TASKSTATS_CMD_ATTR_DEREGISTER_FORK_CPUMASK,
 	__TASKSTATS_CMD_ATTR_MAX,
 };
 
diff --git a/include/linux/taskstats_kern.h b/include/linux/taskstats_kern.h
index 7e9680f..b727370 100644
--- a/include/linux/taskstats_kern.h
+++ b/include/linux/taskstats_kern.h
@@ -26,9 +26,12 @@ static inline void taskstats_tgid_free(struct signal_struct *sig)
 		kmem_cache_free(taskstats_cache, sig->stats);
 }
 
+extern void taskstats_fork(struct task_struct *);
 extern void taskstats_exit(struct task_struct *, int group_dead);
 extern void taskstats_init_early(void);
 #else
+static inline void taskstats_fork(struct task_struct *tsk)
+{}
 static inline void taskstats_exit(struct task_struct *tsk, int group_dead)
 {}
 static inline void taskstats_tgid_init(struct signal_struct *sig)
diff --git a/kernel/fork.c b/kernel/fork.c
index 467746b..8ed8c9f 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -1447,6 +1447,7 @@ long do_fork(unsigned long clone_flags,
 			freezer_count();
 			tracehook_report_vfork_done(p, nr);
 		}
+		taskstats_fork(p);
 	} else {
 		nr = PTR_ERR(p);
 	}
diff --git a/kernel/taskstats.c b/kernel/taskstats.c
index 888adbc..8fcb7d3 100644
--- a/kernel/taskstats.c
+++ b/kernel/taskstats.c
@@ -51,7 +51,9 @@ __read_mostly = {
 	[TASKSTATS_CMD_ATTR_PID]  = { .type = NLA_U32 },
 	[TASKSTATS_CMD_ATTR_TGID] = { .type = NLA_U32 },
 	[TASKSTATS_CMD_ATTR_REGISTER_CPUMASK] = { .type = NLA_STRING },
-	[TASKSTATS_CMD_ATTR_DEREGISTER_CPUMASK] = { .type = NLA_STRING },};
+	[TASKSTATS_CMD_ATTR_DEREGISTER_CPUMASK] = { .type = NLA_STRING },
+	[TASKSTATS_CMD_ATTR_REGISTER_FORK_CPUMASK] = { .type = NLA_STRING },
+	[TASKSTATS_CMD_ATTR_DEREGISTER_FORK_CPUMASK] = { .type = NLA_STRING },};
 
 static struct nla_policy
 cgroupstats_cmd_get_policy[CGROUPSTATS_CMD_ATTR_MAX+1] __read_mostly = {
@@ -68,7 +70,13 @@ struct listener_list {
 	struct rw_semaphore sem;
 	struct list_head list;
 };
-static DEFINE_PER_CPU(struct listener_list, listener_array);
+static DEFINE_PER_CPU(struct listener_list, fork_listener_array);
+static DEFINE_PER_CPU(struct listener_list, exit_listener_array);
+
+enum forkexit {
+	FORK,
+	EXIT
+};
 
 enum actions {
 	REGISTER,
@@ -290,7 +298,8 @@ ret:
 	return;
 }
 
-static int add_del_listener(pid_t pid, const struct cpumask *mask, int isadd)
+static int add_del_listener(pid_t pid, const struct cpumask *mask, int isadd,
+							 enum forkexit forkexit)
 {
 	struct listener_list *listeners;
 	struct listener *s, *tmp;
@@ -309,7 +318,11 @@ static int add_del_listener(pid_t pid, const struct cpumask *mask, int isadd)
 			INIT_LIST_HEAD(&s->list);
 			s->valid = 1;
 
-			listeners = &per_cpu(listener_array, cpu);
+			if (forkexit == FORK)
+				listeners = &per_cpu(fork_listener_array, cpu);
+			else /* forkexit == EXIT */
+				listeners = &per_cpu(exit_listener_array, cpu);
+
 			down_write(&listeners->sem);
 			list_add(&s->list, &listeners->list);
 			up_write(&listeners->sem);
@@ -320,7 +333,12 @@ static int add_del_listener(pid_t pid, const struct cpumask *mask, int isadd)
 	/* Deregister or cleanup */
 cleanup:
 	for_each_cpu(cpu, mask) {
-		listeners = &per_cpu(listener_array, cpu);
+
+		if (forkexit == FORK)
+			listeners = &per_cpu(fork_listener_array, cpu);
+		else /* forkexit == EXIT */
+			listeners = &per_cpu(exit_listener_array, cpu);
+
 		down_write(&listeners->sem);
 		list_for_each_entry_safe(s, tmp, &listeners->list, list) {
 			if (s->pid == pid) {
@@ -436,11 +454,27 @@ static int taskstats_user_cmd(struct sk_buff *skb, struct genl_info *info)
 	if (!alloc_cpumask_var(&mask, GFP_KERNEL))
 		return -ENOMEM;
 
+	rc = parse(info->attrs[TASKSTATS_CMD_ATTR_REGISTER_FORK_CPUMASK], mask);
+	if (rc < 0)
+		goto free_return_rc;
+	if (rc == 0) {
+		rc = add_del_listener(info->snd_pid, mask, REGISTER, FORK);
+		goto free_return_rc;
+	}
+
+	rc = parse(info->attrs[TASKSTATS_CMD_ATTR_DEREGISTER_FORK_CPUMASK], mask);
+	if (rc < 0)
+		goto free_return_rc;
+	if (rc == 0) {
+		rc = add_del_listener(info->snd_pid, mask, DEREGISTER, FORK);
+		goto free_return_rc;
+	}
+
 	rc = parse(info->attrs[TASKSTATS_CMD_ATTR_REGISTER_CPUMASK], mask);
 	if (rc < 0)
 		goto free_return_rc;
 	if (rc == 0) {
-		rc = add_del_listener(info->snd_pid, mask, REGISTER);
+		rc = add_del_listener(info->snd_pid, mask, REGISTER, EXIT);
 		goto free_return_rc;
 	}
 
@@ -448,7 +482,7 @@ static int taskstats_user_cmd(struct sk_buff *skb, struct genl_info *info)
 	if (rc < 0)
 		goto free_return_rc;
 	if (rc == 0) {
-		rc = add_del_listener(info->snd_pid, mask, DEREGISTER);
+		rc = add_del_listener(info->snd_pid, mask, DEREGISTER, EXIT);
 free_return_rc:
 		free_cpumask_var(mask);
 		return rc;
@@ -517,6 +551,44 @@ ret:
 	return sig->stats;
 }
 
+/* Send pid out on fork/clone */
+void taskstats_fork(struct task_struct *tsk)
+{
+	struct sk_buff *rep_skb;
+	size_t size;
+	struct listener_list *listeners;
+	struct nlattr *na;
+
+	if (!family_registered)
+		return;
+
+	size = nla_total_size(sizeof(u32)) + nla_total_size(sizeof(tsk->tgid))
+				 + nla_total_size(0);
+
+	listeners = &__raw_get_cpu_var(fork_listener_array);
+	if (list_empty(&listeners->list))
+		return;
+
+	if (prepare_reply(NULL, TASKSTATS_CMD_NEW, &rep_skb, size) < 0)
+		return;
+
+	na = nla_nest_start(rep_skb, TASKSTATS_TYPE_PID_TGID);
+	if (!na)
+		goto err;
+	if (nla_put(rep_skb, TASKSTATS_TYPE_PID, sizeof(tsk->pid), &tsk->pid) < 0)
+		goto err;
+	if (nla_put(rep_skb, TASKSTATS_TYPE_TGID, sizeof(tsk->tgid), &tsk->tgid) < 0)
+		goto err;
+        nla_nest_end(rep_skb, na);
+
+	send_cpu_listeners(rep_skb, listeners);
+	return;
+
+err:
+	nlmsg_free(rep_skb);
+
+}
+
 /* Send pid data out on exit */
 void taskstats_exit(struct task_struct *tsk, int group_dead)
 {
@@ -544,7 +616,7 @@ void taskstats_exit(struct task_struct *tsk, int group_dead)
 		fill_tgid_exit(tsk);
 	}
 
-	listeners = &__raw_get_cpu_var(listener_array);
+	listeners = &__raw_get_cpu_var(exit_listener_array);
 	if (list_empty(&listeners->list))
 		return;
 
@@ -598,8 +670,10 @@ void __init taskstats_init_early(void)
 
 	taskstats_cache = KMEM_CACHE(taskstats, SLAB_PANIC);
 	for_each_possible_cpu(i) {
-		INIT_LIST_HEAD(&(per_cpu(listener_array, i).list));
-		init_rwsem(&(per_cpu(listener_array, i).sem));
+		INIT_LIST_HEAD(&(per_cpu(fork_listener_array, i).list));
+		init_rwsem(&(per_cpu(fork_listener_array, i).sem));
+		INIT_LIST_HEAD(&(per_cpu(exit_listener_array, i).list));
+		init_rwsem(&(per_cpu(exit_listener_array, i).sem));
 	}
 }
 

Re: [PATCH 1/3] taskstats-fork: Add a new taskstats command to get notification on fork/clone

[Balbir Singh]

* Nikanth Karthikesan <knikanth@suse.de> [2009-07-21 10:31:48]:

> Add a new taskstats command to register for notification, whenever a new task
> forks in the cpumask specified.
>

The changelog is sucky.. why do we need this? Why is proc-events not
sufficient?
 

-- 
	Balbir

Re: [PATCH 1/3] taskstats-fork: Add a new taskstats command to get notification on fork/clone

[Nikanth Karthikesan]

On Tuesday 21 July 2009 11:31:14 Balbir Singh wrote:
> * Nikanth Karthikesan <knikanth@suse.de> [2009-07-21 10:31:48]:
> > Add a new taskstats command to register for notification, whenever a new
> > task forks in the cpumask specified.
>
> The changelog is sucky.. why do we need this? Why is proc-events not
> sufficient?

Ah.. proc-events was the exact thing, I was looking for! Thanks. But it seems 
there is no documentation for it? May be I didn't search properly...

BTW I did mention the need for this, in 
http://patchwork.kernel.org/patch/36451/ , but didn't invest time to write a 
good changelog here, as I suspected something exists already. Sorry. ;-(

[CC-ed iotop/top people, who were CC-ed in the above mail. So that they can 
consider using proc-events interface, if it was not considered/used already]

Thanks
Nikanth

Re: [PATCH 1/3] taskstats-fork: Add a new taskstats command to get notification on fork/clone

[Guillaume Chazarain]

On Tue, Jul 21, 2009 at 8:24 AM, Nikanth Karthikesan<knikanth@suse.de> wrote:
> Ah.. proc-events was the exact thing, I was looking for! Thanks.

AFAICT proc-events can only be used by root, so that makes it
unsuitable for *top.

Anyway, iotop will always have to iterate over all threads to query
their taskstats and other attributes, so the O(nr_threads) complexity
is unavoidable.

Thanks.

-- 
Guillaume

Re: [PATCH 1/3] taskstats-fork: Add a new taskstats command to get notification on fork/clone

[Balbir Singh]

* Guillaume Chazarain <guichaz@gmail.com> [2009-08-30 21:52:22]:

> On Tue, Jul 21, 2009 at 8:24 AM, Nikanth Karthikesan<knikanth@suse.de> wrote:
> > Ah.. proc-events was the exact thing, I was looking for! Thanks.
> 
> AFAICT proc-events can only be used by root, so that makes it
> unsuitable for *top.
> 
> Anyway, iotop will always have to iterate over all threads to query
> their taskstats and other attributes, so the O(nr_threads) complexity
> is unavoidable.
>

Unless you want the stats for tgid in signal struct. In some cases we
accumulate, in some we don't. But yes irrespective or kernel/user
space the complexity exists. 

-- 
	Balbir

Re: [PATCH 1/3] taskstats-fork: Add a new taskstats command to get notification on fork/clone

[Nikanth Karthikesan]

On Monday 31 August 2009 01:22:22 Guillaume Chazarain wrote:
> On Tue, Jul 21, 2009 at 8:24 AM, Nikanth Karthikesan<knikanth@suse.de> 
wrote:
> > Ah.. proc-events was the exact thing, I was looking for! Thanks.
>
> AFAICT proc-events can only be used by root, so that makes it
> unsuitable for *top.
>
> Anyway, iotop will always have to iterate over all threads to query
> their taskstats and other attributes, so the O(nr_threads) complexity
> is unavoidable.
>

If it would simply iterate over all the current threads, the complexity would 
be O(nr_threads). But while iterating through all the current threads, it 
checks whether it is a new thread or a known thread, so that it can calculate 
the delta values. This is what I optimized by using taskstats-fork, which 
could be optimized using proc-events. 

Thanks
Nikanth