[PATCH] Per-task watchers: Enable inheritance

[PATCH] Per-task watchers: Enable inheritance

[Matt Helsley]

This allows per-task watchers to implement inheritance of the same function
and/or data in response to the initialization of new tasks. A watcher might
implement inheritance using the following notifier_call snippet:

int my_notify_func(struct notifier_block *nb, unsigned long val, void *t)
{
	struct task_struct *tsk = t;
	struct notifier_block *child_nb;
	
	switch(get_watch_event(val)){
	case WATCH_TASK_INIT: /* use container_of() to associate extra data */
		child_nb = kzalloc(sizeof(struct notifier_block), GFP_KERNEL);
		if (!child_nb)
			return NOTIFY_DONE;
		child_nb->notifier_call = my_notify_func;
		register_per_task_watcher(tsk, child_nb);
		return NOTIFY_OK;
	case WATCH_TASK_FREE:
		unregister_per_task_watcher(tsk, nb);
		kfree(nb);
		return NOTIFY_OK;

Compile tested only. Peter, is this useful to you?

Signed-off-by: Matt Helsley <matthltc@us.ibm.com>
Cc: Peter Williams <pwil3058@bigpond.net.au>
---

 include/linux/notifier.h |    6 ++++--
 ipc/sem.c                |    2 +-
 kernel/sys.c             |   12 ++++++++----
 3 files changed, 13 insertions(+), 7 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
@@ -462,19 +462,23 @@ static inline int notify_per_task_watche
 		return raw_notifier_call_chain(&task->real_parent->notify,
 		   			       val, task);
 	return 0;
 }
 
-int register_per_task_watcher(struct notifier_block *nb)
+/* tsk must be current or the fork|clone-ing child of current */
+int register_per_task_watcher(struct task_struct *tsk,
+			      struct notifier_block *nb)
 {
-	return raw_notifier_chain_register(&current->notify, nb);
+	return raw_notifier_chain_register(&tsk->notify, nb);
 }
 EXPORT_SYMBOL_GPL(register_per_task_watcher);
 
-int unregister_per_task_watcher(struct notifier_block *nb)
+/* tsk must be current or the fork|clone-ing child of current */
+int unregister_per_task_watcher(struct task_struct *tsk,
+				struct notifier_block *nb)
 {
-	return raw_notifier_chain_unregister(&current->notify, nb);
+	return raw_notifier_chain_unregister(&tsk->notify, nb);
 }
 EXPORT_SYMBOL_GPL(unregister_per_task_watcher);
 
 int notify_watchers(unsigned long val, void *v)
 {
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,12 +154,14 @@ 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);
+extern int register_per_task_watcher(struct task_struct *tsk,
+				     struct notifier_block *nb);
+extern int unregister_per_task_watcher(struct task_struct *tsk,
+				       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 */
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
@@ -1035,11 +1035,11 @@ static inline int get_undo_list(struct s
 			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);
+		retval = register_per_task_watcher(current, semun_nb);
 		if (retval)
 			goto err;
 		memset(undo_list, 0, size);
 		spin_lock_init(&undo_list->lock);
 		atomic_set(&undo_list->refcnt, 1);

Re: [PATCH] Per-task watchers: Enable inheritance

[Peter Williams]

Matt Helsley wrote:
> This allows per-task watchers to implement inheritance of the same function
> and/or data in response to the initialization of new tasks. A watcher might
> implement inheritance using the following notifier_call snippet:
> 
> int my_notify_func(struct notifier_block *nb, unsigned long val, void *t)
> {
> 	struct task_struct *tsk = t;
> 	struct notifier_block *child_nb;
> 	
> 	switch(get_watch_event(val)){
> 	case WATCH_TASK_INIT: /* use container_of() to associate extra data */
> 		child_nb = kzalloc(sizeof(struct notifier_block), GFP_KERNEL);
> 		if (!child_nb)
> 			return NOTIFY_DONE;
> 		child_nb->notifier_call = my_notify_func;
> 		register_per_task_watcher(tsk, child_nb);
> 		return NOTIFY_OK;
> 	case WATCH_TASK_FREE:
> 		unregister_per_task_watcher(tsk, nb);
> 		kfree(nb);
> 		return NOTIFY_OK;
> 
> Compile tested only. Peter, is this useful to you?

I think that it's what I want (i.e. the implementation is what I would 
have done) but I'm confused by you reference to inheritance.  My concern 
is to NOT inherit the data (via the notifier_block) but to have separate 
data for each task which is why I was concerned about not finding where 
"notify" was being initialized on boot.

What I'm doing is using an ordinary watcher to catch new tasks being 
created via WATCH_TASK_INIT and attaching a per task watcher to them at 
that time.  As per your suggestion the notifier_block for the per task 
watcher is contained in a struct which contains the data that I need to 
maintain for each task.  So two layers of watchers :-)

It will be a good test of your mechanism if I can get it to work.

It'll probably take me another couple of days to complete this code as 
I'm having to figure out how it all hangs together as I go.  I'll let 
you know when I've finished.

Peter
-- 
Peter Williams                                   pwil3058@bigpond.net.au

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

Re: [PATCH] Per-task watchers: Enable inheritance

[Matt Helsley]

On Wed, 2006-06-21 at 20:30 +1000, Peter Williams wrote:
> Matt Helsley wrote:
> > This allows per-task watchers to implement inheritance of the same function
> > and/or data in response to the initialization of new tasks. A watcher might
> > implement inheritance using the following notifier_call snippet:
> > 
> > int my_notify_func(struct notifier_block *nb, unsigned long val, void *t)
> > {
> > 	struct task_struct *tsk = t;
> > 	struct notifier_block *child_nb;
> > 	
> > 	switch(get_watch_event(val)){
> > 	case WATCH_TASK_INIT: /* use container_of() to associate extra data */
> > 		child_nb = kzalloc(sizeof(struct notifier_block), GFP_KERNEL);
> > 		if (!child_nb)
> > 			return NOTIFY_DONE;
> > 		child_nb->notifier_call = my_notify_func;
> > 		register_per_task_watcher(tsk, child_nb);
> > 		return NOTIFY_OK;
> > 	case WATCH_TASK_FREE:
> > 		unregister_per_task_watcher(tsk, nb);
> > 		kfree(nb);
> > 		return NOTIFY_OK;
> > 
> > Compile tested only. Peter, is this useful to you?
> 
> I think that it's what I want (i.e. the implementation is what I would 
> have done) but I'm confused by you reference to inheritance.  My concern 
> is to NOT inherit the data (via the notifier_block) but to have separate 
> data for each task which is why I was concerned about not finding where 
> "notify" was being initialized on boot.

Sorry, "inheritance" isn't exactly what it is. Poor choice of wording on
my part.

> What I'm doing is using an ordinary watcher to catch new tasks being 
> created via WATCH_TASK_INIT and attaching a per task watcher to them at 
> that time.  As per your suggestion the notifier_block for the per task 
> watcher is contained in a struct which contains the data that I need to 
> maintain for each task.  So two layers of watchers :-)

	Hmm. Ideally you should need only one layer. When caps have been
established on a group you'd need to create the per-task watchers. From
there on I'd expect new tasks that fork to be added to the same group
using existing per-task watchers. Of course the trick is getting the
initial task(s) into the group. With per-task watchers that's difficult
because the group assignment might originate externally but registration
must happen from the context of the task being registered. I could
remove this restriction by paying an increased cost in complexity.
Please let me know if you run into extreme difficulties with per-task
watchers due to this context constraint.

> It will be a good test of your mechanism if I can get it to work.

Yes.

> It'll probably take me another couple of days to complete this code as 
> I'm having to figure out how it all hangs together as I go.  I'll let 
> you know when I've finished.
> 
> Peter

	Thanks, I look forward to seeing it. Partially as a test and partially
because I'm curious if it will be compatible with the resource groups
(formerly CKRM) group structure.

Cheers,
	-Matt Helsley

Re: [PATCH] Per-task watchers: Enable inheritance

[John T. Kohl]

>>>>> "MattH" ==   <matthltc@us.ibm.com> writes:

MattH> This allows per-task watchers to implement inheritance of the
MattH> same function and/or data in response to the initialization of
MattH> new tasks. A watcher might implement inheritance using the
MattH> following notifier_call snippet:

I think this would meet our needs--we (MVFS) need to initialize some new
state in a child process based on our state in the parent process
(essentially, module-private inherited per-process state).  It may still
be a bit clumsy to find the per-process state in other situations,
though.  While a process is executing our module's code, would it be
safe to traverse current's notifier chain to find our state?

-- 
John Kohl
Senior Software Engineer - Rational Software - IBM Software Group
Lexington, Massachusetts, USA
jtk@us.ibm.com
<http://www.ibm.com/software/rational/>

Re: [PATCH] Per-task watchers: Enable inheritance

[Matt Helsley]

On Fri, 2006-06-23 at 17:17 -0400, John T. Kohl wrote:
> >>>>> "MattH" ==   <matthltc@us.ibm.com> writes:
> 
> MattH> This allows per-task watchers to implement inheritance of the
> MattH> same function and/or data in response to the initialization of
> MattH> new tasks. A watcher might implement inheritance using the
> MattH> following notifier_call snippet:
> 
> I think this would meet our needs--we (MVFS) need to initialize some new
> state in a child process based on our state in the parent process
> (essentially, module-private inherited per-process state).  It may still
> be a bit clumsy to find the per-process state in other situations,
> though.  While a process is executing our module's code, would it be
> safe to traverse current's notifier chain to find our state?

	Hmm. We may need to be careful with terminology here. Keep in mind that
a task is not the same as the userspace concept of a  "process".

	When a task is executing a module's code it will be safe to traverse
the task's notifier chain to find state. It will *not* be safe to
traverse the notifier chain of other tasks -- even if the other task is
a thread in the same "process".

Cheers,
	-Matt Helsley

Re: [PATCH] Per-task watchers: Enable inheritance

[Peter Williams]

Matt Helsley wrote:
> On Fri, 2006-06-23 at 17:17 -0400, John T. Kohl wrote:
>>>>>>> "MattH" ==   <matthltc@us.ibm.com> writes:
>> MattH> This allows per-task watchers to implement inheritance of the
>> MattH> same function and/or data in response to the initialization of
>> MattH> new tasks. A watcher might implement inheritance using the
>> MattH> following notifier_call snippet:
>>
>> I think this would meet our needs--we (MVFS) need to initialize some new
>> state in a child process based on our state in the parent process
>> (essentially, module-private inherited per-process state).  It may still
>> be a bit clumsy to find the per-process state in other situations,
>> though.  While a process is executing our module's code, would it be
>> safe to traverse current's notifier chain to find our state?
> 
> 	Hmm. We may need to be careful with terminology here. Keep in mind that
> a task is not the same as the userspace concept of a  "process".
> 
> 	When a task is executing a module's code it will be safe to traverse
> the task's notifier chain to find state. It will *not* be safe to
> traverse the notifier chain of other tasks -- even if the other task is
> a thread in the same "process".

Yes, the client has to make its own arrangements for protecting this 
type of thing.

The "per process CPU caps" code that I'm working on using task watchers 
has to address these issues and indications (so far) are that they're 
solvable.  I should be in a position to post that code early next week 
and, hopefully, that will give some insight into what can be achieved 
with this type of mechanism.  What I've done might not be the best way 
to solve the issues involved but it should provide a starting point for 
discussion :-)

Peter
-- 
Peter Williams                                   pwil3058@bigpond.net.au

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

Re: [PATCH] Per-task watchers: Enable inheritance

[John T. Kohl]

>>>>> "Matt" ==   <matthltc@us.ibm.com> writes:

Matt> On Fri, 2006-06-23 at 17:17 -0400, John T. Kohl wrote:
>> >>>>> "MattH" ==   <matthltc@us.ibm.com> writes:
>> 
MattH> This allows per-task watchers to implement inheritance of the
MattH> same function and/or data in response to the initialization of
MattH> new tasks. A watcher might implement inheritance using the
MattH> following notifier_call snippet:
>> 
>> I think this would meet our needs--we (MVFS) need to initialize some new
>> state in a child process based on our state in the parent process
>> (essentially, module-private inherited per-process state).  It may still
>> be a bit clumsy to find the per-process state in other situations,
>> though.  While a process is executing our module's code, would it be
>> safe to traverse current's notifier chain to find our state?

Matt> 	Hmm. We may need to be careful with terminology here. Keep in mind that
Matt> a task is not the same as the userspace concept of a  "process".

Right, sorry, I was imprecise in my wording.  What MVFS wants is per-task
private state and state inheritance on task forks.

Matt> 	When a task is executing a module's code it will be safe to traverse
Matt> the task's notifier chain to find state. It will *not* be safe to
Matt> traverse the notifier chain of other tasks -- even if the other task is
Matt> a thread in the same "process".

I'm curious to see Peter's prototype code (mentioned in his reply).  I
worry that to get safe access to the parent task's private state during
fork, we'll need something like a private hash table for our private
per-task state.  Ideally we'd like to just be able to find stuff hanging
off the task structure directly.

-- 
John Kohl
Senior Software Engineer - Rational Software - IBM Software Group
Lexington, Massachusetts, USA
jtk@us.ibm.com
<http://www.ibm.com/software/rational/>

Re: [PATCH] Per-task watchers: Enable inheritance

[Peter Williams]

John T. Kohl wrote:
>>>>>> "Matt" ==   <matthltc@us.ibm.com> writes:
> 
> Matt> On Fri, 2006-06-23 at 17:17 -0400, John T. Kohl wrote:
>>>>>>>> "MattH" ==   <matthltc@us.ibm.com> writes:
> MattH> This allows per-task watchers to implement inheritance of the
> MattH> same function and/or data in response to the initialization of
> MattH> new tasks. A watcher might implement inheritance using the
> MattH> following notifier_call snippet:
>>> I think this would meet our needs--we (MVFS) need to initialize some new
>>> state in a child process based on our state in the parent process
>>> (essentially, module-private inherited per-process state).  It may still
>>> be a bit clumsy to find the per-process state in other situations,
>>> though.  While a process is executing our module's code, would it be
>>> safe to traverse current's notifier chain to find our state?
> 
> Matt> 	Hmm. We may need to be careful with terminology here. Keep in mind that
> Matt> a task is not the same as the userspace concept of a  "process".
> 
> Right, sorry, I was imprecise in my wording.  What MVFS wants is per-task
> private state and state inheritance on task forks.
> 
> Matt> 	When a task is executing a module's code it will be safe to traverse
> Matt> the task's notifier chain to find state. It will *not* be safe to
> Matt> traverse the notifier chain of other tasks -- even if the other task is
> Matt> a thread in the same "process".
> 
> I'm curious to see Peter's prototype code (mentioned in his reply).

It will be delayed as it's gone down my priority list.

>  I
> worry that to get safe access to the parent task's private state during
> fork, we'll need something like a private hash table for our private
> per-task state.  Ideally we'd like to just be able to find stuff hanging
> off the task structure directly.
> 

No, that shouldn't be necessary.  Just use a container_of() wrapper 
model.  During fork() you're in the parent tasks context so should be 
able to access its state through "current".  Fork() is busily copying 
lots of the parent's state into the child so what you want to do should 
be no different.  If you've been using the wrapper idea, the notifier 
block that's passed into the call back code should give access to your 
data for the parent.

If you like I could give you some code snippets to show what I mean.

Peter
-- 
Peter Williams                                   pwil3058@bigpond.net.au

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