[Lse-tech] Re: [PATCH] Simplified Task Notifier (was Process Notification (task_notifiers)

[Matt H. <mat...@us...>]

On Thu, 2005-10-13 at 16:20 -0700, Chandra Seetharaman wrote:
> On Thu, 2005-10-13 at 09:54 -0500, Jack Steiner wrote:

<snip>

> > > So, the overhead for tasks with no notifiers would be calling your
> > > registered global function.
> 
> in this code you can see that the additional overhead for tasks that
> have no notifiers attached is the calling of the function
> _jtask_notifier_call_chain().

	I think this is an interesting approach, and quite possibly the right
way to do it. My only concern is the synchronization primitive used to
protect the global list might prevent notifier functions from sleeping.
Perhaps a read-write semaphore would be better than RCU since it would
allow them to sleep.

> > Both PAGG and the task_notifier mechanism work about the same way. All notifier
> > functions are per-task. There is no global callout list that is maintained &
> > used at fork/exit/etc.
> > 
> > If a task does not require a notifier function to be called at fork/exit/etc,
> > it's task_notifier_block pointer in the task_struct is NULL. The inline code in
> > fork/exit/etc detects the NULL & skips the out-of-line call to the function that
> > performs the notifier callouts.
> > 
> 
> 
>  include/linux/init_task.h     |    1 
>  include/linux/sched.h         |    1 
>  include/linux/task_notifier.h |   41 +++++++++++++
>  kernel/Makefile               |    2 
>  kernel/task_notifier.c        |  128 ++++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 172 insertions(+), 1 deletion(-)
> 
> Index: l2613-t_notify/include/linux/task_notifier.h
> ===================================================================
> --- /dev/null
> +++ l2613-t_notify/include/linux/task_notifier.h
> @@ -0,0 +1,41 @@
> +/*
> + * This file is subject to the terms and conditions of the GNU General Public
> + * License.  See the file "COPYING" in the main directory of this archive
> + * for more details.
> + *
> + * Copyright (C) 2005 Silicon Graphics, Inc. All rights reserved.
> + *
> + *
> + *	Routines to manage task notifier chains for passing task state changes to any
> + *	interested routines.
> + *
> + */
> +
> +#ifndef _LINUX_TASK_NOTIFIER_H
> +#define _LINUX_TASK_NOTIFIER_H
> +#include <linux/compiler.h>
> +#include <linux/types.h>
> +#include <linux/sched.h>
> +#include <linux/task_notify.h>
> +
> +#include <asm/current.h>
> +
> +struct jtask_notifier_block;
> +
> +typedef void (jtask_notifier_func)(unsigned int, struct jtask_notifier_block *nb);
> +
> +struct jtask_notifier_block
> +{
> +	jtask_notifier_func *jtask_notifier_func;
> +	struct jtask_notifier_block *next;
> +	int priority;
> +	unsigned int id_select;
> +};
> +
> +
> +extern void jtask_notifier_chain_register(struct jtask_notifier_block *nb);
> +extern void jtask_notifier_chain_register_child(struct jtask_notifier_block *nb, struct task_struct *p);
> +extern void jtask_notifier_chain_unregister(struct jtask_notifier_block *nb);
> +extern struct jtask_notifier_block *find_task_notifier_block_proc(jtask_notifier_func *func);
> +
> +#endif /* _LINUX_TASK_NOTIFIER_H */
> Index: l2613-t_notify/kernel/task_notifier.c
> ===================================================================
> --- /dev/null
> +++ l2613-t_notify/kernel/task_notifier.c
> @@ -0,0 +1,128 @@
> +/*
> + * This file is subject to the terms and conditions of the GNU General Public
> + * License.  See the file "COPYING" in the main directory of this archive
> + * for more details.
> + *
> + * Copyright (C) 2005 Silicon Graphics, Inc. All rights reserved.
> + */
> +
> +#include <linux/config.h>
> +#include <linux/module.h>
> +#include <linux/task_notifier.h>
> +#include <asm/current.h>
> +
> +/**
> + *  jtask_notifier_chain_register_task - Add a task_notifier to a task.
> + *	@n: Entry to be added to the task notifier chain
> + *	@p: Task (must be current or a child being created during fork/clone
> + */
> +static inline void jtask_notifier_chain_register_task(struct jtask_notifier_block
> +						     *nb, struct task_struct *p)
> +{
> +	struct jtask_notifier_block **list =
> +	    (struct jtask_notifier_block **)&p->task_notifier;
> +
> +	while (*list && nb->priority > (*list)->priority)
> +		list = &((*list)->next);
> +	nb->next = *list;
> +	*list = nb;
> +}
> +
> +/**
> + *  jtask_notifier_chain_register_child - Add a task_notifier to a task during fork/clone.
> + *	@n: Entry to be added to the task notifier chain
> + *	@p: Task (must be current or a child being created during fork/clone
> + */
> +void jtask_notifier_chain_register_child(struct jtask_notifier_block *nb,
> +					struct task_struct *p)
> +{
> +	jtask_notifier_chain_register_task(nb, p);
> +}
> +
> +/**
> + *  jtask_notifier_chain_register - Add a task_notifier to the current task
> + *	@n: Entry to be added to the task notifier chain
> + */
> +void jtask_notifier_chain_register(struct jtask_notifier_block *nb)
> +{
> +	jtask_notifier_chain_register_task(nb, current);
> +}
> +
> +/**
> + *	jtask_notifier_chain_unregister - Remove notifier from the current task
> + *	@nb: Entry in notifier chain
> + *
> + */
> +
> +void jtask_notifier_chain_unregister(struct jtask_notifier_block *nb)
> +{
> +	struct jtask_notifier_block **list =
> +	    (struct jtask_notifier_block **)&current->task_notifier;
> +
> +	while (*list && *list != nb)
> +		list = &((*list)->next);
> +
> +	if (*list == nb)
> +		*list = nb->next;
> +}
> +
> +/**
> + *	find_task_notifier_block_proc - Scan the current task's notifiers for a
> + *	jtask_notifier_block with a pointer to @func
> + *	@func: Function
> + *
> + */
> +
> +struct jtask_notifier_block *find_task_notifier_block_proc(jtask_notifier_func *
> +							  func)
> +{
> +	struct jtask_notifier_block *nb = current->task_notifier;
> +
> +	while (nb && nb->jtask_notifier_func != func)
> +		nb = nb->next;
> +
> +	return nb;
> +}
> +
> +/**
> + *	jtask_notifier_call_chain - Call functions in a notifier chain
> + *	@id: Callout identifier
> + *	@arg: Argument to pass to called fumctions
> + *
> + *	Calls each function in a notifier chain in turn.
> + *
> + */
> +
> +static void _jtask_notifier_call_chain(unsigned int id, struct task_struct *tsk)
> +{
> +	struct jtask_notifier_block *nb, *next_nb = tsk->task_notifier;
> +
> +	if (likely(tsk->task_notifier))
> +		return;

I think the test above should be inverted according to previous
discussions:

if (likely(!tsk->task_notifier))
	return;

<snip>

Cheers,
	-Matt Helsley