Linux Scalability Effort

This patch makes process events utilize task watchers instead of calling from
fork, exec, exit, and [re][ug]id changes directly.

Signed-off-by: Matt Helsley <matthltc@...>
Cc: Guillaume Thouvenin <guillaume.thouvenin@...>
--

 drivers/connector/cn_proc.c |   65 +++++++++++++++++++++++++++++++++++++-------
 fs/exec.c                   |    2 -
 include/linux/cn_proc.h     |   22 --------------
 kernel/exit.c               |    2 -
 kernel/fork.c               |    7 +---
 kernel/sys.c                |    8 -----
 6 files changed, 57 insertions(+), 49 deletions(-)

Index: linux-2.6.17-rc6-mm2/drivers/connector/cn_proc.c
===================================================================
--- linux-2.6.17-rc6-mm2.orig/drivers/connector/cn_proc.c
+++ linux-2.6.17-rc6-mm2/drivers/connector/cn_proc.c
@@ -25,10 +25,11 @@
 #include <linux/module.h>
 #include <linux/kernel.h>
 #include <linux/ktime.h>
 #include <linux/init.h>
 #include <linux/connector.h>
+#include <linux/notifier.h>
 #include <asm/atomic.h>
 
 #include <linux/cn_proc.h>
 
 #define CN_PROC_MSG_SIZE (sizeof(struct cn_msg) + sizeof(struct proc_event))
@@ -44,11 +45,11 @@ static inline void get_seq(__u32 *ts, in
 	*ts = get_cpu_var(proc_event_counts)++;
 	*cpu = smp_processor_id();
 	put_cpu_var(proc_event_counts);
 }
 
-void proc_fork_connector(struct task_struct *task)
+static void proc_fork_connector(struct task_struct *task)
 {
 	struct cn_msg *msg;
 	struct proc_event *ev;
 	__u8 buffer[CN_PROC_MSG_SIZE];
 
@@ -67,14 +68,14 @@ void proc_fork_connector(struct task_str
 
 	memcpy(&msg->id, &cn_proc_event_id, sizeof(msg->id));
 	msg->ack = 0; /* not used */
 	msg->len = sizeof(*ev);
 	/*  If cn_netlink_send() failed, the data is not sent */
-	cn_netlink_send(msg, CN_IDX_PROC, GFP_KERNEL);
+	cn_netlink_send(msg, CN_IDX_PROC, GFP_ATOMIC);
 }
 
-void proc_exec_connector(struct task_struct *task)
+static void proc_exec_connector(struct task_struct *task)
 {
 	struct cn_msg *msg;
 	struct proc_event *ev;
 	__u8 buffer[CN_PROC_MSG_SIZE];
 
@@ -90,14 +91,14 @@ void proc_exec_connector(struct task_str
 	ev->event_data.exec.process_tgid = task->tgid;
 
 	memcpy(&msg->id, &cn_proc_event_id, sizeof(msg->id));
 	msg->ack = 0; /* not used */
 	msg->len = sizeof(*ev);
-	cn_netlink_send(msg, CN_IDX_PROC, GFP_KERNEL);
+	cn_netlink_send(msg, CN_IDX_PROC, GFP_ATOMIC);
 }
 
-void proc_id_connector(struct task_struct *task, int which_id)
+static void proc_id_connector(struct task_struct *task, int which_id)
 {
 	struct cn_msg *msg;
 	struct proc_event *ev;
 	__u8 buffer[CN_PROC_MSG_SIZE];
 
@@ -121,14 +122,14 @@ void proc_id_connector(struct task_struc
 	ktime_get_ts(&ev->timestamp);
 
 	memcpy(&msg->id, &cn_proc_event_id, sizeof(msg->id));
 	msg->ack = 0; /* not used */
 	msg->len = sizeof(*ev);
-	cn_netlink_send(msg, CN_IDX_PROC, GFP_KERNEL);
+	cn_netlink_send(msg, CN_IDX_PROC, GFP_ATOMIC);
 }
 
-void proc_exit_connector(struct task_struct *task)
+static void proc_exit_connector(struct task_struct *task)
 {
 	struct cn_msg *msg;
 	struct proc_event *ev;
 	__u8 buffer[CN_PROC_MSG_SIZE];
 
@@ -146,11 +147,11 @@ void proc_exit_connector(struct task_str
 	ev->event_data.exit.exit_signal = task->exit_signal;
 
 	memcpy(&msg->id, &cn_proc_event_id, sizeof(msg->id));
 	msg->ack = 0; /* not used */
 	msg->len = sizeof(*ev);
-	cn_netlink_send(msg, CN_IDX_PROC, GFP_KERNEL);
+	cn_netlink_send(msg, CN_IDX_PROC, GFP_ATOMIC);
 }
 
 /*
  * Send an acknowledgement message to userspace
  *
@@ -207,10 +208,49 @@ static void cn_proc_mcast_ctl(void *data
 		break;
 	}
 	cn_proc_ack(err, msg->seq, msg->ack);
 }
 
+
+/*
+ * Dispatch task watcher events to the appropriate process event
+ * generation function.
+ */
+static int cn_proc_watch_task(struct notifier_block *nb, unsigned long val,
+			      void *t)
+{
+	struct task_struct *task = t;
+	int rc = NOTIFY_OK;
+
+	switch (get_watch_event(val)) {
+	case WATCH_TASK_CLONE:
+		proc_fork_connector(task);
+		break;
+	case WATCH_TASK_EXEC:
+		proc_exec_connector(task);
+		break;
+	case WATCH_TASK_UID:
+		proc_id_connector(task, PROC_EVENT_UID);
+		break;
+	case WATCH_TASK_GID:
+		proc_id_connector(task, PROC_EVENT_GID);
+		break;
+	case WATCH_TASK_EXIT:
+		proc_exit_connector(task);
+		break;
+	default: /* we don't care about WATCH_TASK_INIT|FREE because we
+		    don't keep per-task info */
+		rc = NOTIFY_DONE; /* ignore all other notifications */
+		break;
+	}
+	return rc;
+}
+
+static struct notifier_block __read_mostly cn_proc_nb = {
+	.notifier_call = cn_proc_watch_task,
+};
+
 /*
  * cn_proc_init - initialization entry point
  *
  * Adds the connector callback to the connector driver.
  */
@@ -219,11 +259,16 @@ static int __init cn_proc_init(void)
 	int err;
 
 	if ((err = cn_add_callback(&cn_proc_event_id, "cn_proc",
 	 			   &cn_proc_mcast_ctl))) {
 		printk(KERN_WARNING "cn_proc failed to register\n");
-		return err;
+		goto out;
 	}
-	return 0;
+
+	err = register_task_watcher(&cn_proc_nb);
+	if (err != 0)
+		cn_del_callback(&cn_proc_event_id);
+out:
+	return err;
 }
 
 module_init(cn_proc_init);
Index: linux-2.6.17-rc6-mm2/include/linux/cn_proc.h
===================================================================
--- linux-2.6.17-rc6-mm2.orig/include/linux/cn_proc.h
+++ linux-2.6.17-rc6-mm2/include/linux/cn_proc.h
@@ -93,28 +93,6 @@ struct proc_event {
 			pid_t process_tgid;
 			__u32 exit_code, exit_signal;
 		} exit;
 	} event_data;
 };
-
-#ifdef __KERNEL__
-#ifdef CONFIG_PROC_EVENTS
-void proc_fork_connector(struct task_struct *task);
-void proc_exec_connector(struct task_struct *task);
-void proc_id_connector(struct task_struct *task, int which_id);
-void proc_exit_connector(struct task_struct *task);
-#else
-static inline void proc_fork_connector(struct task_struct *task)
-{}
-
-static inline void proc_exec_connector(struct task_struct *task)
-{}
-
-static inline void proc_id_connector(struct task_struct *task,
-				     int which_id)
-{}
-
-static inline void proc_exit_connector(struct task_struct *task)
-{}
-#endif	/* CONFIG_PROC_EVENTS */
-#endif	/* __KERNEL__ */
 #endif	/* CN_PROC_H */
Index: linux-2.6.17-rc6-mm2/kernel/exit.c
===================================================================
--- linux-2.6.17-rc6-mm2.orig/kernel/exit.c
+++ linux-2.6.17-rc6-mm2/kernel/exit.c
@@ -31,11 +31,10 @@
 #include <linux/delayacct.h>
 #include <linux/cpuset.h>
 #include <linux/syscalls.h>
 #include <linux/signal.h>
 #include <linux/posix-timers.h>
-#include <linux/cn_proc.h>
 #include <linux/mutex.h>
 #include <linux/futex.h>
 #include <linux/compat.h>
 #include <linux/pipe_fs_i.h>
 #include <linux/audit.h> /* for audit_free() */
@@ -941,11 +940,10 @@ fastcall NORET_TYPE void do_exit(long co
 
 	module_put(task_thread_info(tsk)->exec_domain->module);
 	if (tsk->binfmt)
 		module_put(tsk->binfmt->module);
 
-	proc_exit_connector(tsk);
 	exit_notify(tsk);
 #ifdef CONFIG_NUMA
 	mpol_free(tsk->mempolicy);
 	tsk->mempolicy = NULL;
 #endif
Index: linux-2.6.17-rc6-mm2/kernel/fork.c
===================================================================
--- linux-2.6.17-rc6-mm2.orig/kernel/fork.c
+++ linux-2.6.17-rc6-mm2/kernel/fork.c
@@ -42,11 +42,10 @@
 #include <linux/mount.h>
 #include <linux/audit.h>
 #include <linux/profile.h>
 #include <linux/rmap.h>
 #include <linux/acct.h>
-#include <linux/cn_proc.h>
 #include <linux/delayacct.h>
 #include <linux/notifier.h>
 
 #include <asm/pgtable.h>
 #include <asm/pgalloc.h>
@@ -1251,14 +1250,12 @@ static task_t *copy_process(unsigned lon
 
 	total_forks++;
 	spin_unlock(&current->sighand->siglock);
 	write_unlock_irq(&tasklist_lock);
 	notify_result = notify_watchers(WATCH_TASK_CLONE, p);
-	if (notify_result & NOTIFY_STOP_MASK)
-		goto bad_fork_cleanup_namespaces;
-	proc_fork_connector(p);
-	return p;
+	if (!(notify_result & NOTIFY_STOP_MASK))
+		return p;
 
 bad_fork_cleanup_namespaces:
 	exit_task_namespaces(p);
 bad_fork_cleanup_keys:
 	exit_keys(p);
Index: linux-2.6.17-rc6-mm2/fs/exec.c
===================================================================
--- linux-2.6.17-rc6-mm2.orig/fs/exec.c
+++ linux-2.6.17-rc6-mm2/fs/exec.c
@@ -46,11 +46,10 @@
 #include <linux/mount.h>
 #include <linux/security.h>
 #include <linux/syscalls.h>
 #include <linux/rmap.h>
 #include <linux/acct.h>
-#include <linux/cn_proc.h>
 #include <linux/audit.h>
 #include <linux/notifier.h>
 
 #include <asm/uaccess.h>
 #include <asm/mmu_context.h>
@@ -1097,11 +1096,10 @@ int search_binary_handler(struct linux_b
 				if (bprm->file)
 					fput(bprm->file);
 				bprm->file = NULL;
 				current->did_exec = 1;
 				notify_watchers(WATCH_TASK_EXEC, current);
-				proc_exec_connector(current);
 				return retval;
 			}
 			read_lock(&binfmt_lock);
 			put_binfmt(fmt);
 			if (retval != -ENOEXEC || bprm->mm == NULL)
Index: linux-2.6.17-rc6-mm2/kernel/sys.c
===================================================================
--- linux-2.6.17-rc6-mm2.orig/kernel/sys.c
+++ linux-2.6.17-rc6-mm2/kernel/sys.c
@@ -862,11 +862,10 @@ asmlinkage long sys_setregid(gid_t rgid,
 	current->fsgid = new_egid;
 	current->egid = new_egid;
 	current->gid = new_rgid;
 	key_fsgid_changed(current);
 	notify_watchers(WATCH_TASK_GID, current);
-	proc_id_connector(current, PROC_EVENT_GID);
 	return 0;
 }
 
 /*
  * setgid() is implemented like SysV w/ SAVED_IDS 
@@ -903,11 +902,10 @@ asmlinkage long sys_setgid(gid_t gid)
 	else
 		return -EPERM;
 
 	key_fsgid_changed(current);
 	notify_watchers(WATCH_TASK_GID, current);
-	proc_id_connector(current, PROC_EVENT_GID);
 	return 0;
 }
   
 static int set_user(uid_t new_ruid, int dumpclear)
 {
@@ -994,11 +992,10 @@ asmlinkage long sys_setreuid(uid_t ruid,
 		current->suid = current->euid;
 	current->fsuid = current->euid;
 
 	key_fsuid_changed(current);
 	notify_watchers(WATCH_TASK_UID, current);
-	proc_id_connector(current, PROC_EVENT_UID);
 
 	return security_task_post_setuid(old_ruid, old_euid, old_suid, LSM_SETID_RE);
 }
 

@@ -1043,11 +1040,10 @@ asmlinkage long sys_setuid(uid_t uid)
 	current->fsuid = current->euid = uid;
 	current->suid = new_suid;
 
 	key_fsuid_changed(current);
 	notify_watchers(WATCH_TASK_UID, current);
-	proc_id_connector(current, PROC_EVENT_UID);
 
 	return security_task_post_setuid(old_ruid, old_euid, old_suid, LSM_SETID_ID);
 }
 

@@ -1093,11 +1089,10 @@ asmlinkage long sys_setresuid(uid_t ruid
 	if (suid != (uid_t) -1)
 		current->suid = suid;
 
 	key_fsuid_changed(current);
 	notify_watchers(WATCH_TASK_UID, current);
-	proc_id_connector(current, PROC_EVENT_UID);
 
 	return security_task_post_setuid(old_ruid, old_euid, old_suid, LSM_SETID_RES);
 }
 
 asmlinkage long sys_getresuid(uid_t __user *ruid, uid_t __user *euid, uid_t __user *suid)
@@ -1147,11 +1142,10 @@ asmlinkage long sys_setresgid(gid_t rgid
 	if (sgid != (gid_t) -1)
 		current->sgid = sgid;
 
 	key_fsgid_changed(current);
 	notify_watchers(WATCH_TASK_GID, current);
-	proc_id_connector(current, PROC_EVENT_GID);
 	return 0;
 }
 
 asmlinkage long sys_getresgid(gid_t __user *rgid, gid_t __user *egid, gid_t __user *sgid)
 {
@@ -1191,11 +1185,10 @@ asmlinkage long sys_setfsuid(uid_t uid)
 		current->fsuid = uid;
 	}
 
 	key_fsuid_changed(current);
 	notify_watchers(WATCH_TASK_UID, current);
-	proc_id_connector(current, PROC_EVENT_UID);
 
 	security_task_post_setuid(old_fsuid, (uid_t)-1, (uid_t)-1, LSM_SETID_FS);
 
 	return old_fsuid;
 }
@@ -1221,11 +1214,10 @@ asmlinkage long sys_setfsgid(gid_t gid)
 			smp_wmb();
 		}
 		current->fsgid = gid;
 		key_fsgid_changed(current);
 		notify_watchers(WATCH_TASK_GID, current);
-		proc_id_connector(current, PROC_EVENT_GID);
 	}
 	return old_fsgid;
 }
 
 asmlinkage long sys_times(struct tms __user * tbuf)

--

This patch changes process events so that it may be configured as a module. 

Signed-off-by: Matt Helsley <matthltc@...>
Cc: Guillaume Thouvenin <guillaume.thouvenin@...>
--

 drivers/connector/Kconfig   |    8 ++++----
 drivers/connector/cn_proc.c |   18 ++++++++++++++++++
 2 files changed, 22 insertions(+), 4 deletions(-)

Index: linux-2.6.17-rc6-mm2/drivers/connector/Kconfig
===================================================================
--- linux-2.6.17-rc6-mm2.orig/drivers/connector/Kconfig
+++ linux-2.6.17-rc6-mm2/drivers/connector/Kconfig
@@ -9,13 +9,13 @@ config CONNECTOR
 
 	  Connector support can also be built as a module.  If so, the module
 	  will be called cn.ko.
 
 config PROC_EVENTS
-	boolean "Report process events to userspace"
-	depends on CONNECTOR=y
-	default y
-	---help---
+	tristate "Report process events to userspace"
+	default m
+	depends on CONNECTOR
+	help
 	  Provide a connector that reports process events to userspace. Send
 	  events such as fork, exec, id change (uid, gid, suid, etc), and exit.
 
 endmenu
Index: linux-2.6.17-rc6-mm2/drivers/connector/cn_proc.c
===================================================================
--- linux-2.6.17-rc6-mm2.orig/drivers/connector/cn_proc.c
+++ linux-2.6.17-rc6-mm2/drivers/connector/cn_proc.c
@@ -224,6 +224,24 @@ static int __init cn_proc_init(void)
 		cn_del_callback(&cn_proc_event_id);
 out:
 	return err;
 }
 
+static void cn_proc_fini(void)
+{
+	int err;
+
+	err = unregister_task_watcher(&cn_proc_nb);
+	if (err != 0)
+		printk(KERN_WARNING
+		       "cn_proc failed to unregister its task notify block\n");
+
+	cn_del_callback(&cn_proc_event_id);
+}
+
 module_init(cn_proc_init);
+module_exit(cn_proc_fini);
+
+MODULE_AUTHOR("Matt Helsley <matthltc@...>");
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Notification of process events.");
+MODULE_VERSION("2:1.0");

--

Adapt audit to use task watchers.

Signed-off-by: Matt Helsley <matthltc@...>
Cc: David Woodhouse <dwmw2@...>
Cc: linux-audit@...
--

 kernel/audit.c |   25 ++++++++++++++++++++++++-
 kernel/exit.c  |    3 ---
 kernel/fork.c  |    7 +------
 3 files changed, 25 insertions(+), 10 deletions(-)

Index: linux-2.6.17-rc5-mm2/kernel/exit.c
===================================================================
--- linux-2.6.17-rc5-mm2.orig/kernel/exit.c
+++ linux-2.6.17-rc5-mm2/kernel/exit.c
@@ -35,11 +35,10 @@
 #include <linux/posix-timers.h>
 #include <linux/mutex.h>
 #include <linux/futex.h>
 #include <linux/compat.h>
 #include <linux/pipe_fs_i.h>
-#include <linux/audit.h> /* for audit_free() */
 #include <linux/resource.h>
 #include <linux/notifier.h>
 
 #include <asm/uaccess.h>
 #include <asm/unistd.h>
@@ -914,12 +913,10 @@ fastcall NORET_TYPE void do_exit(long co
 		exit_robust_list(tsk);
 #ifdef CONFIG_COMPAT
 	if (unlikely(tsk->compat_robust_list))
 		compat_exit_robust_list(tsk);
 #endif
-	if (unlikely(tsk->audit_context))
-		audit_free(tsk);
 	tsk->exit_code = code;
 	taskstats_exit_send(tsk, tidstats, tgidstats);
 	taskstats_exit_free(tidstats, tgidstats);
 	delayacct_tsk_exit(tsk);
 	notify_result = notify_watchers(WATCH_TASK_FREE, tsk);
Index: linux-2.6.17-rc5-mm2/kernel/audit.c
===================================================================
--- linux-2.6.17-rc5-mm2.orig/kernel/audit.c
+++ linux-2.6.17-rc5-mm2/kernel/audit.c
@@ -46,10 +46,11 @@
 #include <asm/atomic.h>
 #include <linux/mm.h>
 #include <linux/module.h>
 #include <linux/err.h>
 #include <linux/kthread.h>
+#include <linux/notifier.h>
 
 #include <linux/audit.h>
 
 #include <net/sock.h>
 #include <net/netlink.h>
@@ -64,10 +65,30 @@
 static int	audit_initialized;
 
 /* No syscall auditing will take place unless audit_enabled != 0. */
 int		audit_enabled;
 
+static int audit_task(struct notifier_block *nb, unsigned long val, void *t)
+{
+	struct task_struct *tsk = t;
+
+	switch(get_watch_event(val)) {
+	case WATCH_TASK_INIT:
+		/* Hack: -EFOO sets NOTIFY_STOP_MASK */
+		return audit_alloc(tsk);
+	case WATCH_TASK_FREE:
+		if (unlikely(tsk->audit_context))
+			audit_free(tsk);
+	default:
+		return NOTIFY_DONE;
+	}
+}
+
+static struct notifier_block __read_mostly audit_watch_tasks_nb = {
+	.notifier_call = audit_task,
+};
+
 /* Default state when kernel boots without any parameters. */
 static int	audit_default;
 
 /* If auditing cannot proceed, audit_failure selects what happens. */
 static int	audit_failure = AUDIT_FAIL_PRINTK;
@@ -707,12 +728,14 @@ static int __init audit_enable(char *str
 {
 	audit_default = !!simple_strtol(str, NULL, 0);
 	printk(KERN_INFO "audit: %s%s\n",
 	       audit_default ? "enabled" : "disabled",
 	       audit_initialized ? "" : " (after initialization)");
-	if (audit_initialized)
+	if (audit_initialized) {
 		audit_enabled = audit_default;
+		register_task_watcher(&audit_watch_tasks_nb);
+	}
 	return 1;
 }
 
 __setup("audit=", audit_enable);
 
Index: linux-2.6.17-rc5-mm2/kernel/fork.c
===================================================================
--- linux-2.6.17-rc5-mm2.orig/kernel/fork.c
+++ linux-2.6.17-rc5-mm2/kernel/fork.c
@@ -38,11 +38,10 @@
 #include <linux/jiffies.h>
 #include <linux/futex.h>
 #include <linux/rcupdate.h>
 #include <linux/ptrace.h>
 #include <linux/mount.h>
-#include <linux/audit.h>
 #include <linux/profile.h>
 #include <linux/rmap.h>
 #include <linux/acct.h>
 #include <linux/delayacct.h>
 #include <linux/notifier.h>
@@ -1088,15 +1087,13 @@ static task_t *copy_process(unsigned lon
 	p->softirq_context = 0;
 #endif
 
 	if ((retval = security_task_alloc(p)))
 		goto bad_fork_cleanup_policy;
-	if ((retval = audit_alloc(p)))
-		goto bad_fork_cleanup_security;
 	/* copy all the process information */
 	if ((retval = copy_semundo(clone_flags, p)))
-		goto bad_fork_cleanup_audit;
+		goto bad_fork_cleanup_security;
 	if ((retval = copy_files(clone_flags, p)))
 		goto bad_fork_cleanup_semundo;
 	if ((retval = copy_fs(clone_flags, p)))
 		goto bad_fork_cleanup_files;
 	if ((retval = copy_sighand(clone_flags, p)))
@@ -1270,12 +1267,10 @@ bad_fork_cleanup_fs:
 	exit_fs(p); /* blocking */
 bad_fork_cleanup_files:
 	exit_files(p); /* blocking */
 bad_fork_cleanup_semundo:
 	exit_sem(p);
-bad_fork_cleanup_audit:
-	audit_free(p);
 bad_fork_cleanup_security:
 	security_task_free(p);
 	notify_result = notify_watchers(WATCH_TASK_FREE, p);
 	WARN_ON(notify_result & NOTIFY_STOP_MASK);
 bad_fork_cleanup_policy:

--

This patch switches task_watchers to from atomic to blocking notifier chains,
allowing notifier_calls to sleep.

Signed-off-by: Matt Helsley <matthltc@...>
--

 drivers/connector/cn_proc.c |    2 +-
 kernel/sys.c                |    8 ++++----
 2 files changed, 5 insertions(+), 5 deletions(-)

Index: linux-2.6.17-rc6-mm2/kernel/sys.c
===================================================================
--- linux-2.6.17-rc6-mm2.orig/kernel/sys.c
+++ linux-2.6.17-rc6-mm2/kernel/sys.c
@@ -434,29 +434,29 @@ int unregister_reboot_notifier(struct no
 }
 
 EXPORT_SYMBOL(unregister_reboot_notifier);
 
 /* task watchers notifier chain */
-static ATOMIC_NOTIFIER_HEAD(task_watchers);
+static BLOCKING_NOTIFIER_HEAD(task_watchers);
 
 int register_task_watcher(struct notifier_block *nb)
 {
-	return atomic_notifier_chain_register(&task_watchers, nb);
+	return blocking_notifier_chain_register(&task_watchers, nb);
 }
 
 EXPORT_SYMBOL_GPL(register_task_watcher);
 
 int unregister_task_watcher(struct notifier_block *nb)
 {
-	return atomic_notifier_chain_unregister(&task_watchers, nb);
+	return blocking_notifier_chain_unregister(&task_watchers, nb);
 }
 
 EXPORT_SYMBOL_GPL(unregister_task_watcher);
 
 int notify_watchers(unsigned long val, void *v)
 {
-	return atomic_notifier_call_chain(&task_watchers, val, v);
+	return blocking_notifier_call_chain(&task_watchers, val, v);
 }
 

 static int set_one_prio(struct task_struct *p, int niceval, int error)
 {
Index: linux-2.6.17-rc6-mm2/drivers/connector/cn_proc.c
===================================================================
--- linux-2.6.17-rc6-mm2.orig/drivers/connector/cn_proc.c
+++ linux-2.6.17-rc6-mm2/drivers/connector/cn_proc.c
@@ -193,11 +193,11 @@ static int cn_proc_watch_task(struct not
 	get_seq(&msg->seq, &ev->cpu);
 	ktime_get_ts(&ev->timestamp); /* get high res monotonic timestamp */
 	memcpy(&msg->id, &cn_proc_event_id, sizeof(msg->id));
 	msg->ack = 0; /* not used */
 	msg->len = sizeof(*ev);
-	cn_netlink_send(msg, CN_IDX_PROC, GFP_ATOMIC);
+	cn_netlink_send(msg, CN_IDX_PROC, GFP_KERNEL);
 	/* If cn_netlink_send() fails, drop data */
 	return rc;
 }
 
 static struct notifier_block __read_mostly cn_proc_nb = {

--

Adapts delayacct to use Task Watchers. Does not adapt taskstats to use Task
Watchers.

Signed-off-by: Matt Helsley <matthltc@...>
Cc: Shailabh Nagar <nagar@...>
Cc: Balbir Singh <balbir@...>
Cc: Chandra S. Seetharaman <sekharan@...>
--

 include/linux/delayacct.h |    2 +-
 kernel/delayacct.c        |   23 +++++++++++++++++++++++
 kernel/exit.c             |    2 --
 kernel/fork.c             |    2 --
 4 files changed, 24 insertions(+), 5 deletions(-)

Index: linux-2.6.17-rc5-mm2/kernel/exit.c
===================================================================
--- linux-2.6.17-rc5-mm2.orig/kernel/exit.c
+++ linux-2.6.17-rc5-mm2/kernel/exit.c
@@ -26,11 +26,10 @@
 #include <linux/profile.h>
 #include <linux/mount.h>
 #include <linux/proc_fs.h>
 #include <linux/mempolicy.h>
 #include <linux/taskstats_kern.h>
-#include <linux/delayacct.h>
 #include <linux/cpuset.h>
 #include <linux/syscalls.h>
 #include <linux/signal.h>
 #include <linux/posix-timers.h>
 #include <linux/mutex.h>
@@ -916,11 +915,10 @@ fastcall NORET_TYPE void do_exit(long co
 		compat_exit_robust_list(tsk);
 #endif
 	tsk->exit_code = code;
 	taskstats_exit_send(tsk, tidstats, tgidstats);
 	taskstats_exit_free(tidstats, tgidstats);
-	delayacct_tsk_exit(tsk);
 	notify_result = notify_watchers(WATCH_TASK_FREE, tsk);
 	WARN_ON(notify_result & NOTIFY_STOP_MASK);
 
 	exit_mm(tsk);
 
Index: linux-2.6.17-rc5-mm2/kernel/fork.c
===================================================================
--- linux-2.6.17-rc5-mm2.orig/kernel/fork.c
+++ linux-2.6.17-rc5-mm2/kernel/fork.c
@@ -41,11 +41,10 @@
 #include <linux/ptrace.h>
 #include <linux/mount.h>
 #include <linux/profile.h>
 #include <linux/rmap.h>
 #include <linux/acct.h>
-#include <linux/delayacct.h>
 #include <linux/notifier.h>
 
 #include <asm/pgtable.h>
 #include <asm/pgalloc.h>
 #include <asm/uaccess.h>
@@ -1002,11 +1001,10 @@ static task_t *copy_process(unsigned lon
 
 	if (p->binfmt && !try_module_get(p->binfmt->module))
 		goto bad_fork_cleanup_put_domain;
 
 	p->did_exec = 0;
-	delayacct_tsk_init(p);	/* Must remain after dup_task_struct() */
 	copy_flags(clone_flags, p);
 	p->pid = pid;
 	retval = -EFAULT;
 	if (clone_flags & CLONE_PARENT_SETTID)
 		if (put_user(p->pid, parent_tidptr))
Index: linux-2.6.17-rc5-mm2/kernel/delayacct.c
===================================================================
--- linux-2.6.17-rc5-mm2.orig/kernel/delayacct.c
+++ linux-2.6.17-rc5-mm2/kernel/delayacct.c
@@ -16,10 +16,11 @@
 #include <linux/sched.h>
 #include <linux/slab.h>
 #include <linux/time.h>
 #include <linux/sysctl.h>
 #include <linux/delayacct.h>
+#include <linux/notifier.h>
 
 int delayacct_on __read_mostly;	/* Delay accounting turned on/off */
 kmem_cache_t *delayacct_cache;
 
 static int __init delayacct_setup_enable(char *str)
@@ -27,17 +28,39 @@ static int __init delayacct_setup_enable
 	delayacct_on = 1;
 	return 1;
 }
 __setup("delayacct", delayacct_setup_enable);
 
+static int delayacct_watch_task(struct notifier_block *nb, unsigned long val,
+				void *t)
+{
+	struct task_struct *tsk = t;
+	switch(get_watch_event(val)) {
+	case WATCH_TASK_CLONE:
+		delayacct_tsk_init(tsk);
+		break;
+	case WATCH_TASK_FREE:
+		delayacct_tsk_exit(tsk);
+		break;
+	default:
+		return NOTIFY_DONE;
+	}
+	return NOTIFY_OK;
+}
+
+static struct notifier_block __read_mostly delayacct_nb = {
+	.notifier_call = delayacct_watch_task,
+};
+
 void delayacct_init(void)
 {
 	delayacct_cache = kmem_cache_create("delayacct_cache",
 					sizeof(struct task_delay_info),
 					0,
 					SLAB_PANIC,
 					NULL, NULL);
+	register_task_watcher(&delayacct_nb);
 	delayacct_tsk_init(&init_task);
 }
 
 void __delayacct_tsk_init(struct task_struct *tsk)
 {
Index: linux-2.6.17-rc5-mm2/include/linux/delayacct.h
===================================================================
--- linux-2.6.17-rc5-mm2.orig/include/linux/delayacct.h
+++ linux-2.6.17-rc5-mm2/include/linux/delayacct.h
@@ -59,11 +59,11 @@ static inline void delayacct_tsk_init(st
 		__delayacct_tsk_init(tsk);
 }
 
 static inline void delayacct_tsk_exit(struct task_struct *tsk)
 {
-	if (tsk->delays)
+	if (unlikely(tsk->delays))
 		__delayacct_tsk_exit(tsk);
 }
 
 static inline void delayacct_blkio_start(void)
 {

--

Modify oprofile to use the task watcher chain to watch for task exit.
oprofile uses task exit as a point to synch buffers.

This patch does not modify oprofile to use the task free path of task watchers. 
oprofile has its own task_free atomic notifier chain. Since its an atomic chain
we can't replace it with task watcher. Also, it's called much later when the
actual task struct is really about to be freed.

Signed-off-by: Matt Helsley <matthltc@...>
Cc: Philippe Elie <phil.el@...>
Cc: oprofile-list@...
--

 drivers/oprofile/buffer_sync.c |   11 ++++++-----
 include/linux/profile.h        |    3 +--
 kernel/exit.c                  |    1 -
 kernel/profile.c               |   14 --------------
 4 files changed, 7 insertions(+), 22 deletions(-)

Index: linux-2.6.17-rc6-mm2/kernel/exit.c
===================================================================
--- linux-2.6.17-rc6-mm2.orig/kernel/exit.c
+++ linux-2.6.17-rc6-mm2/kernel/exit.c
@@ -847,11 +847,10 @@ fastcall NORET_TYPE void do_exit(long co
 	struct task_struct *tsk = current;
 	struct taskstats *tidstats, *tgidstats;
 	int group_dead;
 	int notify_result;
 
-	profile_task_exit(tsk);
 	tsk->exit_code = code;
 	notify_result = notify_watchers(WATCH_TASK_EXIT, tsk);
 
 	WARN_ON(atomic_read(&tsk->fs_excl));
 
Index: linux-2.6.17-rc6-mm2/drivers/oprofile/buffer_sync.c
===================================================================
--- linux-2.6.17-rc6-mm2.orig/drivers/oprofile/buffer_sync.c
+++ linux-2.6.17-rc6-mm2/drivers/oprofile/buffer_sync.c
@@ -63,12 +63,13 @@ static int task_free_notify(struct notif
 static int task_exit_notify(struct notifier_block * self, unsigned long val, void * data)
 {
 	/* To avoid latency problems, we only process the current CPU,
 	 * hoping that most samples for the task are on this CPU
 	 */
-	sync_buffer(raw_smp_processor_id());
-  	return 0;
+	if (get_watch_event(val) == WATCH_TASK_EXIT)
+		sync_buffer(raw_smp_processor_id());
+  	return NOTIFY_DONE;
 }
 

 /* The task is about to try a do_munmap(). We peek at what it's going to
  * do, and if it's an executable region, process the samples first, so
@@ -150,11 +151,11 @@ int sync_start(void)
 	start_cpu_work();
 
 	err = task_handoff_register(&task_free_nb);
 	if (err)
 		goto out1;
-	err = profile_event_register(PROFILE_TASK_EXIT, &task_exit_nb);
+	err = register_task_watcher(&task_exit_nb);
 	if (err)
 		goto out2;
 	err = profile_event_register(PROFILE_MUNMAP, &munmap_nb);
 	if (err)
 		goto out3;
@@ -165,11 +166,11 @@ int sync_start(void)
 out:
 	return err;
 out4:
 	profile_event_unregister(PROFILE_MUNMAP, &munmap_nb);
 out3:
-	profile_event_unregister(PROFILE_TASK_EXIT, &task_exit_nb);
+	unregister_task_watcher(&task_exit_nb);
 out2:
 	task_handoff_unregister(&task_free_nb);
 out1:
 	end_sync();
 	goto out;
@@ -178,11 +179,11 @@ out1:
 
 void sync_stop(void)
 {
 	unregister_module_notifier(&module_load_nb);
 	profile_event_unregister(PROFILE_MUNMAP, &munmap_nb);
-	profile_event_unregister(PROFILE_TASK_EXIT, &task_exit_nb);
+	unregister_task_watcher(&task_exit_nb);
 	task_handoff_unregister(&task_free_nb);
 	end_sync();
 }
 
  
Index: linux-2.6.17-rc6-mm2/kernel/profile.c
===================================================================
--- linux-2.6.17-rc6-mm2.orig/kernel/profile.c
+++ linux-2.6.17-rc6-mm2/kernel/profile.c
@@ -85,19 +85,13 @@ void __init profile_init(void)
 
 /* Profile event notifications */
  
 #ifdef CONFIG_PROFILING
  
-static BLOCKING_NOTIFIER_HEAD(task_exit_notifier);
 static ATOMIC_NOTIFIER_HEAD(task_free_notifier);
 static BLOCKING_NOTIFIER_HEAD(munmap_notifier);
  
-void profile_task_exit(struct task_struct * task)
-{
-	blocking_notifier_call_chain(&task_exit_notifier, 0, task);
-}
- 
 int profile_handoff_task(struct task_struct * task)
 {
 	int ret;
 	ret = atomic_notifier_call_chain(&task_free_notifier, 0, task);
 	return (ret == NOTIFY_OK) ? 1 : 0;
@@ -121,14 +115,10 @@ int task_handoff_unregister(struct notif
 int profile_event_register(enum profile_type type, struct notifier_block * n)
 {
 	int err = -EINVAL;
  
 	switch (type) {
-		case PROFILE_TASK_EXIT:
-			err = blocking_notifier_chain_register(
-					&task_exit_notifier, n);
-			break;
 		case PROFILE_MUNMAP:
 			err = blocking_notifier_chain_register(
 					&munmap_notifier, n);
 			break;
 	}
@@ -140,14 +130,10 @@ int profile_event_register(enum profile_
 int profile_event_unregister(enum profile_type type, struct notifier_block * n)
 {
 	int err = -EINVAL;
  
 	switch (type) {
-		case PROFILE_TASK_EXIT:
-			err = blocking_notifier_chain_unregister(
-					&task_exit_notifier, n);
-			break;
 		case PROFILE_MUNMAP:
 			err = blocking_notifier_chain_unregister(
 					&munmap_notifier, n);
 			break;
 	}
Index: linux-2.6.17-rc6-mm2/include/linux/profile.h
===================================================================
--- linux-2.6.17-rc6-mm2.orig/include/linux/profile.h
+++ linux-2.6.17-rc6-mm2/include/linux/profile.h
@@ -24,12 +24,11 @@ void create_prof_cpu_mask(struct proc_di
 #else
 #define create_prof_cpu_mask(x)			do { (void)(x); } while (0)
 #endif
 
 enum profile_type {
-	PROFILE_TASK_EXIT,
-	PROFILE_MUNMAP
+	PROFILE_MUNMAP = 1
 };
 
 #ifdef CONFIG_PROFILING
 
 struct task_struct;

--

This introduces a second, per-task, blocking notifier chain. The per-task
chain offers watchers the chance to register with a specific task nstead of
all tasks. It also allows the watcher to associate a block of data with the task
by wrapping the notifier block using containerof().

Both the global, all-tasks chain and the per-task chain are called from the samefunction. The two types of chains share the same set of notification
values, however registration functions and the registered notifier blocks must
be separate.

These notifiers are only safe if notifier blocks are registered with the current
task while in the context of the current task. This ensures that there are no
races between registration, unregistration, and notification.

Signed-off-by: Matt Helsley <matthltc@...>
Cc: Jes Sorensen <jes@...>
Cc: Chandra S. Seetharaman <sekharan@...>
Cc: Alan Stern <stern@...>
--

 include/linux/init_task.h |    2 ++
 include/linux/notifier.h  |    2 ++
 include/linux/sched.h     |    2 ++
 kernel/sys.c              |   30 +++++++++++++++++++++++++++++-
 4 files changed, 35 insertions(+), 1 deletion(-)

Index: linux-2.6.17-rc6-mm2/include/linux/sched.h
===================================================================
--- linux-2.6.17-rc6-mm2.orig/include/linux/sched.h
+++ linux-2.6.17-rc6-mm2/include/linux/sched.h
@@ -996,10 +996,12 @@ struct task_struct {
 	struct futex_pi_state *pi_state_cache;
 
 	atomic_t fs_excl;	/* holding fs exclusive resources */
 	struct rcu_head rcu;
 
+	struct raw_notifier_head notify; /* generic per-task notifications */
+
 	/*
 	 * cache last used pipe for splice
 	 */
 	struct pipe_inode_info *splice_pipe;
 #ifdef	CONFIG_TASK_DELAY_ACCT
Index: linux-2.6.17-rc6-mm2/include/linux/init_task.h
===================================================================
--- linux-2.6.17-rc6-mm2.orig/include/linux/init_task.h
+++ linux-2.6.17-rc6-mm2/include/linux/init_task.h
@@ -3,10 +3,11 @@
 
 #include <linux/file.h>
 #include <linux/rcupdate.h>
 #include <linux/utsname.h>
 #include <linux/interrupt.h>
+#include <linux/notifier.h>
 
 #define INIT_FDTABLE \
 {							\
 	.max_fds	= NR_OPEN_DEFAULT, 		\
 	.max_fdset	= EMBEDDED_FD_SET_SIZE,		\
@@ -134,10 +135,11 @@ extern struct group_info init_groups;
 	.alloc_lock	= __SPIN_LOCK_UNLOCKED(tsk.alloc_lock),		\
 	.journal_info	= NULL,						\
 	.cpu_timers	= INIT_CPU_TIMERS(tsk.cpu_timers),		\
 	.fs_excl	= ATOMIC_INIT(0),				\
 	.pi_lock	= SPIN_LOCK_UNLOCKED,				\
+	.notify		= RAW_NOTIFIER_INIT(tsk.notify),		\
  	INIT_TRACE_IRQFLAGS						\
  	INIT_LOCKDEP							\
 }
 

Index: linux-2.6.17-rc6-mm2/kernel/sys.c
===================================================================
--- linux-2.6.17-rc6-mm2.orig/kernel/sys.c
+++ linux-2.6.17-rc6-mm2/kernel/sys.c
@@ -450,13 +450,41 @@ int unregister_task_watcher(struct notif
 	return blocking_notifier_chain_unregister(&task_watchers, nb);
 }
 
 EXPORT_SYMBOL_GPL(unregister_task_watcher);
 
+static inline int notify_per_task_watchers(unsigned int val,
+					   struct task_struct *task)
+{
+	if (get_watch_event(val) != WATCH_TASK_INIT)
+		return raw_notifier_call_chain(&task->notify, val, task);
+	RAW_INIT_NOTIFIER_HEAD(&task->notify);
+	if (task->real_parent)
+		return raw_notifier_call_chain(&task->real_parent->notify,
+		   			       val, task);
+}
+
+int register_per_task_watcher(struct notifier_block *nb)
+{
+	return raw_notifier_chain_register(&current->notify, nb);
+}
+EXPORT_SYMBOL_GPL(register_per_task_watcher);
+
+int unregister_per_task_watcher(struct notifier_block *nb)
+{
+	return raw_notifier_chain_unregister(&current->notify, nb);
+}
+EXPORT_SYMBOL_GPL(unregister_per_task_watcher);
+
 int notify_watchers(unsigned long val, void *v)
 {
-	return blocking_notifier_call_chain(&task_watchers, val, v);
+	int retval;
+
+	retval = blocking_notifier_call_chain(&task_watchers, val, v);
+	if (retval & NOTIFY_STOP_MASK)
+		return retval;
+	return notify_per_task_watchers(val, v);
 }
 

 static int set_one_prio(struct task_struct *p, int niceval, int error)
 {
Index: linux-2.6.17-rc6-mm2/include/linux/notifier.h
===================================================================
--- linux-2.6.17-rc6-mm2.orig/include/linux/notifier.h
+++ linux-2.6.17-rc6-mm2/include/linux/notifier.h
@@ -154,10 +154,12 @@ extern int raw_notifier_call_chain(struc
 #define CPU_DOWN_FAILED		0x0006 /* CPU (unsigned)v NOT going down */
 #define CPU_DEAD		0x0007 /* CPU (unsigned)v dead */
 
 extern int register_task_watcher(struct notifier_block *nb);
 extern int unregister_task_watcher(struct notifier_block *nb);
+extern int register_per_task_watcher(struct notifier_block *nb);
+extern int unregister_per_task_watcher(struct notifier_block *nb);
 #define WATCH_FLAGS_MASK		((-1) ^ 0x0FFFFUL)
 #define get_watch_event(v)		({ ((v) & ~WATCH_FLAGS_MASK); })
 #define get_watch_flags(v) 		({ ((v) & WATCH_FLAGS_MASK); })
 
 #define WATCH_TASK_INIT			0x00000001 /* initialize task_struct */

--

This patch simplifies the process events code by factoring many of the common
pieces into the task watcher notifier function. This factoring was enabled
by switching to task watchers instead of calling process events functions
directly.

Signed-off-by: Matt Helsley <matthltc@...>
Cc: Guillaume Thouvenin <guillaume.thouvenin@...>
--

 drivers/connector/cn_proc.c |  113 +++++++++++++-------------------------------
 1 files changed, 34 insertions(+), 79 deletions(-)

Index: linux-2.6.17-rc6-mm2/drivers/connector/cn_proc.c
===================================================================
--- linux-2.6.17-rc6-mm2.orig/drivers/connector/cn_proc.c
+++ linux-2.6.17-rc6-mm2/drivers/connector/cn_proc.c
@@ -45,113 +45,52 @@ static inline void get_seq(__u32 *ts, in
 	*ts = get_cpu_var(proc_event_counts)++;
 	*cpu = smp_processor_id();
 	put_cpu_var(proc_event_counts);
 }
 
-static void proc_fork_connector(struct task_struct *task)
+static inline void proc_fork_connector(struct task_struct *task,
+				       struct proc_event *ev)
 {
-	struct cn_msg *msg;
-	struct proc_event *ev;
-	__u8 buffer[CN_PROC_MSG_SIZE];
-
-	if (atomic_read(&proc_event_num_listeners) < 1)
-		return;
-
-	msg = (struct cn_msg*)buffer;
-	ev = (struct proc_event*)msg->data;
-	get_seq(&msg->seq, &ev->cpu);
-	ktime_get_ts(&ev->timestamp); /* get high res monotonic timestamp */
 	ev->what = PROC_EVENT_FORK;
 	ev->event_data.fork.parent_pid = task->real_parent->pid;
 	ev->event_data.fork.parent_tgid = task->real_parent->tgid;
 	ev->event_data.fork.child_pid = task->pid;
 	ev->event_data.fork.child_tgid = task->tgid;
-
-	memcpy(&msg->id, &cn_proc_event_id, sizeof(msg->id));
-	msg->ack = 0; /* not used */
-	msg->len = sizeof(*ev);
-	/*  If cn_netlink_send() failed, the data is not sent */
-	cn_netlink_send(msg, CN_IDX_PROC, GFP_ATOMIC);
 }
 
-static void proc_exec_connector(struct task_struct *task)
+static inline void proc_exec_connector(struct task_struct *task,
+				       struct proc_event *ev)
 {
-	struct cn_msg *msg;
-	struct proc_event *ev;
-	__u8 buffer[CN_PROC_MSG_SIZE];
-
-	if (atomic_read(&proc_event_num_listeners) < 1)
-		return;
-
-	msg = (struct cn_msg*)buffer;
-	ev = (struct proc_event*)msg->data;
-	get_seq(&msg->seq, &ev->cpu);
-	ktime_get_ts(&ev->timestamp);
 	ev->what = PROC_EVENT_EXEC;
 	ev->event_data.exec.process_pid = task->pid;
 	ev->event_data.exec.process_tgid = task->tgid;
-
-	memcpy(&msg->id, &cn_proc_event_id, sizeof(msg->id));
-	msg->ack = 0; /* not used */
-	msg->len = sizeof(*ev);
-	cn_netlink_send(msg, CN_IDX_PROC, GFP_ATOMIC);
 }
 
-static void proc_id_connector(struct task_struct *task, int which_id)
+static inline void proc_id_connector(struct task_struct *task, int which_id,
+		       		     struct proc_event *ev)
 {
-	struct cn_msg *msg;
-	struct proc_event *ev;
-	__u8 buffer[CN_PROC_MSG_SIZE];
-
-	if (atomic_read(&proc_event_num_listeners) < 1)
-		return;
-
-	msg = (struct cn_msg*)buffer;
-	ev = (struct proc_event*)msg->data;
 	ev->what = which_id;
 	ev->event_data.id.process_pid = task->pid;
 	ev->event_data.id.process_tgid = task->tgid;
 	if (which_id == PROC_EVENT_UID) {
 	 	ev->event_data.id.r.ruid = task->uid;
 	 	ev->event_data.id.e.euid = task->euid;
 	} else if (which_id == PROC_EVENT_GID) {
 	   	ev->event_data.id.r.rgid = task->gid;
 	   	ev->event_data.id.e.egid = task->egid;
-	} else
-	     	return;
-	get_seq(&msg->seq, &ev->cpu);
-	ktime_get_ts(&ev->timestamp);
-
-	memcpy(&msg->id, &cn_proc_event_id, sizeof(msg->id));
-	msg->ack = 0; /* not used */
-	msg->len = sizeof(*ev);
-	cn_netlink_send(msg, CN_IDX_PROC, GFP_ATOMIC);
+	}
+	WARN_ON((which_id != PROC_EVENT_UID) && (which_id != PROC_EVENT_GID));
 }
 
-static void proc_exit_connector(struct task_struct *task)
+static inline void proc_exit_connector(struct task_struct *task,
+				       struct proc_event *ev)
 {
-	struct cn_msg *msg;
-	struct proc_event *ev;
-	__u8 buffer[CN_PROC_MSG_SIZE];
-
-	if (atomic_read(&proc_event_num_listeners) < 1)
-		return;
-
-	msg = (struct cn_msg*)buffer;
-	ev = (struct proc_event*)msg->data;
-	get_seq(&msg->seq, &ev->cpu);
-	ktime_get_ts(&ev->timestamp);
 	ev->what = PROC_EVENT_EXIT;
 	ev->event_data.exit.process_pid = task->pid;
 	ev->event_data.exit.process_tgid = task->tgid;
 	ev->event_data.exit.exit_code = task->exit_code;
 	ev->event_data.exit.exit_signal = task->exit_signal;
-
-	memcpy(&msg->id, &cn_proc_event_id, sizeof(msg->id));
-	msg->ack = 0; /* not used */
-	msg->len = sizeof(*ev);
-	cn_netlink_send(msg, CN_IDX_PROC, GFP_ATOMIC);
 }
 
 /*
  * Send an acknowledgement message to userspace
  *
@@ -217,33 +156,49 @@ static void cn_proc_mcast_ctl(void *data
  */
 static int cn_proc_watch_task(struct notifier_block *nb, unsigned long val,
 			      void *t)
 {
 	struct task_struct *task = t;
+	struct cn_msg *msg;
+	struct proc_event *ev;
+	__u8 buffer[CN_PROC_MSG_SIZE];
 	int rc = NOTIFY_OK;
 
+	if (atomic_read(&proc_event_num_listeners) < 1)
+		return rc;
+
+	msg = (struct cn_msg*)buffer;
+	ev = (struct proc_event*)msg->data;
 	switch (get_watch_event(val)) {
 	case WATCH_TASK_CLONE:
-		proc_fork_connector(task);
+		proc_fork_connector(task, ev);
 		break;
 	case WATCH_TASK_EXEC:
-		proc_exec_connector(task);
+		proc_exec_connector(task, ev);
 		break;
 	case WATCH_TASK_UID:
-		proc_id_connector(task, PROC_EVENT_UID);
+		proc_id_connector(task, PROC_EVENT_UID, ev);
 		break;
 	case WATCH_TASK_GID:
-		proc_id_connector(task, PROC_EVENT_GID);
+		proc_id_connector(task, PROC_EVENT_GID, ev);
 		break;
 	case WATCH_TASK_EXIT:
-		proc_exit_connector(task);
+		proc_exit_connector(task, ev);
 		break;
-	default: /* we don't care about WATCH_TASK_INIT|FREE because we
-		    don't keep per-task info */
-		rc = NOTIFY_DONE; /* ignore all other notifications */
+	default: /* ignore WATCH_TASK_INIT|FREE - we don't keep per-task info */
+		rc = NOTIFY_DONE;
 		break;
 	}
+	if (rc != NOTIFY_OK)
+		return rc;
+	get_seq(&msg->seq, &ev->cpu);
+	ktime_get_ts(&ev->timestamp); /* get high res monotonic timestamp */
+	memcpy(&msg->id, &cn_proc_event_id, sizeof(msg->id));
+	msg->ack = 0; /* not used */
+	msg->len = sizeof(*ev);
+	cn_netlink_send(msg, CN_IDX_PROC, GFP_ATOMIC);
+	/* If cn_netlink_send() fails, drop data */
 	return rc;
 }
 
 static struct notifier_block __read_mostly cn_proc_nb = {
 	.notifier_call = cn_proc_watch_task,

--

This patch uses the task_watcher notifier chain to invoke exit_sem() at
appropriate times.

Signed-off-by: Matt Helsley <matthltc@...>
Cc: Jes Sorensen <jes@...>
--

 ipc/sem.c     |   23 +++++++++++++++++++++++
 kernel/exit.c |    1 -
 kernel/fork.c |    4 +---
 3 files changed, 24 insertions(+), 4 deletions(-)

Index: linux-2.6.17-rc6-mm2/ipc/sem.c
===================================================================
--- linux-2.6.17-rc6-mm2.orig/ipc/sem.c
+++ linux-2.6.17-rc6-mm2/ipc/sem.c
@@ -82,10 +82,11 @@
 #include <linux/audit.h>
 #include <linux/capability.h>
 #include <linux/seq_file.h>
 #include <linux/mutex.h>
 #include <linux/nsproxy.h>
+#include <linux/notifier.h>
 
 #include <asm/uaccess.h>
 #include "util.h"
 
 #define sem_ids(ns)	(*((ns)->ids[IPC_SEM_IDS]))
@@ -121,10 +122,31 @@ static int sysvipc_sem_proc_show(struct 
 #define sc_semmsl	sem_ctls[0]
 #define sc_semmns	sem_ctls[1]
 #define sc_semopm	sem_ctls[2]
 #define sc_semmni	sem_ctls[3]
 
+static int sem_undo_task_exit(struct notifier_block *nb, unsigned long val,
+			      void *t)
+{
+	switch(get_watch_event(val)) {
+	/*
+	 * If it weren't for the fact that we need clone flags to call
+	 * it we could also implement the copy_semundo portion of
+	 * copy_process inside case WATCH_TASK_INIT
+	 */
+	case WATCH_TASK_FREE:
+		exit_sem(t);
+		return NOTIFY_OK;
+	default: /* Don't care */
+		return NOTIFY_DONE;
+	}
+}
+
+static struct notifier_block sem_watch_tasks_nb = {
+	.notifier_call = sem_undo_task_exit
+};
+
 static void __ipc_init __sem_init_ns(struct ipc_namespace *ns, struct ipc_ids *ids)
 {
 	ns->ids[IPC_SEM_IDS] = ids;
 	ns->sc_semmsl = SEMMSL;
 	ns->sc_semmns = SEMMNS;
@@ -171,10 +193,11 @@ void __init sem_init (void)
 {
 	__sem_init_ns(&init_ipc_ns, &init_sem_ids);
 	ipc_init_proc_interface("sysvipc/sem",
 				"       key      semid perms      nsems   uid   gid  cuid  cgid      otime      ctime\n",
 				IPC_SEM_IDS, sysvipc_sem_proc_show);
+	register_task_watcher(&sem_watch_tasks_nb);
 }
 
 /*
  * Lockless wakeup algorithm:
  * Without the check/retry algorithm a lockless wakeup is possible:
Index: linux-2.6.17-rc6-mm2/kernel/exit.c
===================================================================
--- linux-2.6.17-rc6-mm2.orig/kernel/exit.c
+++ linux-2.6.17-rc6-mm2/kernel/exit.c
@@ -919,11 +919,10 @@ fastcall NORET_TYPE void do_exit(long co
 	notify_result = notify_watchers(WATCH_TASK_FREE, tsk);
 	WARN_ON(notify_result & NOTIFY_STOP_MASK);
 
 	exit_mm(tsk);
 
-	exit_sem(tsk);
 	__exit_files(tsk);
 	__exit_fs(tsk);
 	exit_task_namespaces(tsk);
 	exit_thread();
 	cpuset_exit(tsk);
Index: linux-2.6.17-rc6-mm2/kernel/fork.c
===================================================================
--- linux-2.6.17-rc6-mm2.orig/kernel/fork.c
+++ linux-2.6.17-rc6-mm2/kernel/fork.c
@@ -1089,11 +1089,11 @@ static task_t *copy_process(unsigned lon
 		goto bad_fork_cleanup_policy;
 	/* copy all the process information */
 	if ((retval = copy_semundo(clone_flags, p)))
 		goto bad_fork_cleanup_security;
 	if ((retval = copy_files(clone_flags, p)))
-		goto bad_fork_cleanup_semundo;
+		goto bad_fork_cleanup_security;
 	if ((retval = copy_fs(clone_flags, p)))
 		goto bad_fork_cleanup_files;
 	if ((retval = copy_sighand(clone_flags, p)))
 		goto bad_fork_cleanup_fs;
 	if ((retval = copy_signal(clone_flags, p)))
@@ -1263,12 +1263,10 @@ bad_fork_cleanup_sighand:
 	__cleanup_sighand(p->sighand);
 bad_fork_cleanup_fs:
 	exit_fs(p); /* blocking */
 bad_fork_cleanup_files:
 	exit_files(p); /* blocking */
-bad_fork_cleanup_semundo:
-	exit_sem(p);
 bad_fork_cleanup_security:
 	security_task_free(p);
 	notify_result = notify_watchers(WATCH_TASK_FREE, p);
 	WARN_ON(notify_result & NOTIFY_STOP_MASK);
 bad_fork_cleanup_policy:

--

This patch switches semundo from using the global task_watchers notifier chain
to a per-task notifier chain. In the case where a task does not use SysV
semaphores this would save a call to exit_sem().

Based off Jes Sorensen's patch implementing this with task_notifiers.

Signed-off-by: Matt Helsley <matthltc@...>
Cc: Jes Sorensen <jes@...>
--

 ipc/sem.c |   23 ++++++++++++++++-------
 1 files changed, 16 insertions(+), 7 deletions(-)

Index: linux-2.6.17-rc6-mm2/ipc/sem.c
===================================================================
--- linux-2.6.17-rc6-mm2.orig/ipc/sem.c
+++ linux-2.6.17-rc6-mm2/ipc/sem.c
@@ -139,14 +139,10 @@ static int sem_undo_task_exit(struct not
 	default: /* Don't care */
 		return NOTIFY_DONE;
 	}
 }
 
-static struct notifier_block sem_watch_tasks_nb = {
-	.notifier_call = sem_undo_task_exit
-};
-
 static void __ipc_init __sem_init_ns(struct ipc_namespace *ns, struct ipc_ids *ids)
 {
 	ns->ids[IPC_SEM_IDS] = ids;
 	ns->sc_semmsl = SEMMSL;
 	ns->sc_semmns = SEMMNS;
@@ -193,11 +189,10 @@ void __init sem_init (void)
 {
 	__sem_init_ns(&init_ipc_ns, &init_sem_ids);
 	ipc_init_proc_interface("sysvipc/sem",
 				"       key      semid perms      nsems   uid   gid  cuid  cgid      otime      ctime\n",
 				IPC_SEM_IDS, sysvipc_sem_proc_show);
-	register_task_watcher(&sem_watch_tasks_nb);
 }
 
 /*
  * Lockless wakeup algorithm:
  * Without the check/retry algorithm a lockless wakeup is possible:
@@ -1010,11 +1005,10 @@ static inline void unlock_semundo(void)
 	undo_list = current->sysvsem.undo_list;
 	if (undo_list)
 		spin_unlock(&undo_list->lock);
 }
 
-
 /* If the task doesn't already have a undo_list, then allocate one
  * here.  We guarantee there is only one thread using this undo list,
  * and current is THE ONE
  *
  * If this allocation and assignment succeeds, but later
@@ -1026,24 +1020,39 @@ static inline void unlock_semundo(void)
  */
 static inline int get_undo_list(struct sem_undo_list **undo_listp)
 {
 	struct sem_undo_list *undo_list;
 	int size;
+	struct notifier_block *semun_nb;
+	int retval;
 
 	undo_list = current->sysvsem.undo_list;
 	if (!undo_list) {
+		semun_nb = NULL;
+		retval = -ENOMEM;
 		size = sizeof(struct sem_undo_list);
 		undo_list = (struct sem_undo_list *) kmalloc(size, GFP_KERNEL);
 		if (undo_list == NULL)
-			return -ENOMEM;
+			goto err;
+		semun_nb = kzalloc(sizeof(*semun_nb), GFP_KERNEL);
+		if (semun_nb == NULL)
+			goto err;
+		semun_nb->notifier_call = sem_undo_task_exit;
+		retval = register_per_task_watcher(semun_nb);
+		if (retval)
+			goto err;
 		memset(undo_list, 0, size);
 		spin_lock_init(&undo_list->lock);
 		atomic_set(&undo_list->refcnt, 1);
 		current->sysvsem.undo_list = undo_list;
 	}
 	*undo_listp = undo_list;
 	return 0;
+err:
+	kfree(semun_nb);
+	kfree(undo_list);
+	return retval;
 }
 
 static struct sem_undo *lookup_undo(struct sem_undo_list *ulp, int semid)
 {
 	struct sem_undo **last, *un;

--

On Tuesday 13 June 2006 18:54, Matt Helsley wrote:

> +static void cn_proc_fini(void)
> +{
> +	int err;
> +
> +	err = unregister_task_watcher(&cn_proc_nb);
> +	if (err != 0)
> +		printk(KERN_WARNING
> +		       "cn_proc failed to unregister its task notify block\n");

How about if (err), or if (unregister_task_watcher(&cn_proc_nb))?

> +	cn_del_callback(&cn_proc_event_id);
> +}
> +
>  module_init(cn_proc_init);
> +module_exit(cn_proc_fini);

Thanks,
Chase

On Tuesday 13 June 2006 18:54, Matt Helsley wrote:

>  	switch (type) {
> -		case PROFILE_TASK_EXIT:
> -			err = blocking_notifier_chain_register(
> -					&task_exit_notifier, n);
> -			break;
>  		case PROFILE_MUNMAP:
>  			err = blocking_notifier_chain_register(
>  					&munmap_notifier, n);
>  			break;
>  	}

	if (type == PROFILE_MUNMAP)

?

> @@ -140,14 +130,10 @@ int profile_event_register(enum profile_
>  int profile_event_unregister(enum profile_type type, struct notifier_block
> * n) {
>  	int err = -EINVAL;
>
>  	switch (type) {
> -		case PROFILE_TASK_EXIT:
> -			err = blocking_notifier_chain_unregister(
> -					&task_exit_notifier, n);
> -			break;
>  		case PROFILE_MUNMAP:
>  			err = blocking_notifier_chain_unregister(
>  					&munmap_notifier, n);
>  			break;
>  	}

Same...

Thanks,
Chase

On Tue, 2006-06-13 at 19:39 -0500, Chase Venters wrote:
> On Tuesday 13 June 2006 18:54, Matt Helsley wrote:
> 
> > +static int cn_proc_watch_task(struct notifier_block *nb, unsigned long
> > val, +			      void *t)
> > +{
> > +	struct task_struct *task = t;
> 
> Why the copy?

	It shouldn't result in a copy. Since t is a void* I don't think any
additional instructions or stack space are required. This should have
zero runtime cost while improving clarity of the code. It's also needed
in the next patch.

> > +	int rc = NOTIFY_OK;
> > +
> > +	switch (get_watch_event(val)) {
> > +	case WATCH_TASK_CLONE:
> > +		proc_fork_connector(task);
> > +		break;
> > +	case WATCH_TASK_EXEC:
> > +		proc_exec_connector(task);
> > +		break;
> > +	case WATCH_TASK_UID:
> > +		proc_id_connector(task, PROC_EVENT_UID);
> > +		break;
> > +	case WATCH_TASK_GID:
> > +		proc_id_connector(task, PROC_EVENT_GID);
> > +		break;
> > +	case WATCH_TASK_EXIT:
> > +		proc_exit_connector(task);
> > +		break;
> > +	default: /* we don't care about WATCH_TASK_INIT|FREE because we
> > +		    don't keep per-task info */
> > +		rc = NOTIFY_DONE; /* ignore all other notifications */
> > +		break;
> > +	}
> > +	return rc;
> > +}
> > +
> 
> >  /*
> >   * cn_proc_init - initialization entry point
> >   *
> >   * Adds the connector callback to the connector driver.
> >   */
> > @@ -219,11 +259,16 @@ static int __init cn_proc_init(void)
> >  	int err;
> >
> >  	if ((err = cn_add_callback(&cn_proc_event_id, "cn_proc",
> >  	 			   &cn_proc_mcast_ctl))) {
> >  		printk(KERN_WARNING "cn_proc failed to register\n");
> > -		return err;
> > +		goto out;
> >  	}
> > -	return 0;
> > +
> > +	err = register_task_watcher(&cn_proc_nb);
> > +	if (err != 0)
> 
> if (err)

I don't see any benefit to changing this. Care to elaborate on why this
is important?

> Thanks,
> Chase

On Tue, 2006-06-13 at 19:19 -0500, Chase Venters wrote:
> On Tuesday 13 June 2006 18:53, Matt Helsley wrote:
> 
> > @@ -847,12 +848,15 @@ static void exit_notify(struct task_stru
> >  fastcall NORET_TYPE void do_exit(long code)
> >  {
> >  	struct task_struct *tsk = current;
> >  	struct taskstats *tidstats, *tgidstats;
> >  	int group_dead;
> > +	int notify_result;
> >
> >  	profile_task_exit(tsk);
> > +	tsk->exit_code = code;
> > +	notify_result = notify_watchers(WATCH_TASK_EXIT, tsk);
> 
> Are you using this specific return value?

Nope. I was wary of compiler warnings. I'll try removing this first
assignment. However, the variable is necessary for another
notify_watchers() call later in the function. I introduced it because I
didn't think wrapping the function call like this:

WARN_ON(notify_watchers(WATCH_TASK_FREE, tsk) & NOTIFY_STOP_MASK);

would be very readable.

> > +int notify_watchers(unsigned long val, void *v)
> > +{
> > +	return atomic_notifier_call_chain(&task_watchers, val, v);
> > +}
> 
> Might this be called notify_task_watchers()?

Seems like a good idea. I'll make the necessary changes.

> Thanks,
> Chase

Cheers,
	-Matt Helsley

On Tue, 2006-06-13 at 19:43 -0500, Chase Venters wrote:
> On Tuesday 13 June 2006 18:54, Matt Helsley wrote:
> 
> > +	WARN_ON((which_id != PROC_EVENT_UID) && (which_id != PROC_EVENT_GID));
> >  }
> 
> How about WARN_ON(!(which_id & (PROC_EVENT_UID | PROC_EVENT_GID))) to save a 
> cmp?
> 
> Thanks,
> Chase

I think the compiler is capable of making such optimizations. I also
think what I have now is clearer to anyone skimming the code.

Cheers,
	-Matt Helsley

On Tue, 2006-06-13 at 19:59 -0500, Chase Venters wrote:
> On Tuesday 13 June 2006 18:54, Matt Helsley wrote:
> 
> >  	switch (type) {
> > -		case PROFILE_TASK_EXIT:
> > -			err = blocking_notifier_chain_register(
> > -					&task_exit_notifier, n);
> > -			break;
> >  		case PROFILE_MUNMAP:
> >  			err = blocking_notifier_chain_register(
> >  					&munmap_notifier, n);
> >  			break;
> >  	}
> 
> 	if (type == PROFILE_MUNMAP)
> 
> ?
>
> > @@ -140,14 +130,10 @@ int profile_event_register(enum profile_
> >  int profile_event_unregister(enum profile_type type, struct notifier_block
> > * n) {
> >  	int err = -EINVAL;
> >
> >  	switch (type) {
> > -		case PROFILE_TASK_EXIT:
> > -			err = blocking_notifier_chain_unregister(
> > -					&task_exit_notifier, n);
> > -			break;
> >  		case PROFILE_MUNMAP:
> >  			err = blocking_notifier_chain_unregister(
> >  					&munmap_notifier, n);
> >  			break;
> >  	}
> 
> Same...
> 
> Thanks,
> Chase

Hmm. Perhaps I ought to get rid of the condition and enum entirely then
change the names of the functions to profile_mmmap() and
profile_munmap().

	It really depends on what additional changes, if any, are expected
here. Since I don't have any plans to further modify profiling beyond
what I've outlined in these patches I'm not sure what the best course is
here.

Thanks,
	-Matt Helsley

On Tue, 2006-06-13 at 19:54 -0500, Chase Venters wrote:
> On Tuesday 13 June 2006 18:54, Matt Helsley wrote:
> 
> > +static void cn_proc_fini(void)
> > +{
> > +	int err;
> > +
> > +	err = unregister_task_watcher(&cn_proc_nb);
> > +	if (err != 0)
> > +		printk(KERN_WARNING
> > +		       "cn_proc failed to unregister its task notify block\n");
> 
> How about if (err), or if (unregister_task_watcher(&cn_proc_nb))?

I don't see any worthwhile benefit to the former and I've seen feedback
that the latter is less readable.

> > +	cn_del_callback(&cn_proc_event_id);
> > +}
> > +
> >  module_init(cn_proc_init);
> > +module_exit(cn_proc_fini);
> 
> Thanks,
> Chase

Matt Helsley wrote:

>Adapts delayacct to use Task Watchers. Does not adapt taskstats to use Task
>Watchers.
>
>Signed-off-by: Matt Helsley <matthltc@...>
>Cc: Shailabh Nagar <nagar@...>
>Cc: Balbir Singh <balbir@...>
>Cc: Chandra S. Seetharaman <sekharan@...>
>--
>
> include/linux/delayacct.h |    2 +-
> kernel/delayacct.c        |   23 +++++++++++++++++++++++
> kernel/exit.c             |    2 --
> kernel/fork.c             |    2 --
> 4 files changed, 24 insertions(+), 5 deletions(-)
>
>
>  
>
<snip>

>Index: linux-2.6.17-rc5-mm2/include/linux/delayacct.h
>===================================================================
>--- linux-2.6.17-rc5-mm2.orig/include/linux/delayacct.h
>+++ linux-2.6.17-rc5-mm2/include/linux/delayacct.h
>@@ -59,11 +59,11 @@ static inline void delayacct_tsk_init(st
> 		__delayacct_tsk_init(tsk);
> }
> 
> static inline void delayacct_tsk_exit(struct task_struct *tsk)
> {
>-	if (tsk->delays)
>+	if (unlikely(tsk->delays))
> 		__delayacct_tsk_exit(tsk);
> }
>  
>

This snippet does not belong to this patchset...since the same check 
(for tsk->delays) is
being used elsewhere in the delay accounting code, changes to use of 
likely/unlikely should
be done elsewhere too, if deemed necesssary.

Otherwise patch looks good though I've not tested whether delay 
accounting works unaffected
(don't see any reason why it shouldn't).


--Shailabh

On Tuesday 13 June 2006 20:11, Matt Helsley wrote:
> On Tue, 2006-06-13 at 19:43 -0500, Chase Venters wrote:
> > On Tuesday 13 June 2006 18:54, Matt Helsley wrote:
> > > +	WARN_ON((which_id != PROC_EVENT_UID) && (which_id !=
> > > PROC_EVENT_GID)); }
> >
> > How about WARN_ON(!(which_id & (PROC_EVENT_UID | PROC_EVENT_GID))) to
> > save a cmp?
> >
> > Thanks,
> > Chase
>
> I think the compiler is capable of making such optimizations. I also
> think what I have now is clearer to anyone skimming the code.

Can the compiler test that (which_id != PROC_EVENT_UID) && (which_id != 
PROC_EVENT_GID) merely by masking? Since they're bits, one mask testing both 
could technically match both (true result), which would not happen in the != 
case (false result). It is a small point though.

> Cheers,
> 	-Matt Helsley

Thanks,
Chase

On Tue, Jun 13, 2006 at 04:54:46PM -0700, Matt Helsley wrote:
> Adapt audit to use task watchers.

audit_free(p) really expects that either p is a stillborn (never ran)
*or* that p == current.

On Tue, 2006-06-13 at 23:31 -0400, Shailabh Nagar wrote:
> 
> 
> Matt Helsley wrote:
> 
> >Adapts delayacct to use Task Watchers. Does not adapt taskstats to use Task
> >Watchers.
> >
> >Signed-off-by: Matt Helsley <matthltc@...>
> >Cc: Shailabh Nagar <nagar@...>
> >Cc: Balbir Singh <balbir@...>
> >Cc: Chandra S. Seetharaman <sekharan@...>
> >--
> >
> > include/linux/delayacct.h |    2 +-
> > kernel/delayacct.c        |   23 +++++++++++++++++++++++
> > kernel/exit.c             |    2 --
> > kernel/fork.c             |    2 --
> > 4 files changed, 24 insertions(+), 5 deletions(-)
> >
> >
> >  
> >
> <snip>
> 
> >Index: linux-2.6.17-rc5-mm2/include/linux/delayacct.h
> >===================================================================
> >--- linux-2.6.17-rc5-mm2.orig/include/linux/delayacct.h
> >+++ linux-2.6.17-rc5-mm2/include/linux/delayacct.h
> >@@ -59,11 +59,11 @@ static inline void delayacct_tsk_init(st
> > 		__delayacct_tsk_init(tsk);
> > }
> > 
> > static inline void delayacct_tsk_exit(struct task_struct *tsk)
> > {
> >-	if (tsk->delays)
> >+	if (unlikely(tsk->delays))
> > 		__delayacct_tsk_exit(tsk);
> > }
> >  
> >
> 
> This snippet does not belong to this patchset...since the same check 
> (for tsk->delays) is
> being used elsewhere in the delay accounting code, changes to use of 
> likely/unlikely should
> be done elsewhere too, if deemed necesssary.

Good point. I'll remove this chunk of the patch.

<snip>

Cheers,
	-Matt Helsley

On Wed, 2006-06-14 at 10:46 -0400, Alexander Viro wrote:
> On Tue, Jun 13, 2006 at 04:54:46PM -0700, Matt Helsley wrote:
> > Adapt audit to use task watchers.
> 
> audit_free(p) really expects that either p is a stillborn (never ran)
> *or* that p == current.

	Makes sense. I think the task watcher patches are consistent with this.
I think the first patch of this series helps explain how this patch
remains consistent with the above. I should have cc'd linux-audit when
posting that patch -- here's a link for now:
http://www.ussg.iu.edu/hypermail/linux/kernel/0606.1/1800.html

	In copy_process() and do_exit() notify_watchers() passes the same
pointers as audit_alloc() and audit_free() used before. The patches also
do not introduce or remove calls to audit_alloc() or audit_free(). The
patches trigger these calls with notify_watchers() while passing
WATCH_TASK_INIT and WATCH_TASK_FREE for audit_alloc() and audit_free()
respectively. WATCH_TASK_INIT (and hence audit_alloc()) only happens in
copy_process(). WATCH_TASK_FREE (and hence audit_free()) happens in
copy_process()'s error recovery path and in do_exit().

	This results in the same calls to audit_alloc() and audit_free() except
with an additional function call preceding them on the stack.

	Are you concerned that future modifications of task watchers would pass
in task structs that violate these expectations? I can alter the patches
to incorporate these restrictions:

copy_process()
{
	...
	notify_watchers(WATCH_TASK_INIT, p);
	...
	if (<succeeding>)
		notify_watchers(WATCH_TASK_CLONE, p);
	...
bad_foo:
	...
- 	notify_watchers(WATCH_TASK_FREE, p);
+ 	notify_watchers(WATCH_TASK_ABORT, p);
	...
}

<change all other notify_watchers() invocations to pass NULL as
the second parameter, e.g.>

do_exit()
{
	...
	notify_watchers(WATCH_TSK_FREE, NULL); /* callees must use current */
}

However this requires that I modify each user of task watchers with
something like:

int foo (struct notifier_block *nb, unsigned long val, void *v)
{
-	struct task_struct *tsk = v;
+	struct task_struct *tsk = current;
	...
	switch(get_watch_event(val)) {
	case WATCH_TASK_INIT:
+ 		tsk = v; /* INIT and ABORT use v, the rest use current */
	...
+ 	case WATCH_TASK_ABORT:
+ 		tsk = v; /* fall through */
	case WATCH_TASK_FREE:
		...
	}
	...
}

Which seems a bit more complicated. Is this worth it?

Cheers,
	-Matt Helsley

Matt Helsley wrote:
> This introduces a second, per-task, blocking notifier chain. The per-task
> chain offers watchers the chance to register with a specific task nstead of
> all tasks. It also allows the watcher to associate a block of data with the task
> by wrapping the notifier block using containerof().
> 
> Both the global, all-tasks chain and the per-task chain are called from the samefunction. The two types of chains share the same set of notification
> values, however registration functions and the registered notifier blocks must
> be separate.
> 
> These notifiers are only safe if notifier blocks are registered with the current
> task while in the context of the current task. This ensures that there are no
> races between registration, unregistration, and notification.
> 
> Signed-off-by: Matt Helsley <matthltc@...>
> Cc: Jes Sorensen <jes@...>
> Cc: Chandra S. Seetharaman <sekharan@...>
> Cc: Alan Stern <stern@...>
[bits deleted]

> Index: linux-2.6.17-rc6-mm2/kernel/sys.c
> ===================================================================
> --- linux-2.6.17-rc6-mm2.orig/kernel/sys.c
> +++ linux-2.6.17-rc6-mm2/kernel/sys.c
> @@ -450,13 +450,41 @@ int unregister_task_watcher(struct notif
>  	return blocking_notifier_chain_unregister(&task_watchers, nb);
>  }
>  
>  EXPORT_SYMBOL_GPL(unregister_task_watcher);
>  
> +static inline int notify_per_task_watchers(unsigned int val,
> +					   struct task_struct *task)
> +{
> +	if (get_watch_event(val) != WATCH_TASK_INIT)
> +		return raw_notifier_call_chain(&task->notify, val, task);
> +	RAW_INIT_NOTIFIER_HEAD(&task->notify);
> +	if (task->real_parent)
> +		return raw_notifier_call_chain(&task->real_parent->notify,
> +		   			       val, task);
> +}

It's possible for this task to exit without returning a result.

Peter
-- 
Peter Williams                                   pwil3058@...

"Learning, n. The kind of ignorance distinguishing the studious."
  -- Ambrose Bierce

On Tue, 2006-06-20 at 15:28 +1000, Peter Williams wrote:
> Matt Helsley wrote:
> > This introduces a second, per-task, blocking notifier chain. The per-task
> > chain offers watchers the chance to register with a specific task nstead of
> > all tasks. It also allows the watcher to associate a block of data with the task
> > by wrapping the notifier block using containerof().
> > 
> > Both the global, all-tasks chain and the per-task chain are called from the samefunction. The two types of chains share the same set of notification
> > values, however registration functions and the registered notifier blocks must
> > be separate.
> > 
> > These notifiers are only safe if notifier blocks are registered with the current
> > task while in the context of the current task. This ensures that there are no
> > races between registration, unregistration, and notification.
> > 
> > Signed-off-by: Matt Helsley <matthltc@...>
> > Cc: Jes Sorensen <jes@...>
> > Cc: Chandra S. Seetharaman <sekharan@...>
> > Cc: Alan Stern <stern@...>
> [bits deleted]
> 
> > Index: linux-2.6.17-rc6-mm2/kernel/sys.c
> > ===================================================================
> > --- linux-2.6.17-rc6-mm2.orig/kernel/sys.c
> > +++ linux-2.6.17-rc6-mm2/kernel/sys.c
> > @@ -450,13 +450,41 @@ int unregister_task_watcher(struct notif
> >  	return blocking_notifier_chain_unregister(&task_watchers, nb);
> >  }
> >  
> >  EXPORT_SYMBOL_GPL(unregister_task_watcher);
> >  
> > +static inline int notify_per_task_watchers(unsigned int val,
> > +					   struct task_struct *task)
> > +{
> > +	if (get_watch_event(val) != WATCH_TASK_INIT)
> > +		return raw_notifier_call_chain(&task->notify, val, task);
> > +	RAW_INIT_NOTIFIER_HEAD(&task->notify);
> > +	if (task->real_parent)
> > +		return raw_notifier_call_chain(&task->real_parent->notify,
> > +		   			       val, task);
> > +}
> 
> It's possible for this task to exit without returning a result.

Assuming you meant s/task/function/:

	In the common case this will return a result because most tasks have a
real parent. The only exception should be the init task. However, the
init task does not "fork" from another task so this function will never
get called with WATCH_TASK_INIT and the init task.

	This means that if one wants to use per-task watchers to associate data
and a function call with *every* task, special care will need to be
taken to register with the init task.

> Peter

Matt Helsley <matthltc@...> wrote:
>
> > > +static inline int notify_per_task_watchers(unsigned int val,
> > > +					   struct task_struct *task)
> > > +{
> > > +	if (get_watch_event(val) != WATCH_TASK_INIT)
> > > +		return raw_notifier_call_chain(&task->notify, val, task);
> > > +	RAW_INIT_NOTIFIER_HEAD(&task->notify);
> > > +	if (task->real_parent)
> > > +		return raw_notifier_call_chain(&task->real_parent->notify,
> > > +		   			       val, task);
> > > +}
> > 
> > It's possible for this task to exit without returning a result.
> 
> Assuming you meant s/task/function/:
> 
> 	In the common case this will return a result because most tasks have a
> real parent. The only exception should be the init task. However, the
> init task does not "fork" from another task so this function will never
> get called with WATCH_TASK_INIT and the init task.
> 
> 	This means that if one wants to use per-task watchers to associate data
> and a function call with *every* task, special care will need to be
> taken to register with the init task.

no......

It's possible for this function to fall off the end without returning
anything.  The compiler should have spat a warning.

Matt Helsley wrote:
> On Tue, 2006-06-20 at 15:28 +1000, Peter Williams wrote:
>> Matt Helsley wrote:
>>> This introduces a second, per-task, blocking notifier chain. The per-task
>>> chain offers watchers the chance to register with a specific task nstead of
>>> all tasks. It also allows the watcher to associate a block of data with the task
>>> by wrapping the notifier block using containerof().
>>>
>>> Both the global, all-tasks chain and the per-task chain are called from the samefunction. The two types of chains share the same set of notification
>>> values, however registration functions and the registered notifier blocks must
>>> be separate.
>>>
>>> These notifiers are only safe if notifier blocks are registered with the current
>>> task while in the context of the current task. This ensures that there are no
>>> races between registration, unregistration, and notification.
>>>
>>> Signed-off-by: Matt Helsley <matthltc@...>
>>> Cc: Jes Sorensen <jes@...>
>>> Cc: Chandra S. Seetharaman <sekharan@...>
>>> Cc: Alan Stern <stern@...>
>> [bits deleted]
>>
>>> Index: linux-2.6.17-rc6-mm2/kernel/sys.c
>>> ===================================================================
>>> --- linux-2.6.17-rc6-mm2.orig/kernel/sys.c
>>> +++ linux-2.6.17-rc6-mm2/kernel/sys.c
>>> @@ -450,13 +450,41 @@ int unregister_task_watcher(struct notif
>>>  	return blocking_notifier_chain_unregister(&task_watchers, nb);
>>>  }
>>>  
>>>  EXPORT_SYMBOL_GPL(unregister_task_watcher);
>>>  
>>> +static inline int notify_per_task_watchers(unsigned int val,
>>> +					   struct task_struct *task)
>>> +{
>>> +	if (get_watch_event(val) != WATCH_TASK_INIT)
>>> +		return raw_notifier_call_chain(&task->notify, val, task);
>>> +	RAW_INIT_NOTIFIER_HEAD(&task->notify);
>>> +	if (task->real_parent)
>>> +		return raw_notifier_call_chain(&task->real_parent->notify,
>>> +		   			       val, task);
>>> +}
>> It's possible for this task to exit without returning a result.
> 
> Assuming you meant s/task/function/:

Yes, sorry.

> 
> 	In the common case this will return a result because most tasks have a
> real parent. The only exception should be the init task. However, the
> init task does not "fork" from another task so this function will never
> get called with WATCH_TASK_INIT and the init task.

OK.  But it causes a compiler warning:

/home/peterw/KERNELS/CpuCaps/TW-2.6.17-rc6-mm2/kernel/sys.c: In function 
‘notify_per_task_watchers’:
/home/peterw/KERNELS/CpuCaps/TW-2.6.17-rc6-mm2/kernel/sys.c:464: 
warning: control reaches end of non-void function

> 
> 	This means that if one wants to use per-task watchers to associate data
> and a function call with *every* task, special care will need to be
> taken to register with the init task.

OK.  I think I can safely ignore init.

Peter
-- 
Peter Williams                                   pwil3058@...

"Learning, n. The kind of ignorance distinguishing the studious."
  -- Ambrose Bierce

Andrew Morton wrote:
> Matt Helsley <matthltc@...> wrote:
>>>> +static inline int notify_per_task_watchers(unsigned int val,
>>>> +					   struct task_struct *task)
>>>> +{
>>>> +	if (get_watch_event(val) != WATCH_TASK_INIT)
>>>> +		return raw_notifier_call_chain(&task->notify, val, task);
>>>> +	RAW_INIT_NOTIFIER_HEAD(&task->notify);
>>>> +	if (task->real_parent)
>>>> +		return raw_notifier_call_chain(&task->real_parent->notify,
>>>> +		   			       val, task);
>>>> +}
>>> It's possible for this task to exit without returning a result.
>> Assuming you meant s/task/function/:
>>
>> 	In the common case this will return a result because most tasks have a
>> real parent. The only exception should be the init task. However, the
>> init task does not "fork" from another task so this function will never
>> get called with WATCH_TASK_INIT and the init task.
>>
>> 	This means that if one wants to use per-task watchers to associate data
>> and a function call with *every* task, special care will need to be
>> taken to register with the init task.
> 
> no......
> 
> It's possible for this function to fall off the end without returning
> anything.  The compiler should have spat a warning.

I checked and it does.

-- 
Peter Williams                                   pwil3058@...

"Learning, n. The kind of ignorance distinguishing the studious."
  -- Ambrose Bierce

On Tue, 2006-06-20 at 16:15 -0700, Andrew Morton wrote:
> Matt Helsley <matthltc@...> wrote:
> >
> > > > +static inline int notify_per_task_watchers(unsigned int val,
> > > > +					   struct task_struct *task)
> > > > +{
> > > > +	if (get_watch_event(val) != WATCH_TASK_INIT)
> > > > +		return raw_notifier_call_chain(&task->notify, val, task);
> > > > +	RAW_INIT_NOTIFIER_HEAD(&task->notify);
> > > > +	if (task->real_parent)
> > > > +		return raw_notifier_call_chain(&task->real_parent->notify,
> > > > +		   			       val, task);
> > > > +}
> > > 
> > > It's possible for this task to exit without returning a result.
> > 
> > Assuming you meant s/task/function/:
> > 
> > 	In the common case this will return a result because most tasks have a
> > real parent. The only exception should be the init task. However, the
> > init task does not "fork" from another task so this function will never
> > get called with WATCH_TASK_INIT and the init task.
> > 
> > 	This means that if one wants to use per-task watchers to associate data
> > and a function call with *every* task, special care will need to be
> > taken to register with the init task.
> 
> no......

	I've been looking through the source and, from what I can see, the end
of the function is not reachable. I think I need to add:

notify_watchers(WATCH_TASK_INIT, &init_task);

to make this into an applicable warning.

> It's possible for this function to fall off the end without returning
> anything.  The compiler should have spat a warning.

	I'll add a return value at the end of the function as well as the above
notification to keep things uniform and address the compiler warning.

	Incidentally, I've looked at my compilation logs and I did not get any
warnings (gcc version 3.3.4 (Debian 1:3.3.4-3)).

Cheers,
	-Matt Helsley

On Tue, 20 Jun 2006 18:20:48 -0700
Matt Helsley <matthltc@...> wrote:

> > > > > +static inline int notify_per_task_watchers(unsigned int val,
> > > > > +					   struct task_struct *task)
> > > > > +{
> > > > > +	if (get_watch_event(val) != WATCH_TASK_INIT)
> > > > > +		return raw_notifier_call_chain(&task->notify, val, task);
> > > > > +	RAW_INIT_NOTIFIER_HEAD(&task->notify);
> > > > > +	if (task->real_parent)
> > > > > +		return raw_notifier_call_chain(&task->real_parent->notify,
> > > > > +		   			       val, task);
> > > > > +}
> > > > 
> > > > It's possible for this task to exit without returning a result.
> > > 
> > > Assuming you meant s/task/function/:
> > > 
> > > 	In the common case this will return a result because most tasks have a
> > > real parent. The only exception should be the init task. However, the
> > > init task does not "fork" from another task so this function will never
> > > get called with WATCH_TASK_INIT and the init task.
> > > 
> > > 	This means that if one wants to use per-task watchers to associate data
> > > and a function call with *every* task, special care will need to be
> > > taken to register with the init task.
> > 
> > no......
> 
> 	I've been looking through the source and, from what I can see, the end
> of the function is not reachable. I think I need to add:
> 
> notify_watchers(WATCH_TASK_INIT, &init_task);
> 
> to make this into an applicable warning.

If the end of the function isn't reachable then the
`if (task->real_parent)' can simply be removed.

Andrew Morton wrote:
> On Tue, 20 Jun 2006 18:20:48 -0700
> Matt Helsley <matthltc@...> wrote:
> 
>>>>>> +static inline int notify_per_task_watchers(unsigned int val,
>>>>>> +					   struct task_struct *task)
>>>>>> +{
>>>>>> +	if (get_watch_event(val) != WATCH_TASK_INIT)
>>>>>> +		return raw_notifier_call_chain(&task->notify, val, task);
>>>>>> +	RAW_INIT_NOTIFIER_HEAD(&task->notify);
>>>>>> +	if (task->real_parent)
>>>>>> +		return raw_notifier_call_chain(&task->real_parent->notify,
>>>>>> +		   			       val, task);
>>>>>> +}
>>>>> It's possible for this task to exit without returning a result.
>>>> Assuming you meant s/task/function/:
>>>>
>>>> 	In the common case this will return a result because most tasks have a
>>>> real parent. The only exception should be the init task. However, the
>>>> init task does not "fork" from another task so this function will never
>>>> get called with WATCH_TASK_INIT and the init task.
>>>>
>>>> 	This means that if one wants to use per-task watchers to associate data
>>>> and a function call with *every* task, special care will need to be
>>>> taken to register with the init task.
>>> no......
>> 	I've been looking through the source and, from what I can see, the end
>> of the function is not reachable. I think I need to add:
>>
>> notify_watchers(WATCH_TASK_INIT, &init_task);
>>
>> to make this into an applicable warning.
> 
> If the end of the function isn't reachable then the
> `if (task->real_parent)' can simply be removed.

Perhaps with a comment to say that it's safe (and why) to dereference 
task->real_parent to help code reviewers?

Peter
-- 
Peter Williams                                   pwil3058@...

"Learning, n. The kind of ignorance distinguishing the studious."
  -- Ambrose Bierce

On Tue, 2006-06-20 at 18:46 -0700, Andrew Morton wrote:
> On Tue, 20 Jun 2006 18:20:48 -0700
> Matt Helsley <matthltc@...> wrote:
> 
> > > > > > +static inline int notify_per_task_watchers(unsigned int val,
> > > > > > +					   struct task_struct *task)
> > > > > > +{
> > > > > > +	if (get_watch_event(val) != WATCH_TASK_INIT)
> > > > > > +		return raw_notifier_call_chain(&task->notify, val, task);
> > > > > > +	RAW_INIT_NOTIFIER_HEAD(&task->notify);
> > > > > > +	if (task->real_parent)
> > > > > > +		return raw_notifier_call_chain(&task->real_parent->notify,
> > > > > > +		   			       val, task);
> > > > > > +}
> > > > > 
> > > > > It's possible for this task to exit without returning a result.
> > > > 
> > > > Assuming you meant s/task/function/:
> > > > 
> > > > 	In the common case this will return a result because most tasks have a
> > > > real parent. The only exception should be the init task. However, the
> > > > init task does not "fork" from another task so this function will never
> > > > get called with WATCH_TASK_INIT and the init task.
> > > > 
> > > > 	This means that if one wants to use per-task watchers to associate data
> > > > and a function call with *every* task, special care will need to be
> > > > taken to register with the init task.
> > > 
> > > no......
> > 
> > 	I've been looking through the source and, from what I can see, the end
> > of the function is not reachable. I think I need to add:
> > 
> > notify_watchers(WATCH_TASK_INIT, &init_task);
> > 
> > to make this into an applicable warning.
> 
> If the end of the function isn't reachable then the
> `if (task->real_parent)' can simply be removed.

You're right -- good point.

Cheers,
	-Matt Helsley

Peter Williams wrote:
> Andrew Morton wrote:
>> On Tue, 20 Jun 2006 18:20:48 -0700
>> Matt Helsley <matthltc@...> wrote:
>>
>>>>>>> +static inline int notify_per_task_watchers(unsigned int val,
>>>>>>> +                       struct task_struct *task)
>>>>>>> +{
>>>>>>> +    if (get_watch_event(val) != WATCH_TASK_INIT)
>>>>>>> +        return raw_notifier_call_chain(&task->notify, val, task);
>>>>>>> +    RAW_INIT_NOTIFIER_HEAD(&task->notify);
>>>>>>> +    if (task->real_parent)
>>>>>>> +        return raw_notifier_call_chain(&task->real_parent->notify,
>>>>>>> +                              val, task);
>>>>>>> +}
>>>>>> It's possible for this task to exit without returning a result.
>>>>> Assuming you meant s/task/function/:
>>>>>
>>>>>     In the common case this will return a result because most tasks 
>>>>> have a
>>>>> real parent. The only exception should be the init task. However, the
>>>>> init task does not "fork" from another task so this function will 
>>>>> never
>>>>> get called with WATCH_TASK_INIT and the init task.
>>>>>
>>>>>     This means that if one wants to use per-task watchers to 
>>>>> associate data
>>>>> and a function call with *every* task, special care will need to be
>>>>> taken to register with the init task.
>>>> no......
>>>     I've been looking through the source and, from what I can see, 
>>> the end
>>> of the function is not reachable. I think I need to add:
>>>
>>> notify_watchers(WATCH_TASK_INIT, &init_task);
>>>
>>> to make this into an applicable warning.
>>
>> If the end of the function isn't reachable then the
>> `if (task->real_parent)' can simply be removed.
> 
> Perhaps with a comment to say that it's safe (and why) to dereference 
> task->real_parent to help code reviewers?

Now that I understand this code better, I'm at a loss to understand why 
you're calling the parent with the notifier.  I can't think of any 
circumstances where this would be useful.

Peter
-- 
Peter Williams                                   pwil3058@...

"Learning, n. The kind of ignorance distinguishing the studious."
  -- Ambrose Bierce

Peter Williams wrote:
> Peter Williams wrote:
>> Andrew Morton wrote:
>>> On Tue, 20 Jun 2006 18:20:48 -0700
>>> Matt Helsley <matthltc@...> wrote:
>>>
>>>>>>>> +static inline int notify_per_task_watchers(unsigned int val,
>>>>>>>> +                       struct task_struct *task)
>>>>>>>> +{
>>>>>>>> +    if (get_watch_event(val) != WATCH_TASK_INIT)
>>>>>>>> +        return raw_notifier_call_chain(&task->notify, val, task);
>>>>>>>> +    RAW_INIT_NOTIFIER_HEAD(&task->notify);
>>>>>>>> +    if (task->real_parent)
>>>>>>>> +        return raw_notifier_call_chain(&task->real_parent->notify,
>>>>>>>> +                              val, task);
>>>>>>>> +}
>>>>>>> It's possible for this task to exit without returning a result.
>>>>>> Assuming you meant s/task/function/:
>>>>>>
>>>>>>     In the common case this will return a result because most 
>>>>>> tasks have a
>>>>>> real parent. The only exception should be the init task. However, the
>>>>>> init task does not "fork" from another task so this function will 
>>>>>> never
>>>>>> get called with WATCH_TASK_INIT and the init task.
>>>>>>
>>>>>>     This means that if one wants to use per-task watchers to 
>>>>>> associate data
>>>>>> and a function call with *every* task, special care will need to be
>>>>>> taken to register with the init task.
>>>>> no......
>>>>     I've been looking through the source and, from what I can see, 
>>>> the end
>>>> of the function is not reachable. I think I need to add:
>>>>
>>>> notify_watchers(WATCH_TASK_INIT, &init_task);
>>>>
>>>> to make this into an applicable warning.
>>>
>>> If the end of the function isn't reachable then the
>>> `if (task->real_parent)' can simply be removed.
>>
>> Perhaps with a comment to say that it's safe (and why) to dereference 
>> task->real_parent to help code reviewers?
> 
> Now that I understand this code better, I'm at a loss to understand why 
> you're calling the parent with the notifier.  I can't think of any 
> circumstances where this would be useful.

I spoke to soon and can now see how I can use this functionality.  If I 
register my per task watcher with init_task, this functionality will 
allow me to do away with the normal watcher.

Sorry for doubting,
Peter
-- 
Peter Williams                                   pwil3058@...

"Learning, n. The kind of ignorance distinguishing the studious."
  -- Ambrose Bierce