[PATCH 01/11] Task watchers: Task Watchers

[PATCH 01/11] Task watchers: Task Watchers

[Matt Helsley]

Use a notifier chain to inform watchers that a task is forking, execing,
changing an id, or exiting. This allows watchers to monitor these paths without
adding their own code directly to the paths.

Adding a watcher is likely to be much more maintainable when it is insensitive
to the order it is added to the chain. This means watchers should avoid
setting the priority field of the notifier blocks they are registering.
If ordering is necessary then adding calls directly in the paths in question
is probably a better idea.

WATCH_TASK_INIT is called before fork/clone complete. WATCH_TASK_CLONE is
called just before completion for fork/clone. Watchers may prevent a 
WATCH_TASK_CLONE from succeeding by returning with NOTIFY_STOP_MASK set.
However watchers are strongly discouraged from returning with NOTIFY_STOP_MASK
set from WATCH_TASK_INIT -- it may interfere with the operation of other
watchers.

WATCH_TASK_EXEC is called just before successfully returning from the exec
system call.

WATCH_TASK_UID is called every time a task's real or effective user id change.

WATCH_TASK_GID is called every time a task's real or effective group id change.

WATCH_TASK_EXIT is called at the beginning of do_exit when a task is exiting
for any reason. WATCH_TASK_FREE is called before critical task structures like
the mm_struct become inaccessible and the task is subsequently freed. Watchers
must never return NOTIFY_STOP_MASK in response to WATCH_TASK_FREE. Doing so
will prevent other watchers from cleaning up and could cause a wide variety of 
"bad things" to happen.

For every WATCH_TASK_INIT and WATCH_TASK_CLONE, a corresponding
WATCH_TASK_FREE is guaranteed.

Because fork/clone may be failed by another watcher, a watcher may see a
WATCH_TASK_FREE without a preceding WATCH_TASK_INIT or WATCH_TASK_CLONE.

Signed-off-by: Matt Helsley <matthltc@us.ibm.com>
Cc: Jes Sorensen <jes@sgi.com>
Cc: Alan Stern <stern@rowland.harvard.edu>
Cc: Chandra S. Seetharaman <sekharan@us.ibm.com>
Cc: Christoph Hellwig <hch@lst.de>
--

ChangeLog:
        Added ability to cause fork to fail with NOTIFY_STOP_MASK
        Added WARN_ON() when watchers cause WATCH_TASK_FREE to stop early
        Moved fork invocation
        Moved exec invocation
        Added current as argument to exec invocation
        Moved exit code assignment
        Added id change invocations

 fs/exec.c                |    2 ++
 include/linux/notifier.h |   14 ++++++++++++++
 include/linux/sched.h    |    1 +
 kernel/exit.c            |    8 +++++++-
 kernel/fork.c            |   18 +++++++++++++++---
 kernel/sys.c             |   31 +++++++++++++++++++++++++++++++
 6 files changed, 70 insertions(+), 4 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
@@ -38,10 +38,11 @@
 #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>
 #include <asm/pgtable.h>
 #include <asm/mmu_context.h>
@@ -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);
 
 	WARN_ON(atomic_read(&tsk->fs_excl));
 
 	if (unlikely(in_interrupt()))
 		panic("Aiee, killing interrupt handler!");
@@ -913,13 +917,16 @@ fastcall NORET_TYPE void do_exit(long co
 	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);
+	WARN_ON(notify_result & NOTIFY_STOP_MASK);
 
 	exit_mm(tsk);
 
 	exit_sem(tsk);
 	__exit_files(tsk);
@@ -934,11 +941,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);
 
-	tsk->exit_code = code;
 	proc_exit_connector(tsk);
 	exit_notify(tsk);
 #ifdef CONFIG_NUMA
 	mpol_free(tsk->mempolicy);
 	tsk->mempolicy = NULL;
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
@@ -44,10 +44,11 @@
 #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>
 #include <asm/uaccess.h>
 #include <asm/mmu_context.h>
@@ -942,10 +943,11 @@ static task_t *copy_process(unsigned lon
 				 int __user *parent_tidptr,
 				 int __user *child_tidptr,
 				 int pid)
 {
 	int retval;
+	int notify_result;
 	struct task_struct *p = NULL;
 
 	if ((clone_flags & (CLONE_NEWNS|CLONE_FS)) == (CLONE_NEWNS|CLONE_FS))
 		return ERR_PTR(-EINVAL);
 
@@ -1040,10 +1042,18 @@ static task_t *copy_process(unsigned lon
 	do_posix_clock_monotonic_gettime(&p->start_time);
 	p->security = NULL;
 	p->io_context = NULL;
 	p->io_wait = NULL;
 	p->audit_context = NULL;
+
+	p->tgid = p->pid;
+	if (clone_flags & CLONE_THREAD)
+		p->tgid = current->tgid;
+
+	notify_result = notify_watchers(WATCH_TASK_INIT, p);
+	if (notify_result & NOTIFY_STOP_MASK)
+		goto bad_fork_cleanup;
 	cpuset_fork(p);
 #ifdef CONFIG_NUMA
  	p->mempolicy = mpol_copy(p->mempolicy);
  	if (IS_ERR(p->mempolicy)) {
  		retval = PTR_ERR(p->mempolicy);
@@ -1076,13 +1086,10 @@ static task_t *copy_process(unsigned lon
 	p->softirq_disable_ip = 0;
 	p->softirq_disable_event = 0;
 	p->hardirq_context = 0;
 	p->softirq_context = 0;
 #endif
-	p->tgid = p->pid;
-	if (clone_flags & CLONE_THREAD)
-		p->tgid = current->tgid;
 
 	if ((retval = security_task_alloc(p)))
 		goto bad_fork_cleanup_policy;
 	if ((retval = audit_alloc(p)))
 		goto bad_fork_cleanup_security;
@@ -1243,10 +1250,13 @@ 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;
 
 bad_fork_cleanup_namespaces:
 	exit_task_namespaces(p);
@@ -1267,10 +1277,12 @@ 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:
 #ifdef CONFIG_NUMA
 	mpol_free(p->mempolicy);
 bad_fork_cleanup_cpuset:
 #endif
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
@@ -48,10 +48,11 @@
 #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>
 
 #ifdef CONFIG_KMOD
@@ -1095,10 +1096,11 @@ int search_binary_handler(struct linux_b
 				allow_write_access(bprm->file);
 				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);
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
@@ -152,7 +152,21 @@ extern int raw_notifier_call_chain(struc
 #define CPU_UP_CANCELED		0x0004 /* CPU (unsigned)v NOT coming up */
 #define CPU_DOWN_PREPARE	0x0005 /* CPU (unsigned)v going down */
 #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);
+#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 */
+#define WATCH_TASK_CLONE		0x00000002 /* "after" clone */
+#define WATCH_TASK_EXEC			0x00000003
+#define WATCH_TASK_UID			0x00000004  /* [re]uid changed */
+#define WATCH_TASK_GID			0x00000005  /* [re]gid changed */
+#define WATCH_TASK_EXIT			0x0000FFFE
+#define WATCH_TASK_FREE			0x0000FFFF
+
 #endif /* __KERNEL__ */
 #endif /* _LINUX_NOTIFIER_H */
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
@@ -210,10 +210,11 @@ long io_schedule_timeout(long timeout);
 
 extern void cpu_init (void);
 extern void trap_init(void);
 extern void update_process_times(int user);
 extern void scheduler_tick(void);
+extern int notify_watchers(unsigned long, void *);
 
 #ifdef CONFIG_DETECT_SOFTLOCKUP
 extern void softlockup_tick(void);
 extern void spawn_softlockup_task(void);
 extern void touch_softlockup_watchdog(void);
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
@@ -433,10 +433,33 @@ int unregister_reboot_notifier(struct no
 	return blocking_notifier_chain_unregister(&reboot_notifier_list, nb);
 }
 
 EXPORT_SYMBOL(unregister_reboot_notifier);
 
+/* task watchers notifier chain */
+static ATOMIC_NOTIFIER_HEAD(task_watchers);
+
+int register_task_watcher(struct notifier_block *nb)
+{
+	return atomic_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);
+}
+
+EXPORT_SYMBOL_GPL(unregister_task_watcher);
+
+int notify_watchers(unsigned long val, void *v)
+{
+	return atomic_notifier_call_chain(&task_watchers, val, v);
+}
+
+
 static int set_one_prio(struct task_struct *p, int niceval, int error)
 {
 	int no_nice;
 
 	if (p->uid != current->euid &&
@@ -838,10 +861,11 @@ asmlinkage long sys_setregid(gid_t rgid,
 		current->sgid = new_egid;
 	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;
 }
 
 /*
@@ -878,10 +902,11 @@ 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)
@@ -968,10 +993,11 @@ asmlinkage long sys_setreuid(uid_t ruid,
 	    (euid != (uid_t) -1 && euid != old_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);
 }
 
@@ -1016,10 +1042,11 @@ 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);
 }
 
@@ -1065,10 +1092,11 @@ asmlinkage long sys_setresuid(uid_t ruid
 	current->fsuid = current->euid;
 	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);
 }
 
@@ -1118,10 +1146,11 @@ asmlinkage long sys_setresgid(gid_t rgid
 		current->gid = 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)
@@ -1161,10 +1190,11 @@ 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;
@@ -1190,10 +1220,11 @@ asmlinkage long sys_setfsgid(gid_t gid)
 			current->mm->dumpable = suid_dumpable;
 			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;
 }
 

--

Re: [PATCH 01/11] Task watchers: Task Watchers

[Chase Venters]

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?

> +int notify_watchers(unsigned long val, void *v)
> +{
> +	return atomic_notifier_call_chain(&task_watchers, val, v);
> +}

Might this be called notify_task_watchers()?

Thanks,
Chase

Re: [PATCH 01/11] Task watchers: Task Watchers

[Matt Helsley]

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

[PATCH 02/11] Task watchers: Register process events task watcher

[Matt Helsley]

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@us.ibm.com>
Cc: Guillaume Thouvenin <guillaume.thouvenin@bull.net>
--

 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)

--

Re: [PATCH 02/11] Task watchers: Register process events task watcher

[Chase Venters]

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?

> +	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)

Thanks,
Chase

Re: [PATCH 02/11] Task watchers: Register process events task watcher

[Matt Helsley]

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

[PATCH 03/11] Task watchers: Refactor process events

[Matt Helsley]

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@us.ibm.com>
Cc: Guillaume Thouvenin <guillaume.thouvenin@bull.net>
--

 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,

--

Re: [PATCH 03/11] Task watchers: Refactor process events

[Chase Venters]

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

Re: [PATCH 03/11] Task watchers: Refactor process events

[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

Re: [PATCH 03/11] Task watchers: Refactor process events

[Chase Venters]

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

[PATCH 04/11] Task watchers: Make process events configurable as a module

[Matt Helsley]

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

Signed-off-by: Matt Helsley <matthltc@us.ibm.com>
Cc: Guillaume Thouvenin <guillaume.thouvenin@bull.net>
--

 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@us.ibm.com>");
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Notification of process events.");
+MODULE_VERSION("2:1.0");

--

Re: [PATCH 04/11] Task watchers: Make process events configurable as a module

[Chase Venters]

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

Re: [Lse-tech] [PATCH 04/11] Task watchers: Make process events configurable as a module

[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

[PATCH 05/11] Task watchers: Allow task watchers to block

[Matt Helsley]

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

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

 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 = {

--

[PATCH 06/11] Task watchers: Register audit task watcher

[Matt Helsley]

Adapt audit to use task watchers.

Signed-off-by: Matt Helsley <matthltc@us.ibm.com>
Cc: David Woodhouse <dwmw2@infradead.org>
Cc: linux-audit@redhat.com
--

 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:

--

Re: [PATCH 06/11] Task watchers: Register audit task watcher

[Alexander Viro]

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.

Re: [PATCH 06/11] Task watchers: Register audit task watcher

[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

[PATCH 07/11] Task watchers: Register per-task delay accounting task watcher

[Matt Helsley]

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

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

 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)
 {

--

Re: [PATCH 07/11] Task watchers: Register per-task delay accounting task watcher

[Shailabh Nagar]

Matt Helsley wrote:

>Adapts delayacct to use Task Watchers. Does not adapt taskstats to use Task
>Watchers.
>
>Signed-off-by: Matt Helsley <matthltc@us.ibm.com>
>Cc: Shailabh Nagar <nagar@watson.ibm.com>
>Cc: Balbir Singh <balbir@in.ibm.com>
>Cc: Chandra S. Seetharaman <sekharan@us.ibm.com>
>--
>
> 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

Re: [PATCH 07/11] Task watchers: Register per-task delay accounting task watcher

[Matt Helsley]

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@us.ibm.com>
> >Cc: Shailabh Nagar <nagar@watson.ibm.com>
> >Cc: Balbir Singh <balbir@in.ibm.com>
> >Cc: Chandra S. Seetharaman <sekharan@us.ibm.com>
> >--
> >
> > 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

[PATCH 08/11] Task watchers: Register profile as a task watcher

[Matt Helsley]

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@us.ibm.com>
Cc: Philippe Elie <phil.el@wanadoo.fr>
Cc: oprofile-list@lists.sf.net
--

 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;

--

Re: [PATCH 08/11] Task watchers: Register profile as a task watcher

[Chase Venters]

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

Re: [Lse-tech] [PATCH 08/11] Task watchers: Register profile as a task watcher

[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

[PATCH 09/11] Task watchers: Add support for per-task watchers

[Matt Helsley]

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@us.ibm.com>
Cc: Jes Sorensen <jes@sgi.com>
Cc: Chandra S. Seetharaman <sekharan@us.ibm.com>
Cc: Alan Stern <stern@rowland.harvard.edu>
--

 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 */

--

Re: [PATCH 09/11] Task watchers: Add support for per-task watchers

[Peter Williams]

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@us.ibm.com>
> Cc: Jes Sorensen <jes@sgi.com>
> Cc: Chandra S. Seetharaman <sekharan@us.ibm.com>
> Cc: Alan Stern <stern@rowland.harvard.edu>
[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@bigpond.net.au

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

Re: [Lse-tech] [PATCH 09/11] Task watchers: Add support for per-task watchers

[Matt Helsley]

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@us.ibm.com>
> > Cc: Jes Sorensen <jes@sgi.com>
> > Cc: Chandra S. Seetharaman <sekharan@us.ibm.com>
> > Cc: Alan Stern <stern@rowland.harvard.edu>
> [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

Re: [Lse-tech] [PATCH 09/11] Task watchers: Add support for per-task watchers

[Andrew Morton]

Matt Helsley <matthltc@us.ibm.com> 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.

Re: [Lse-tech] [PATCH 09/11] Task watchers: Add support for per-task watchers

[Peter Williams]

Andrew Morton wrote:
> Matt Helsley <matthltc@us.ibm.com> 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@bigpond.net.au

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

Re: [Lse-tech] [PATCH 09/11] Task watchers: Add support for per-task watchers

[Matt Helsley]

On Tue, 2006-06-20 at 16:15 -0700, Andrew Morton wrote:
> Matt Helsley <matthltc@us.ibm.com> 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

Re: [Lse-tech] [PATCH 09/11] Task watchers: Add support for per-task watchers

[Andrew Morton]

On Tue, 20 Jun 2006 18:20:48 -0700
Matt Helsley <matthltc@us.ibm.com> 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.

Re: [Lse-tech] [PATCH 09/11] Task watchers: Add support for per-task watchers

[Peter Williams]

Andrew Morton wrote:
> On Tue, 20 Jun 2006 18:20:48 -0700
> Matt Helsley <matthltc@us.ibm.com> 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@bigpond.net.au

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

Re: [Lse-tech] [PATCH 09/11] Task watchers: Add support for per-task watchers

[Peter Williams]

Peter Williams wrote:
> Andrew Morton wrote:
>> On Tue, 20 Jun 2006 18:20:48 -0700
>> Matt Helsley <matthltc@us.ibm.com> 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@bigpond.net.au

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

Re: [Lse-tech] [PATCH 09/11] Task watchers: Add support for per-task watchers

[Peter Williams]

Peter Williams wrote:
> Peter Williams wrote:
>> Andrew Morton wrote:
>>> On Tue, 20 Jun 2006 18:20:48 -0700
>>> Matt Helsley <matthltc@us.ibm.com> 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@bigpond.net.au

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

Re: [Lse-tech] [PATCH 09/11] Task watchers: Add support for per-task watchers

[Matt Helsley]

On Tue, 2006-06-20 at 18:46 -0700, Andrew Morton wrote:
> On Tue, 20 Jun 2006 18:20:48 -0700
> Matt Helsley <matthltc@us.ibm.com> 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

Re: [Lse-tech] [PATCH 09/11] Task watchers: Add support for per-task watchers

[Peter Williams]

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@us.ibm.com>
>>> Cc: Jes Sorensen <jes@sgi.com>
>>> Cc: Chandra S. Seetharaman <sekharan@us.ibm.com>
>>> Cc: Alan Stern <stern@rowland.harvard.edu>
>> [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@bigpond.net.au

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

[PATCH 10/11] Task watchers: Register semundo task watcher

[Matt Helsley]

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

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

 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:

--

[PATCH 11/11] Task watchers: Register per-task semundo watcher

[Matt Helsley]

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@us.ibm.com>
Cc: Jes Sorensen <jes@sgi.com>
--

 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;

--