Re: [PATCH 1/4] add task handling notifier: base definitions
On Wed, 2008-01-09 at 09:46 +, Jan Beulich wrote: > >> +BLOCKING_NOTIFIER_HEAD(task_notifier_list); > >> +EXPORT_SYMBOL_GPL(task_notifier_list); > >> +ATOMIC_NOTIFIER_HEAD(atomic_task_notifier_list); > >> +EXPORT_SYMBOL_GPL(atomic_task_notifier_list); > >> + > > > >When these global notifier lists were proposed years ago folks at SGI > >loudly objected with concerns over anticipated cache line bouncing on > >512+ cpu machines. Is that no longer a concern? > > I can't see an alternative, since the serialization is unavoidable. There are definitely alternatives. Naturally they are all much more complex than using a single notifier chain. You could do per-cpu chains for example (ugly, I know..). > >> @@ -121,6 +127,9 @@ void __put_task_struct(struct task_struc > >>WARN_ON(atomic_read(>usage)); > >>WARN_ON(tsk == current); > >> > >> + atomic_notifier_call_chain(_task_notifier_list, > >> + TASK_DELETE, tsk); > >> + > >>security_task_free(tsk); > >>free_uid(tsk->user); > >>put_group_info(tsk->group_info); > > > >Would the atomic notifier call chain be necessary if you hooked into an > >earlier section of do_exit() instead? > > I'm afraid it is, as I was told that sleeping in the do_exit() path is not > generally possible. > > Jan Odd. Last I checked I thought I saw a bunch of calls in do_exit() that could sleep. Only in certain sections and at the end did it appear to prevent sleeping. Sorry for the late reply. Cheers, -Matt Helsley -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/4] add task handling notifier: base definitions
On Wed, 2008-01-09 at 09:46 +, Jan Beulich wrote: +BLOCKING_NOTIFIER_HEAD(task_notifier_list); +EXPORT_SYMBOL_GPL(task_notifier_list); +ATOMIC_NOTIFIER_HEAD(atomic_task_notifier_list); +EXPORT_SYMBOL_GPL(atomic_task_notifier_list); + When these global notifier lists were proposed years ago folks at SGI loudly objected with concerns over anticipated cache line bouncing on 512+ cpu machines. Is that no longer a concern? I can't see an alternative, since the serialization is unavoidable. There are definitely alternatives. Naturally they are all much more complex than using a single notifier chain. You could do per-cpu chains for example (ugly, I know..). @@ -121,6 +127,9 @@ void __put_task_struct(struct task_struc WARN_ON(atomic_read(tsk-usage)); WARN_ON(tsk == current); + atomic_notifier_call_chain(atomic_task_notifier_list, + TASK_DELETE, tsk); + security_task_free(tsk); free_uid(tsk-user); put_group_info(tsk-group_info); Would the atomic notifier call chain be necessary if you hooked into an earlier section of do_exit() instead? I'm afraid it is, as I was told that sleeping in the do_exit() path is not generally possible. Jan Odd. Last I checked I thought I saw a bunch of calls in do_exit() that could sleep. Only in certain sections and at the end did it appear to prevent sleeping. Sorry for the late reply. Cheers, -Matt Helsley -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/4] add task handling notifier: base definitions
>> +BLOCKING_NOTIFIER_HEAD(task_notifier_list); >> +EXPORT_SYMBOL_GPL(task_notifier_list); >> +ATOMIC_NOTIFIER_HEAD(atomic_task_notifier_list); >> +EXPORT_SYMBOL_GPL(atomic_task_notifier_list); >> + > >When these global notifier lists were proposed years ago folks at SGI >loudly objected with concerns over anticipated cache line bouncing on >512+ cpu machines. Is that no longer a concern? I can't see an alternative, since the serialization is unavoidable. >> @@ -121,6 +127,9 @@ void __put_task_struct(struct task_struc >> WARN_ON(atomic_read(>usage)); >> WARN_ON(tsk == current); >> >> +atomic_notifier_call_chain(_task_notifier_list, >> + TASK_DELETE, tsk); >> + >> security_task_free(tsk); >> free_uid(tsk->user); >> put_group_info(tsk->group_info); > >Would the atomic notifier call chain be necessary if you hooked into an >earlier section of do_exit() instead? I'm afraid it is, as I was told that sleeping in the do_exit() path is not generally possible. Jan -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/4] add task handling notifier: base definitions
+BLOCKING_NOTIFIER_HEAD(task_notifier_list); +EXPORT_SYMBOL_GPL(task_notifier_list); +ATOMIC_NOTIFIER_HEAD(atomic_task_notifier_list); +EXPORT_SYMBOL_GPL(atomic_task_notifier_list); + When these global notifier lists were proposed years ago folks at SGI loudly objected with concerns over anticipated cache line bouncing on 512+ cpu machines. Is that no longer a concern? I can't see an alternative, since the serialization is unavoidable. @@ -121,6 +127,9 @@ void __put_task_struct(struct task_struc WARN_ON(atomic_read(tsk-usage)); WARN_ON(tsk == current); +atomic_notifier_call_chain(atomic_task_notifier_list, + TASK_DELETE, tsk); + security_task_free(tsk); free_uid(tsk-user); put_group_info(tsk-group_info); Would the atomic notifier call chain be necessary if you hooked into an earlier section of do_exit() instead? I'm afraid it is, as I was told that sleeping in the do_exit() path is not generally possible. Jan -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/4] add task handling notifier: base definitions
On Thu, 2007-12-20 at 13:12 +, Jan Beulich wrote:
> This is the base patch, adding notification for task creation and
> deletion.
>
> Signed-off-by: Jan Beulich <[EMAIL PROTECTED]>
> ---
> include/linux/sched.h |8 +++-
> kernel/fork.c | 11 +++
> 2 files changed, 18 insertions(+), 1 deletion(-)
>
> --- 2.6.24-rc5-notify-task.orig/include/linux/sched.h
> +++ 2.6.24-rc5-notify-task/include/linux/sched.h
> @@ -80,7 +80,7 @@ struct sched_param {
> #include
> #include
> #include
> -
> +#include
> #include
> #include
> #include
> @@ -1700,6 +1700,12 @@ extern int do_execve(char *, char __user
> extern long do_fork(unsigned long, unsigned long, struct pt_regs *, unsigned
> long, int __user *, int __user *);
> struct task_struct *fork_idle(int);
>
> +#define TASK_NEW 1
> +#define TASK_DELETE 2
> +
> +extern struct blocking_notifier_head task_notifier_list;
> +extern struct atomic_notifier_head atomic_task_notifier_list;
> +
> extern void set_task_comm(struct task_struct *tsk, char *from);
> extern void get_task_comm(char *to, struct task_struct *tsk);
>
> --- 2.6.24-rc5-notify-task.orig/kernel/fork.c
> +++ 2.6.24-rc5-notify-task/kernel/fork.c
> @@ -46,6 +46,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -71,6 +72,11 @@ DEFINE_PER_CPU(unsigned long, process_co
>
> __cacheline_aligned DEFINE_RWLOCK(tasklist_lock); /* outer */
>
> +BLOCKING_NOTIFIER_HEAD(task_notifier_list);
> +EXPORT_SYMBOL_GPL(task_notifier_list);
> +ATOMIC_NOTIFIER_HEAD(atomic_task_notifier_list);
> +EXPORT_SYMBOL_GPL(atomic_task_notifier_list);
> +
When these global notifier lists were proposed years ago folks at SGI
loudly objected with concerns over anticipated cache line bouncing on
512+ cpu machines. Is that no longer a concern?
> int nr_processes(void)
> {
> int cpu;
> @@ -121,6 +127,9 @@ void __put_task_struct(struct task_struc
> WARN_ON(atomic_read(>usage));
> WARN_ON(tsk == current);
>
> + atomic_notifier_call_chain(_task_notifier_list,
> +TASK_DELETE, tsk);
> +
> security_task_free(tsk);
> free_uid(tsk->user);
> put_group_info(tsk->group_info);
Would the atomic notifier call chain be necessary if you hooked into an
earlier section of do_exit() instead?
Cheers,
-Matt Helsley
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/4] add task handling notifier: base definitions
On Thu, 2007-12-20 at 13:12 +, Jan Beulich wrote:
This is the base patch, adding notification for task creation and
deletion.
Signed-off-by: Jan Beulich [EMAIL PROTECTED]
---
include/linux/sched.h |8 +++-
kernel/fork.c | 11 +++
2 files changed, 18 insertions(+), 1 deletion(-)
--- 2.6.24-rc5-notify-task.orig/include/linux/sched.h
+++ 2.6.24-rc5-notify-task/include/linux/sched.h
@@ -80,7 +80,7 @@ struct sched_param {
#include linux/rcupdate.h
#include linux/futex.h
#include linux/rtmutex.h
-
+#include linux/notifier.h
#include linux/time.h
#include linux/param.h
#include linux/resource.h
@@ -1700,6 +1700,12 @@ extern int do_execve(char *, char __user
extern long do_fork(unsigned long, unsigned long, struct pt_regs *, unsigned
long, int __user *, int __user *);
struct task_struct *fork_idle(int);
+#define TASK_NEW 1
+#define TASK_DELETE 2
+
+extern struct blocking_notifier_head task_notifier_list;
+extern struct atomic_notifier_head atomic_task_notifier_list;
+
extern void set_task_comm(struct task_struct *tsk, char *from);
extern void get_task_comm(char *to, struct task_struct *tsk);
--- 2.6.24-rc5-notify-task.orig/kernel/fork.c
+++ 2.6.24-rc5-notify-task/kernel/fork.c
@@ -46,6 +46,7 @@
#include linux/tsacct_kern.h
#include linux/cn_proc.h
#include linux/freezer.h
+#include linux/notifier.h
#include linux/delayacct.h
#include linux/taskstats_kern.h
#include linux/random.h
@@ -71,6 +72,11 @@ DEFINE_PER_CPU(unsigned long, process_co
__cacheline_aligned DEFINE_RWLOCK(tasklist_lock); /* outer */
+BLOCKING_NOTIFIER_HEAD(task_notifier_list);
+EXPORT_SYMBOL_GPL(task_notifier_list);
+ATOMIC_NOTIFIER_HEAD(atomic_task_notifier_list);
+EXPORT_SYMBOL_GPL(atomic_task_notifier_list);
+
When these global notifier lists were proposed years ago folks at SGI
loudly objected with concerns over anticipated cache line bouncing on
512+ cpu machines. Is that no longer a concern?
int nr_processes(void)
{
int cpu;
@@ -121,6 +127,9 @@ void __put_task_struct(struct task_struc
WARN_ON(atomic_read(tsk-usage));
WARN_ON(tsk == current);
+ atomic_notifier_call_chain(atomic_task_notifier_list,
+TASK_DELETE, tsk);
+
security_task_free(tsk);
free_uid(tsk-user);
put_group_info(tsk-group_info);
Would the atomic notifier call chain be necessary if you hooked into an
earlier section of do_exit() instead?
Cheers,
-Matt Helsley
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/4] add task handling notifier: base definitions
"Jan Beulich" <[EMAIL PROTECTED]> writes: > This is the base patch, adding notification for task creation and > deletion. I like the basic concept. At some point I suspect we'll even need a per task dynamic data allocator, that would remove even more cruft. Could you change the block notifier calls first to not take their lock when there is nothing in the list (the common case)? It is weird that they don't already do this, but they should definitely here. I think the use of the blocking/atomic notifiers needs some comment. I think I understand why you did it -- exit atomic because sleeping in exit can be illegal and blocking for fork to allow GFP_KERNEL allocations -- but that should be documented somewhere. -Andi -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/4] add task handling notifier: base definitions
Jan Beulich [EMAIL PROTECTED] writes: This is the base patch, adding notification for task creation and deletion. I like the basic concept. At some point I suspect we'll even need a per task dynamic data allocator, that would remove even more cruft. Could you change the block notifier calls first to not take their lock when there is nothing in the list (the common case)? It is weird that they don't already do this, but they should definitely here. I think the use of the blocking/atomic notifiers needs some comment. I think I understand why you did it -- exit atomic because sleeping in exit can be illegal and blocking for fork to allow GFP_KERNEL allocations -- but that should be documented somewhere. -Andi -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/4] add task handling notifier: base definitions
This is the base patch, adding notification for task creation and
deletion.
Signed-off-by: Jan Beulich <[EMAIL PROTECTED]>
---
include/linux/sched.h |8 +++-
kernel/fork.c | 11 +++
2 files changed, 18 insertions(+), 1 deletion(-)
--- 2.6.24-rc5-notify-task.orig/include/linux/sched.h
+++ 2.6.24-rc5-notify-task/include/linux/sched.h
@@ -80,7 +80,7 @@ struct sched_param {
#include
#include
#include
-
+#include
#include
#include
#include
@@ -1700,6 +1700,12 @@ extern int do_execve(char *, char __user
extern long do_fork(unsigned long, unsigned long, struct pt_regs *, unsigned
long, int __user *, int __user *);
struct task_struct *fork_idle(int);
+#define TASK_NEW 1
+#define TASK_DELETE 2
+
+extern struct blocking_notifier_head task_notifier_list;
+extern struct atomic_notifier_head atomic_task_notifier_list;
+
extern void set_task_comm(struct task_struct *tsk, char *from);
extern void get_task_comm(char *to, struct task_struct *tsk);
--- 2.6.24-rc5-notify-task.orig/kernel/fork.c
+++ 2.6.24-rc5-notify-task/kernel/fork.c
@@ -46,6 +46,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -71,6 +72,11 @@ DEFINE_PER_CPU(unsigned long, process_co
__cacheline_aligned DEFINE_RWLOCK(tasklist_lock); /* outer */
+BLOCKING_NOTIFIER_HEAD(task_notifier_list);
+EXPORT_SYMBOL_GPL(task_notifier_list);
+ATOMIC_NOTIFIER_HEAD(atomic_task_notifier_list);
+EXPORT_SYMBOL_GPL(atomic_task_notifier_list);
+
int nr_processes(void)
{
int cpu;
@@ -121,6 +127,9 @@ void __put_task_struct(struct task_struc
WARN_ON(atomic_read(>usage));
WARN_ON(tsk == current);
+ atomic_notifier_call_chain(_task_notifier_list,
+ TASK_DELETE, tsk);
+
security_task_free(tsk);
free_uid(tsk->user);
put_group_info(tsk->group_info);
@@ -1450,6 +1459,8 @@ long do_fork(unsigned long clone_flags,
set_tsk_thread_flag(p, TIF_SIGPENDING);
}
+ blocking_notifier_call_chain(_notifier_list, TASK_NEW, p);
+
if (!(clone_flags & CLONE_STOPPED))
wake_up_new_task(p, clone_flags);
else
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/4] add task handling notifier: base definitions
This is the base patch, adding notification for task creation and
deletion.
Signed-off-by: Jan Beulich [EMAIL PROTECTED]
---
include/linux/sched.h |8 +++-
kernel/fork.c | 11 +++
2 files changed, 18 insertions(+), 1 deletion(-)
--- 2.6.24-rc5-notify-task.orig/include/linux/sched.h
+++ 2.6.24-rc5-notify-task/include/linux/sched.h
@@ -80,7 +80,7 @@ struct sched_param {
#include linux/rcupdate.h
#include linux/futex.h
#include linux/rtmutex.h
-
+#include linux/notifier.h
#include linux/time.h
#include linux/param.h
#include linux/resource.h
@@ -1700,6 +1700,12 @@ extern int do_execve(char *, char __user
extern long do_fork(unsigned long, unsigned long, struct pt_regs *, unsigned
long, int __user *, int __user *);
struct task_struct *fork_idle(int);
+#define TASK_NEW 1
+#define TASK_DELETE 2
+
+extern struct blocking_notifier_head task_notifier_list;
+extern struct atomic_notifier_head atomic_task_notifier_list;
+
extern void set_task_comm(struct task_struct *tsk, char *from);
extern void get_task_comm(char *to, struct task_struct *tsk);
--- 2.6.24-rc5-notify-task.orig/kernel/fork.c
+++ 2.6.24-rc5-notify-task/kernel/fork.c
@@ -46,6 +46,7 @@
#include linux/tsacct_kern.h
#include linux/cn_proc.h
#include linux/freezer.h
+#include linux/notifier.h
#include linux/delayacct.h
#include linux/taskstats_kern.h
#include linux/random.h
@@ -71,6 +72,11 @@ DEFINE_PER_CPU(unsigned long, process_co
__cacheline_aligned DEFINE_RWLOCK(tasklist_lock); /* outer */
+BLOCKING_NOTIFIER_HEAD(task_notifier_list);
+EXPORT_SYMBOL_GPL(task_notifier_list);
+ATOMIC_NOTIFIER_HEAD(atomic_task_notifier_list);
+EXPORT_SYMBOL_GPL(atomic_task_notifier_list);
+
int nr_processes(void)
{
int cpu;
@@ -121,6 +127,9 @@ void __put_task_struct(struct task_struc
WARN_ON(atomic_read(tsk-usage));
WARN_ON(tsk == current);
+ atomic_notifier_call_chain(atomic_task_notifier_list,
+ TASK_DELETE, tsk);
+
security_task_free(tsk);
free_uid(tsk-user);
put_group_info(tsk-group_info);
@@ -1450,6 +1459,8 @@ long do_fork(unsigned long clone_flags,
set_tsk_thread_flag(p, TIF_SIGPENDING);
}
+ blocking_notifier_call_chain(task_notifier_list, TASK_NEW, p);
+
if (!(clone_flags CLONE_STOPPED))
wake_up_new_task(p, clone_flags);
else
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
