This patch replaces the relay_fork module and it implements a fork
connector in the kernel/fork.c:do_fork() routine. It applies on a kernel
2.6.11-rc4-mm1. The connector sends information about parent PID and
child PID over a netlink interface. It allows to several user space
applications to be informed when a fork occurs in the kernel. The main
drawback is that even if nobody listens, message is send. I don't know
how to avoid that.
It is used by ELSA to manage group of processes in user space and can
be used by any other user space application that needs this kind of
information.
I add a callback that turns on/off the fork connector. It's a very
simple mechanism and, as Evgeniy suggested, it may need a more complex
protocol.
ChangeLog:
- "fork_id" is now declared as a static const
- Replace snprintf() by scnprintf()
- Protect "seq" by a spin lock because the seq number can be
used by the daemon to check if all forks are intercepted
- "msg" send is now local
- Add a callback that turns on/off the fork connector.
- memset is only used to clear the msg->data part instead of all
message.
Todo:
- Test the performance impact with lmbench
- Improve the callback that turns on/off the fork connector
- Create a specific module to register the callback.
Thanks to Andrew, Evgeniy and everyone for the comments,
Guillaume
Signed-off-by: Guillaume Thouvenin <[email protected]>
---
drivers/connector/Kconfig | 11 ++++++++++
drivers/connector/connector.c | 20 ++++++++++++++++++
include/linux/connector.h | 3 ++
kernel/fork.c | 45 ++++++++++++++++++++++++++++++++++++++++++
4 files changed, 79 insertions(+)
diff -uprN -X dontdiff linux-2.6.11-rc4-mm1/drivers/connector/connector.c linux-2.6.11-rc4-mm1-cnfork/drivers/connector/connector.c
--- linux-2.6.11-rc4-mm1/drivers/connector/connector.c 2005-02-23 11:12:15.000000000 +0100
+++ linux-2.6.11-rc4-mm1-cnfork/drivers/connector/connector.c 2005-02-24 10:21:59.000000000 +0100
@@ -45,8 +45,10 @@ static DEFINE_SPINLOCK(notify_lock);
static LIST_HEAD(notify_list);
static struct cn_dev cdev;
+static struct cb_id cn_fork_id = { CN_IDX_FORK, CN_VAL_FORK };
int cn_already_initialized = 0;
+int cn_fork_enable = 0;
/*
* msg->seq and msg->ack are used to determine message genealogy.
@@ -467,6 +469,14 @@ static void cn_callback(void * data)
}
}
+static void cn_fork_callback(void *data)
+{
+ if (cn_already_initialized)
+ cn_fork_enable = cn_fork_enable ? 0 : 1;
+
+ printk(KERN_NOTICE "cn_fork_enable is set to %i\n", cn_fork_enable);
+}
+
static int cn_init(void)
{
struct cn_dev *dev = &cdev;
@@ -498,6 +508,15 @@ static int cn_init(void)
return -EINVAL;
}
+ err = cn_add_callback(&cn_fork_id, "cn_fork", &cn_fork_callback);
+ if (err) {
+ cn_del_callback(&dev->id);
+ cn_queue_free_dev(dev->cbdev);
+ if (dev->nls->sk_socket)
+ sock_release(dev->nls->sk_socket);
+ return -EINVAL;
+ }
+
cn_already_initialized = 1;
return 0;
@@ -507,6 +526,7 @@ static void cn_fini(void)
{
struct cn_dev *dev = &cdev;
+ cn_del_callback(&cn_fork_id);
cn_del_callback(&dev->id);
cn_queue_free_dev(dev->cbdev);
if (dev->nls->sk_socket)
diff -uprN -X dontdiff linux-2.6.11-rc4-mm1/drivers/connector/Kconfig linux-2.6.11-rc4-mm1-cnfork/drivers/connector/Kconfig
--- linux-2.6.11-rc4-mm1/drivers/connector/Kconfig 2005-02-23 11:12:15.000000000 +0100
+++ linux-2.6.11-rc4-mm1-cnfork/drivers/connector/Kconfig 2005-02-24 10:29:11.000000000 +0100
@@ -10,4 +10,15 @@ config CONNECTOR
Connector support can also be built as a module. If so, the module
will be called cn.ko.
+config FORK_CONNECTOR
+ bool "Enable fork connector"
+ depends on CONNECTOR=y
+ default y
+ ---help---
+ It adds a connector in kernel/fork.c:do_fork() function. When a fork
+ occurs, netlink is used to transfer information about the parent and
+ its child. This information can be used by a user space application.
+ The fork connector can be enable/disable by sending a message to the
+ connector with the corresponding group id.
+
endmenu
diff -uprN -X dontdiff linux-2.6.11-rc4-mm1/include/linux/connector.h linux-2.6.11-rc4-mm1-cnfork/include/linux/connector.h
--- linux-2.6.11-rc4-mm1/include/linux/connector.h 2005-02-23 11:12:17.000000000 +0100
+++ linux-2.6.11-rc4-mm1-cnfork/include/linux/connector.h 2005-02-24 10:25:22.000000000 +0100
@@ -28,6 +28,8 @@
#define CN_VAL_KOBJECT_UEVENT 0x0000
#define CN_IDX_SUPERIO 0xaabb /* SuperIO subsystem */
#define CN_VAL_SUPERIO 0xccdd
+#define CN_IDX_FORK 0xfeed /* fork events */
+#define CN_VAL_FORK 0xbeef
#define CONNECTOR_MAX_MSG_SIZE 1024
@@ -133,6 +135,7 @@ struct cn_dev
};
extern int cn_already_initialized;
+extern int cn_fork_enable;
int cn_add_callback(struct cb_id *, char *, void (* callback)(void *));
void cn_del_callback(struct cb_id *);
diff -uprN -X dontdiff linux-2.6.11-rc4-mm1/kernel/fork.c linux-2.6.11-rc4-mm1-cnfork/kernel/fork.c
--- linux-2.6.11-rc4-mm1/kernel/fork.c 2005-02-23 11:12:17.000000000 +0100
+++ linux-2.6.11-rc4-mm1-cnfork/kernel/fork.c 2005-02-24 10:27:02.000000000 +0100
@@ -41,6 +41,7 @@
#include <linux/profile.h>
#include <linux/rmap.h>
#include <linux/acct.h>
+#include <linux/connector.h>
#include <asm/pgtable.h>
#include <asm/pgalloc.h>
@@ -63,6 +64,48 @@ DEFINE_PER_CPU(unsigned long, process_co
EXPORT_SYMBOL(tasklist_lock);
+#ifdef CONFIG_FORK_CONNECTOR
+
+#define CN_FORK_INFO_SIZE 64
+#define CN_FORK_MSG_SIZE sizeof(struct cn_msg) + CN_FORK_INFO_SIZE
+
+spinlock_t fork_cn_lock = SPIN_LOCK_UNLOCKED;
+
+static inline void fork_connector(pid_t parent, pid_t child)
+{
+ static const struct cb_id fork_id = { CN_IDX_FORK, CN_VAL_FORK };
+ static __u32 seq; /* used to test if message is lost */
+
+ if (cn_fork_enable) {
+ struct cn_msg *msg;
+ __u8 buffer[CN_FORK_MSG_SIZE];
+
+ msg = (struct cn_msg *)buffer;
+
+ memcpy(&msg->id, &fork_id, sizeof(msg->id));
+ spin_lock(&fork_cn_lock);
+ msg->seq = seq++;
+ spin_unlock(&fork_cn_lock);
+ msg->ack = 0; /* not used */
+ /*
+ * size of data is the number of characters
+ * printed plus one for the trailing '\0'
+ */
+ /* just fill the data part with '\0' */
+ memset(msg->data, '\0', CN_FORK_INFO_SIZE);
+ msg->len = scnprintf(msg->data, CN_FORK_INFO_SIZE-1,
+ "%i %i", parent, child) + 1;
+
+ cn_netlink_send(msg, CN_IDX_FORK);
+ }
+}
+#else
+static inline void fork_connector(pid_t parent, pid_t child)
+{
+ return;
+}
+#endif
+
int nr_processes(void)
{
int cpu;
@@ -1238,6 +1281,8 @@ long do_fork(unsigned long clone_flags,
if (unlikely (current->ptrace & PT_TRACE_VFORK_DONE))
ptrace_notify ((PTRACE_EVENT_VFORK_DONE << 8) | SIGTRAP);
}
+
+ fork_connector(current->pid, p->pid);
} else {
free_pidmap(pid);
pid = PTR_ERR(p);
On Thu, 2005-02-24 at 11:24 +0100, Guillaume Thouvenin wrote:
> This patch replaces the relay_fork module and it implements a fork
> connector in the kernel/fork.c:do_fork() routine. It applies on a kernel
> 2.6.11-rc4-mm1. The connector sends information about parent PID and
> child PID over a netlink interface. It allows to several user space
> applications to be informed when a fork occurs in the kernel. The main
> drawback is that even if nobody listens, message is send. I don't know
> how to avoid that.
>
> It is used by ELSA to manage group of processes in user space and can
> be used by any other user space application that needs this kind of
> information.
>
> I add a callback that turns on/off the fork connector. It's a very
> simple mechanism and, as Evgeniy suggested, it may need a more complex
> protocol.
>
> ChangeLog:
>
> - "fork_id" is now declared as a static const
> - Replace snprintf() by scnprintf()
> - Protect "seq" by a spin lock because the seq number can be
> used by the daemon to check if all forks are intercepted
> - "msg" send is now local
> - Add a callback that turns on/off the fork connector.
> - memset is only used to clear the msg->data part instead of all
> message.
>
> Todo:
>
> - Test the performance impact with lmbench
> - Improve the callback that turns on/off the fork connector
> - Create a specific module to register the callback.
Besides connector.c changes I do not have technical objections...
But I really would like to see your second and third TODO entries in
ChangeLog before you will push it upstream.
> Thanks to Andrew, Evgeniy and everyone for the comments,
> Guillaume
>
> Signed-off-by: Guillaume Thouvenin <[email protected]>
>
> ---
>
> drivers/connector/Kconfig | 11 ++++++++++
> drivers/connector/connector.c | 20 ++++++++++++++++++
> include/linux/connector.h | 3 ++
> kernel/fork.c | 45 ++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 79 insertions(+)
>
> diff -uprN -X dontdiff linux-2.6.11-rc4-mm1/drivers/connector/connector.c linux-2.6.11-rc4-mm1-cnfork/drivers/connector/connector.c
> --- linux-2.6.11-rc4-mm1/drivers/connector/connector.c 2005-02-23 11:12:15.000000000 +0100
> +++ linux-2.6.11-rc4-mm1-cnfork/drivers/connector/connector.c 2005-02-24 10:21:59.000000000 +0100
> @@ -45,8 +45,10 @@ static DEFINE_SPINLOCK(notify_lock);
> static LIST_HEAD(notify_list);
>
> static struct cn_dev cdev;
> +static struct cb_id cn_fork_id = { CN_IDX_FORK, CN_VAL_FORK };
>
> int cn_already_initialized = 0;
> +int cn_fork_enable = 0;
>
> /*
> * msg->seq and msg->ack are used to determine message genealogy.
> @@ -467,6 +469,14 @@ static void cn_callback(void * data)
> }
> }
>
> +static void cn_fork_callback(void *data)
> +{
> + if (cn_already_initialized)
> + cn_fork_enable = cn_fork_enable ? 0 : 1;
> +
> + printk(KERN_NOTICE "cn_fork_enable is set to %i\n", cn_fork_enable);
> +}
> +
> static int cn_init(void)
> {
> struct cn_dev *dev = &cdev;
> @@ -498,6 +508,15 @@ static int cn_init(void)
> return -EINVAL;
> }
>
> + err = cn_add_callback(&cn_fork_id, "cn_fork", &cn_fork_callback);
> + if (err) {
> + cn_del_callback(&dev->id);
> + cn_queue_free_dev(dev->cbdev);
> + if (dev->nls->sk_socket)
> + sock_release(dev->nls->sk_socket);
> + return -EINVAL;
> + }
> +
> cn_already_initialized = 1;
>
> return 0;
> @@ -507,6 +526,7 @@ static void cn_fini(void)
> {
> struct cn_dev *dev = &cdev;
>
> + cn_del_callback(&cn_fork_id);
> cn_del_callback(&dev->id);
> cn_queue_free_dev(dev->cbdev);
> if (dev->nls->sk_socket)
All above should really live in the separate module.
> diff -uprN -X dontdiff linux-2.6.11-rc4-mm1/drivers/connector/Kconfig linux-2.6.11-rc4-mm1-cnfork/drivers/connector/Kconfig
> --- linux-2.6.11-rc4-mm1/drivers/connector/Kconfig 2005-02-23 11:12:15.000000000 +0100
> +++ linux-2.6.11-rc4-mm1-cnfork/drivers/connector/Kconfig 2005-02-24 10:29:11.000000000 +0100
> @@ -10,4 +10,15 @@ config CONNECTOR
> Connector support can also be built as a module. If so, the module
> will be called cn.ko.
>
> +config FORK_CONNECTOR
> + bool "Enable fork connector"
> + depends on CONNECTOR=y
> + default y
> + ---help---
> + It adds a connector in kernel/fork.c:do_fork() function. When a fork
> + occurs, netlink is used to transfer information about the parent and
> + its child. This information can be used by a user space application.
> + The fork connector can be enable/disable by sending a message to the
> + connector with the corresponding group id.
> +
> endmenu
> diff -uprN -X dontdiff linux-2.6.11-rc4-mm1/include/linux/connector.h linux-2.6.11-rc4-mm1-cnfork/include/linux/connector.h
> --- linux-2.6.11-rc4-mm1/include/linux/connector.h 2005-02-23 11:12:17.000000000 +0100
> +++ linux-2.6.11-rc4-mm1-cnfork/include/linux/connector.h 2005-02-24 10:25:22.000000000 +0100
> @@ -28,6 +28,8 @@
> #define CN_VAL_KOBJECT_UEVENT 0x0000
> #define CN_IDX_SUPERIO 0xaabb /* SuperIO subsystem */
> #define CN_VAL_SUPERIO 0xccdd
> +#define CN_IDX_FORK 0xfeed /* fork events */
> +#define CN_VAL_FORK 0xbeef
>
>
> #define CONNECTOR_MAX_MSG_SIZE 1024
> @@ -133,6 +135,7 @@ struct cn_dev
> };
>
> extern int cn_already_initialized;
> +extern int cn_fork_enable;
>
> int cn_add_callback(struct cb_id *, char *, void (* callback)(void *));
> void cn_del_callback(struct cb_id *);
> diff -uprN -X dontdiff linux-2.6.11-rc4-mm1/kernel/fork.c linux-2.6.11-rc4-mm1-cnfork/kernel/fork.c
> --- linux-2.6.11-rc4-mm1/kernel/fork.c 2005-02-23 11:12:17.000000000 +0100
> +++ linux-2.6.11-rc4-mm1-cnfork/kernel/fork.c 2005-02-24 10:27:02.000000000 +0100
> @@ -41,6 +41,7 @@
> #include <linux/profile.h>
> #include <linux/rmap.h>
> #include <linux/acct.h>
> +#include <linux/connector.h>
>
> #include <asm/pgtable.h>
> #include <asm/pgalloc.h>
> @@ -63,6 +64,48 @@ DEFINE_PER_CPU(unsigned long, process_co
>
> EXPORT_SYMBOL(tasklist_lock);
>
> +#ifdef CONFIG_FORK_CONNECTOR
> +
> +#define CN_FORK_INFO_SIZE 64
> +#define CN_FORK_MSG_SIZE sizeof(struct cn_msg) + CN_FORK_INFO_SIZE
> +
> +spinlock_t fork_cn_lock = SPIN_LOCK_UNLOCKED;
> +
> +static inline void fork_connector(pid_t parent, pid_t child)
> +{
> + static const struct cb_id fork_id = { CN_IDX_FORK, CN_VAL_FORK };
> + static __u32 seq; /* used to test if message is lost */
> +
> + if (cn_fork_enable) {
> + struct cn_msg *msg;
> + __u8 buffer[CN_FORK_MSG_SIZE];
> +
> + msg = (struct cn_msg *)buffer;
> +
> + memcpy(&msg->id, &fork_id, sizeof(msg->id));
> + spin_lock(&fork_cn_lock);
> + msg->seq = seq++;
> + spin_unlock(&fork_cn_lock);
> + msg->ack = 0; /* not used */
> + /*
> + * size of data is the number of characters
> + * printed plus one for the trailing '\0'
> + */
> + /* just fill the data part with '\0' */
> + memset(msg->data, '\0', CN_FORK_INFO_SIZE);
> + msg->len = scnprintf(msg->data, CN_FORK_INFO_SIZE-1,
> + "%i %i", parent, child) + 1;
> +
> + cn_netlink_send(msg, CN_IDX_FORK);
> + }
> +}
> +#else
> +static inline void fork_connector(pid_t parent, pid_t child)
> +{
> + return;
> +}
> +#endif
> +
> int nr_processes(void)
> {
> int cpu;
> @@ -1238,6 +1281,8 @@ long do_fork(unsigned long clone_flags,
> if (unlikely (current->ptrace & PT_TRACE_VFORK_DONE))
> ptrace_notify ((PTRACE_EVENT_VFORK_DONE << 8) | SIGTRAP);
> }
> +
> + fork_connector(current->pid, p->pid);
> } else {
> free_pidmap(pid);
> pid = PTR_ERR(p);
>
>
--
Evgeniy Polyakov
Crash is better than data corruption -- Arthur Grabowski
Guillaume Thouvenin <[email protected]> wrote:
>
> +#define CN_FORK_MSG_SIZE sizeof(struct cn_msg) + CN_FORK_INFO_SIZE
This really should be parenthesized.
> +spinlock_t fork_cn_lock = SPIN_LOCK_UNLOCKED;
This should have static scope, and could be local to fork_connector().
Please use DEFINE_SPINLOCK(). (There's a reason for this, but I forget
what it was).
> +static inline void fork_connector(pid_t parent, pid_t child)
> +{
> + static const struct cb_id fork_id = { CN_IDX_FORK, CN_VAL_FORK };
It's a bit lame to have two copies of this. Maybe have just a single copy,
declare it in connector.h?
On Thu, 2005-02-24 at 13:45 +0300, Evgeniy Polyakov wrote:
> >
> > Todo:
> >
> > - Test the performance impact with lmbench
> > - Improve the callback that turns on/off the fork connector
> > - Create a specific module to register the callback.
>
> Besides connector.c changes I do not have technical objections...
> But I really would like to see your second and third TODO entries in
> ChangeLog before you will push it upstream.
Yes, I agree with that and I'm working on those two points.
Guillaume
On Thu, Feb 24, 2005 at 02:46:05AM -0800, Andrew Morton wrote:
> Guillaume Thouvenin <[email protected]> wrote:
>
> > +spinlock_t fork_cn_lock = SPIN_LOCK_UNLOCKED;
>
> This should have static scope, and could be local to fork_connector().
>
> Please use DEFINE_SPINLOCK(). (There's a reason for this, but I forget
> what it was).
Static analysis tools, IIRC. (Stanford checker, sparse)
--
Ryan Anderson
sometimes Pug Majere
On Thu, 24 Feb 2005 11:24:37 +0100, Guillaume Thouvenin
<[email protected]> wrote:
> This patch replaces the relay_fork module and it implements a fork
> connector in the kernel/fork.c:do_fork() routine. It applies on a kernel
> 2.6.11-rc4-mm1. The connector sends information about parent PID and
> child PID over a netlink interface. It allows to several user space
> applications to be informed when a fork occurs in the kernel. The main
> drawback is that even if nobody listens, message is send. I don't know
> how to avoid that.
>
> It is used by ELSA to manage group of processes in user space and can
> be used by any other user space application that needs this kind of
> information.
>
> I add a callback that turns on/off the fork connector. It's a very
> simple mechanism and, as Evgeniy suggested, it may need a more complex
> protocol.
>
> ChangeLog:
>
> - "fork_id" is now declared as a static const
> - Replace snprintf() by scnprintf()
> - Protect "seq" by a spin lock because the seq number can be
> used by the daemon to check if all forks are intercepted
> - "msg" send is now local
> - Add a callback that turns on/off the fork connector.
> - memset is only used to clear the msg->data part instead of all
> message.
>
> Todo:
>
> - Test the performance impact with lmbench
> - Improve the callback that turns on/off the fork connector
> - Create a specific module to register the callback.
>
> Thanks to Andrew, Evgeniy and everyone for the comments,
> Guillaume
>
> Signed-off-by: Guillaume Thouvenin <[email protected]>
>
> ---
>
> drivers/connector/Kconfig | 11 ++++++++++
> drivers/connector/connector.c | 20 ++++++++++++++++++
> include/linux/connector.h | 3 ++
> kernel/fork.c | 45 ++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 79 insertions(+)
>
> diff -uprN -X dontdiff linux-2.6.11-rc4-mm1/drivers/connector/connector.c linux-2.6.11-rc4-mm1-cnfork/drivers/connector/connector.c
> --- linux-2.6.11-rc4-mm1/drivers/connector/connector.c 2005-02-23 11:12:15.000000000 +0100
> +++ linux-2.6.11-rc4-mm1-cnfork/drivers/connector/connector.c 2005-02-24 10:21:59.000000000 +0100
> @@ -45,8 +45,10 @@ static DEFINE_SPINLOCK(notify_lock);
> static LIST_HEAD(notify_list);
>
> static struct cn_dev cdev;
> +static struct cb_id cn_fork_id = { CN_IDX_FORK, CN_VAL_FORK };
>
> int cn_already_initialized = 0;
> +int cn_fork_enable = 0;
>
> /*
> * msg->seq and msg->ack are used to determine message genealogy.
> @@ -467,6 +469,14 @@ static void cn_callback(void * data)
> }
> }
>
> +static void cn_fork_callback(void *data)
> +{
> + if (cn_already_initialized)
> + cn_fork_enable = cn_fork_enable ? 0 : 1;
> +
> + printk(KERN_NOTICE "cn_fork_enable is set to %i\n", cn_fork_enable);
> +}
> +
> static int cn_init(void)
> {
> struct cn_dev *dev = &cdev;
> @@ -498,6 +508,15 @@ static int cn_init(void)
> return -EINVAL;
> }
>
> + err = cn_add_callback(&cn_fork_id, "cn_fork", &cn_fork_callback);
> + if (err) {
> + cn_del_callback(&dev->id);
> + cn_queue_free_dev(dev->cbdev);
> + if (dev->nls->sk_socket)
> + sock_release(dev->nls->sk_socket);
> + return -EINVAL;
> + }
> +
> cn_already_initialized = 1;
>
> return 0;
> @@ -507,6 +526,7 @@ static void cn_fini(void)
> {
> struct cn_dev *dev = &cdev;
>
> + cn_del_callback(&cn_fork_id);
> cn_del_callback(&dev->id);
> cn_queue_free_dev(dev->cbdev);
> if (dev->nls->sk_socket)
> diff -uprN -X dontdiff linux-2.6.11-rc4-mm1/drivers/connector/Kconfig linux-2.6.11-rc4-mm1-cnfork/drivers/connector/Kconfig
> --- linux-2.6.11-rc4-mm1/drivers/connector/Kconfig 2005-02-23 11:12:15.000000000 +0100
> +++ linux-2.6.11-rc4-mm1-cnfork/drivers/connector/Kconfig 2005-02-24 10:29:11.000000000 +0100
> @@ -10,4 +10,15 @@ config CONNECTOR
> Connector support can also be built as a module. If so, the module
> will be called cn.ko.
>
> +config FORK_CONNECTOR
> + bool "Enable fork connector"
> + depends on CONNECTOR=y
> + default y
> + ---help---
> + It adds a connector in kernel/fork.c:do_fork() function. When a fork
> + occurs, netlink is used to transfer information about the parent and
> + its child. This information can be used by a user space application.
> + The fork connector can be enable/disable by sending a message to the
> + connector with the corresponding group id.
> +
> endmenu
> diff -uprN -X dontdiff linux-2.6.11-rc4-mm1/include/linux/connector.h linux-2.6.11-rc4-mm1-cnfork/include/linux/connector.h
> --- linux-2.6.11-rc4-mm1/include/linux/connector.h 2005-02-23 11:12:17.000000000 +0100
> +++ linux-2.6.11-rc4-mm1-cnfork/include/linux/connector.h 2005-02-24 10:25:22.000000000 +0100
> @@ -28,6 +28,8 @@
> #define CN_VAL_KOBJECT_UEVENT 0x0000
> #define CN_IDX_SUPERIO 0xaabb /* SuperIO subsystem */
> #define CN_VAL_SUPERIO 0xccdd
> +#define CN_IDX_FORK 0xfeed /* fork events */
> +#define CN_VAL_FORK 0xbeef
>
> #define CONNECTOR_MAX_MSG_SIZE 1024
> @@ -133,6 +135,7 @@ struct cn_dev
> };
>
> extern int cn_already_initialized;
> +extern int cn_fork_enable;
>
> int cn_add_callback(struct cb_id *, char *, void (* callback)(void *));
> void cn_del_callback(struct cb_id *);
> diff -uprN -X dontdiff linux-2.6.11-rc4-mm1/kernel/fork.c linux-2.6.11-rc4-mm1-cnfork/kernel/fork.c
> --- linux-2.6.11-rc4-mm1/kernel/fork.c 2005-02-23 11:12:17.000000000 +0100
> +++ linux-2.6.11-rc4-mm1-cnfork/kernel/fork.c 2005-02-24 10:27:02.000000000 +0100
> @@ -41,6 +41,7 @@
> #include <linux/profile.h>
> #include <linux/rmap.h>
> #include <linux/acct.h>
> +#include <linux/connector.h>
>
> #include <asm/pgtable.h>
> #include <asm/pgalloc.h>
> @@ -63,6 +64,48 @@ DEFINE_PER_CPU(unsigned long, process_co
>
> EXPORT_SYMBOL(tasklist_lock);
>
> +#ifdef CONFIG_FORK_CONNECTOR
> +
> +#define CN_FORK_INFO_SIZE 64
> +#define CN_FORK_MSG_SIZE sizeof(struct cn_msg) + CN_FORK_INFO_SIZE
> +
> +spinlock_t fork_cn_lock = SPIN_LOCK_UNLOCKED;
> +
> +static inline void fork_connector(pid_t parent, pid_t child)
> +{
> + static const struct cb_id fork_id = { CN_IDX_FORK, CN_VAL_FORK };
> + static __u32 seq; /* used to test if message is lost */
> +
> + if (cn_fork_enable) {
> + struct cn_msg *msg;
> + __u8 buffer[CN_FORK_MSG_SIZE];
> +
> + msg = (struct cn_msg *)buffer;
> +
> + memcpy(&msg->id, &fork_id, sizeof(msg->id));
> + spin_lock(&fork_cn_lock);
> + msg->seq = seq++;
> + spin_unlock(&fork_cn_lock);
> + msg->ack = 0; /* not used */
> + /*
> + * size of data is the number of characters
> + * printed plus one for the trailing '\0'
> + */
> + /* just fill the data part with '\0' */
> + memset(msg->data, '\0', CN_FORK_INFO_SIZE);
> + msg->len = scnprintf(msg->data, CN_FORK_INFO_SIZE-1,
> + "%i %i", parent, child) + 1;
> +
> + cn_netlink_send(msg, CN_IDX_FORK);
> + }
> +}
> +#else
> +static inline void fork_connector(pid_t parent, pid_t child)
> +{
> + return;
> +}
> +#endif
> +
> int nr_processes(void)
> {
> int cpu;
> @@ -1238,6 +1281,8 @@ long do_fork(unsigned long clone_flags,
> if (unlikely (current->ptrace & PT_TRACE_VFORK_DONE))
> ptrace_notify ((PTRACE_EVENT_VFORK_DONE << 8) | SIGTRAP);
> }
> +
> + fork_connector(current->pid, p->pid);
> } else {
> free_pidmap(pid);
> pid = PTR_ERR(p);
Guillaume, I see that you are trying to discover if the kernel has
CONFIG_FORK_CONNECTOR defined or not. In case CONFIG_FORK_CONNECTOR is
not defined, you will call a "dummy" fork_connector (which just call
"return"). But why you dont move the checking to where you call
fork_connector? In this case, if CONFIG_FORK_CONNECTOR is not defined,
nothing called, and of course you dont need a "dummy" fork_connector
like in the above code.
Just try something like this:
+#ifdef CONFIG_FORK_CONNECTOR
+
+ fork_connector(current->pid, p->pid);
#endif
regards,
AQ
> > +#ifdef CONFIG_FORK_CONNECTOR
[...]
> > +static inline void fork_connector(pid_t parent, pid_t child)
> > +{
[...]
> > +}
> > +#else
> > +static inline void fork_connector(pid_t parent, pid_t child)
> > +{
> > + return;
> > +}
> > +#endif
[...]
> > @@ -1238,6 +1281,8 @@ long do_fork(unsigned long clone_flags,
> > if (unlikely (current->ptrace & PT_TRACE_VFORK_DONE))
> > ptrace_notify ((PTRACE_EVENT_VFORK_DONE << 8) | SIGTRAP);
> > }
> > +
> > + fork_connector(current->pid, p->pid);
> > } else {
> > free_pidmap(pid);
> > pid = PTR_ERR(p);
> Guillaume, I see that you are trying to discover if the kernel has
> CONFIG_FORK_CONNECTOR defined or not. In case CONFIG_FORK_CONNECTOR is
> not defined, you will call a "dummy" fork_connector (which just call
> "return"). But why you dont move the checking to where you call
> fork_connector? In this case, if CONFIG_FORK_CONNECTOR is not defined,
> nothing called, and of course you dont need a "dummy" fork_connector
> like in the above code.
>
> Just try something like this:
>
> +#ifdef CONFIG_FORK_CONNECTOR
> +
> + fork_connector(current->pid, p->pid);
> #endif
No, Guillaume did it right. Don't litter the code with useless #ifdefs
that turn one simple line of code into three for no good.
The dummy routine is optimized away by the compiler anyways.
Only the name "fork_connector()" might be discussed...
Tim
* aq ([email protected]) wrote:
> Just try something like this:
>
> +#ifdef CONFIG_FORK_CONNECTOR
> +
> + fork_connector(current->pid, p->pid);
> #endif
It's generally preferred to bury this in header files.
thanks,
-chris
--
Linux Security Modules http://lsm.immunix.org http://lsm.bkbits.net
ChangeLog:
- Add parenthesis around sizeof(struct cn_msg) + CN_FORK_INFO_SIZE
in the CN_FORK_MSG_SIZE macro
- fork_cn_lock is declareed with DEFINE_SPINLOCK()
- fork_cn_lock is defined as static and local to fork_connector()
- Create a specific module cn_fork.c in drivers/connector to
register the callback.
- Improve the callback that turns on/off the fork connector
I also run the lmbench and results are send in response to another
thread "A common layer for Accounting packages". When fork connector is
turned off the overhead is negligible. This patch works with another
small patch that fix a problem in the connector. Without it, there is a
message that says "skb does not have enough length". It will be fix in
the next -mm tree I think.
Thanks everyone for the comments,
Guillaume
Signed-off-by: Guillaume Thouvenin <[email protected]>
---
drivers/connector/Kconfig | 11 +++++
drivers/connector/Makefile | 1
drivers/connector/cn_fork.c | 85 ++++++++++++++++++++++++++++++++++++++++++++
include/linux/connector.h | 4 ++
kernel/fork.c | 44 ++++++++++++++++++++++
5 files changed, 145 insertions(+)
diff -uprN -X dontdiff linux-2.6.11-rc4-mm1/drivers/connector/cn_fork.c linux-2.6.11-rc4-mm1-cnfork/drivers/connector/cn_fork.c
--- linux-2.6.11-rc4-mm1/drivers/connector/cn_fork.c 1970-01-01 01:00:00.000000000 +0100
+++ linux-2.6.11-rc4-mm1-cnfork/drivers/connector/cn_fork.c 2005-03-01 13:13:05.000000000 +0100
@@ -0,0 +1,85 @@
+/*
+ * cn_fork.c
+ *
+ * 2005 Copyright (c) Guillaume Thouvenin <[email protected]>
+ * All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/init.h>
+
+#include <linux/connector.h>
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Guillaume Thouvenin <[email protected]>");
+MODULE_DESCRIPTION("Enable or disable the usage of the fork connector");
+
+int cn_fork_enable = 0;
+struct cb_id cb_fork_id = { CN_IDX_FORK, CN_VAL_FORK };
+
+/**
+ * cn_fork_callback - enable or disable the fork connector
+ * @data: message send by the connector
+ *
+ * The callback allows to enable or disable the sending of information
+ * about fork in the do_fork() routine. To enable the fork, the user
+ * space application must send the integer 1 in the data part of the
+ * message. To disable the fork connector, it must send the integer 0.
+ */
+static void cn_fork_callback(void *data)
+{
+ struct cn_msg *msg = (struct cn_msg *)data;
+
+ if (cn_already_initialized && (msg->len == sizeof(cn_fork_enable)))
+ memcpy(&cn_fork_enable, msg->data, sizeof(cn_fork_enable));
+}
+
+/**
+ * cn_fork_init - initialization entry point
+ *
+ * This routine will be run at kernel boot time because this driver is
+ * built in the kernel. It adds the connector callback to the connector
+ * driver.
+ */
+static int cn_fork_init(void)
+{
+ int err;
+
+ err = cn_add_callback(&cb_fork_id, "cn_fork", &cn_fork_callback);
+ if (err) {
+ printk(KERN_WARNING "Failed to register cn_fork\n");
+ return -EINVAL;
+ }
+
+ printk(KERN_NOTICE "cn_fork is registered\n");
+ return 0;
+}
+
+/**
+ * cn_fork_exit - exit entry point
+ *
+ * As this driver is always statically compiled into the kernel the
+ * cn_fork_exit has no effect.
+ */
+static void cn_fork_exit(void)
+{
+ cn_del_callback(&cb_fork_id);
+}
+
+module_init(cn_fork_init);
+module_exit(cn_fork_exit);
diff -uprN -X dontdiff linux-2.6.11-rc4-mm1/drivers/connector/Kconfig linux-2.6.11-rc4-mm1-cnfork/drivers/connector/Kconfig
--- linux-2.6.11-rc4-mm1/drivers/connector/Kconfig 2005-02-23 11:12:15.000000000 +0100
+++ linux-2.6.11-rc4-mm1-cnfork/drivers/connector/Kconfig 2005-02-24 10:29:11.000000000 +0100
@@ -10,4 +10,15 @@ config CONNECTOR
Connector support can also be built as a module. If so, the module
will be called cn.ko.
+config FORK_CONNECTOR
+ bool "Enable fork connector"
+ depends on CONNECTOR=y
+ default y
+ ---help---
+ It adds a connector in kernel/fork.c:do_fork() function. When a fork
+ occurs, netlink is used to transfer information about the parent and
+ its child. This information can be used by a user space application.
+ The fork connector can be enable/disable by sending a message to the
+ connector with the corresponding group id.
+
endmenu
diff -uprN -X dontdiff linux-2.6.11-rc4-mm1/drivers/connector/Makefile linux-2.6.11-rc4-mm1-cnfork/drivers/connector/Makefile
--- linux-2.6.11-rc4-mm1/drivers/connector/Makefile 2005-02-23 11:12:15.000000000 +0100
+++ linux-2.6.11-rc4-mm1-cnfork/drivers/connector/Makefile 2005-02-25 13:49:57.000000000 +0100
@@ -1,2 +1,3 @@
obj-$(CONFIG_CONNECTOR) += cn.o
+obj-$(CONFIG_FORK_CONNECTOR) += cn_fork.o
cn-objs := cn_queue.o connector.o
diff -uprN -X dontdiff linux-2.6.11-rc4-mm1/include/linux/connector.h linux-2.6.11-rc4-mm1-cnfork/include/linux/connector.h
--- linux-2.6.11-rc4-mm1/include/linux/connector.h 2005-02-23 11:12:17.000000000 +0100
+++ linux-2.6.11-rc4-mm1-cnfork/include/linux/connector.h 2005-03-01 12:44:50.000000000 +0100
@@ -28,6 +28,8 @@
#define CN_VAL_KOBJECT_UEVENT 0x0000
#define CN_IDX_SUPERIO 0xaabb /* SuperIO subsystem */
#define CN_VAL_SUPERIO 0xccdd
+#define CN_IDX_FORK 0xfeed /* fork events */
+#define CN_VAL_FORK 0xbeef
#define CONNECTOR_MAX_MSG_SIZE 1024
@@ -133,6 +135,8 @@ struct cn_dev
};
extern int cn_already_initialized;
+extern int cn_fork_enable;
+extern struct cb_id cb_fork_id;
int cn_add_callback(struct cb_id *, char *, void (* callback)(void *));
void cn_del_callback(struct cb_id *);
diff -uprN -X dontdiff linux-2.6.11-rc4-mm1/kernel/fork.c linux-2.6.11-rc4-mm1-cnfork/kernel/fork.c
--- linux-2.6.11-rc4-mm1/kernel/fork.c 2005-02-23 11:12:17.000000000 +0100
+++ linux-2.6.11-rc4-mm1-cnfork/kernel/fork.c 2005-03-01 08:39:13.000000000 +0100
@@ -41,6 +41,7 @@
#include <linux/profile.h>
#include <linux/rmap.h>
#include <linux/acct.h>
+#include <linux/connector.h>
#include <asm/pgtable.h>
#include <asm/pgalloc.h>
@@ -63,6 +64,47 @@ DEFINE_PER_CPU(unsigned long, process_co
EXPORT_SYMBOL(tasklist_lock);
+#ifdef CONFIG_FORK_CONNECTOR
+
+#define CN_FORK_INFO_SIZE 64
+#define CN_FORK_MSG_SIZE (sizeof(struct cn_msg) + CN_FORK_INFO_SIZE)
+
+static inline void fork_connector(pid_t parent, pid_t child)
+{
+ static DEFINE_SPINLOCK(cn_fork_lock);
+ static __u32 seq; /* used to test if message is lost */
+
+ if (cn_fork_enable) {
+ struct cn_msg *msg;
+
+ __u8 buffer[CN_FORK_MSG_SIZE];
+
+ msg = (struct cn_msg *)buffer;
+
+ memcpy(&msg->id, &cb_fork_id, sizeof(msg->id));
+ spin_lock(&cn_fork_lock);
+ msg->seq = seq++;
+ spin_unlock(&cn_fork_lock);
+ msg->ack = 0; /* not used */
+ /*
+ * size of data is the number of characters
+ * printed plus one for the trailing '\0'
+ */
+ /* just fill the data part with '\0' */
+ memset(msg->data, '\0', CN_FORK_INFO_SIZE);
+ msg->len = scnprintf(msg->data, CN_FORK_INFO_SIZE-1,
+ "%i %i", parent, child) + 1;
+
+ cn_netlink_send(msg, CN_IDX_FORK);
+ }
+}
+#else
+static inline void fork_connector(pid_t parent, pid_t child)
+{
+ return;
+}
+#endif
+
int nr_processes(void)
{
int cpu;
@@ -1238,6 +1280,8 @@ long do_fork(unsigned long clone_flags,
if (unlikely (current->ptrace & PT_TRACE_VFORK_DONE))
ptrace_notify ((PTRACE_EVENT_VFORK_DONE << 8) | SIGTRAP);
}
+
+ fork_connector(current->pid, p->pid);
} else {
free_pidmap(pid);
pid = PTR_ERR(p);
Guillaume wrote:
>
> I also run the lmbench and results are send in response to another
> thread "A common layer for Accounting packages". When fork connector is
> turned off the overhead is negligible.
Good.
If I read this code right:
>
> +static inline void fork_connector(pid_t parent, pid_t child)
> +{
> + static DEFINE_SPINLOCK(cn_fork_lock);
> + static __u32 seq; /* used to test if message is lost */
> +
> + if (cn_fork_enable) {
then the code executed if the fork connector is off is a call to an
inline function that tests an integer, finds it zero, and returns.
This is sufficiently little code that I for one would hardly
even need lmbench to be comfortable that fork() wasn't impacted
seriously, in the case that the fork connector is disabled.
--
I won't rest till it's the best ...
Programmer, Linux Scalability
Paul Jackson <[email protected]> 1.650.933.1373, 1.925.600.0401
In addition to worrying about performance and scaling, with accounting
enabled or disabled, one should also try to minimize code clutter in key
kernel files, such as fork.c
For example, one might, instead of adding 40 lines os fork_connector()
code to kernel/fork.c, instead add something like just the #include
<linux/connector.h> and the "fork_connector(current->pid, p->pid)" call
to kernel/fork.c, where include/linux/connector.h had something like:
#ifdef CONFIG_FORK_CONNECTOR
static inline void fork_connector(pid_t parent, pid_t child)
{
if (cn_fork_enable)
__fork_connector(parent, child);
}
#else
static inline void fork_connector(pid_t parent, pid_t child) {}
#endif
Then bury the interesting code in the implementation of __fork_connector(),
in drivers/connector/cn_fork.c or some such place.
This adds a real function call in the case that cn_fork_enable is set.
That code path requires more than that anyway (and it makes kernel stack
backtraces more transparent).
But it removes 40 lines of fork_connector detail from fork.c. And it
avoids marking a 40 line routine as inline ...
--
I won't rest till it's the best ...
Programmer, Linux Scalability
Paul Jackson <[email protected]> 1.650.933.1373, 1.925.600.0401
On Wednesday, March 2, 2005 6:51 am, Paul Jackson wrote:
> Guillaume wrote:
> > I also run the lmbench and results are send in response to another
> > thread "A common layer for Accounting packages". When fork connector is
> > turned off the overhead is negligible.
>
> Good.
>
> If I read this code right:
> > +static inline void fork_connector(pid_t parent, pid_t child)
> > +{
> > + static DEFINE_SPINLOCK(cn_fork_lock);
> > + static __u32 seq; /* used to test if message is lost */
> > +
> > + if (cn_fork_enable) {
>
> then the code executed if the fork connector is off is a call to an
> inline function that tests an integer, finds it zero, and returns.
>
> This is sufficiently little code that I for one would hardly
> even need lmbench to be comfortable that fork() wasn't impacted
> seriously, in the case that the fork connector is disabled.
But if it *is* enabled, it takes a global lock on every fork. That can't
scale on a big multiprocessor if lots of CPUs are doing lots of forks...
Jesse
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <asm/types.h>
#include <sys/types.h>
#include <sys/socket.h>
#include <linux/netlink.h>
void usage(){
puts("usage: fclisten <on|off>");
puts(" Default -> listening fork-connector");
puts(" on -> fork-connector Enable");
puts(" off -> fork-connector Disable");
exit(0);
}
#define MODE_LISTEN (1)
#define MODE_ENABLE (2)
#define MODE_DISABLE (3)
struct cb_id
{
__u32 idx;
__u32 val;
};
struct cn_msg
{
struct cb_id id;
__u32 seq;
__u32 ack;
__u32 len; /* Length of the following data */
__u8 data[0];
};
int main(int argc, char *argv[]){
char buf[4096];
int mode, sockfd, len;
struct sockaddr_nl ad;
struct nlmsghdr *hdr = (struct nlmsghdr *)buf;
struct cn_msg *msg = (struct cn_msg *)(buf+sizeof(struct nlmsghdr));
switch(argc){
case 1:
mode = MODE_LISTEN;
break;
case 2:
if (strcasecmp("on",argv[1])==0) {
mode = MODE_ENABLE;
}else if (strcasecmp("off",argv[1])==0){
mode = MODE_DISABLE;
}else{
usage();
}
break;
default:
usage();
break;
}
if( (sockfd=socket(PF_NETLINK, SOCK_RAW, NETLINK_NFLOG)) < 0 ){
fprintf(stderr, "Fault on socket().\n");
return( 1 );
}
ad.nl_family = AF_NETLINK;
ad.nl_pad = 0;
ad.nl_pid = getpid();
ad.nl_groups = -1;
if( bind(sockfd, (struct sockaddr *)&ad, sizeof(ad)) ){
fprintf(stderr, "Fault on bind to netlink.\n");
return( 2 );
}
if (mode==MODE_LISTEN) {
while(-1){
len = recvfrom(sockfd, buf, 4096, 0, NULL, NULL);
printf("%d-byte recv Seq=%d\n", len, hdr->nlmsg_seq);
}
}else{
ad.nl_family = AF_NETLINK;
ad.nl_pad = 0;
ad.nl_pid = 0;
ad.nl_groups = 1;
hdr->nlmsg_len = sizeof(struct nlmsghdr) + sizeof(struct cn_msg) + sizeof(int);
hdr->nlmsg_type = 0;
hdr->nlmsg_flags = 0;
hdr->nlmsg_seq = 0;
hdr->nlmsg_pid = getpid();
msg->id.idx = 0xfeed;
msg->id.val = 0xbeef;
msg->seq = msg->ack = 0;
msg->len = sizeof(int);
if (mode==MODE_ENABLE){
(*(int *)(msg->data)) = 1;
} else {
(*(int *)(msg->data)) = 0;
}
sendto(sockfd, buf, sizeof(struct nlmsghdr)+sizeof(struct cn_msg)+sizeof(int),
0, (struct sockaddr *)&ad, sizeof(ad));
}
}
On Thu, Mar 03, 2005 at 12:18:25PM +0900, Kaigai Kohei ([email protected]) wrote:
> Hello, Guillaume
>
> I tried to measure the process-creation/destruction performance on 2.6.11-rc4-mm1 plus
> some extensiton(Normal/with PAGG/with Fork-Connector).
> But I received a following messages endlessly on system console with Fork-Connector extensiton.
>
> # on IA-64 environment / When an simple fork() iteration is run in parallel.
> skb does not have enough length: requested msg->len=10[28], nlh->nlmsg_len=48[32], skb->len=48[must be 30].
> skb does not have enough length: requested msg->len=10[28], nlh->nlmsg_len=48[32], skb->len=48[must be 30].
> skb does not have enough length: requested msg->len=10[28], nlh->nlmsg_len=48[32], skb->len=48[must be 30].
> :
>
> Is's generated at drivers/connector/connector.c:__cn_rx_skb(), and this warn the length of msg's payload
> does not fit in nlmsghdr's length.
> This message means netlink packet is not sent to user space.
> I was notified occurence of fork() by printk(). :-(
No, lengths are correct, but skb can be dropped due to misaligned sizes check.
> The attached simple *.c file can enable/disable fork-connector and listen the fork-notification.
> Because It's first experimence for me to write a code to use netlink, point out a right how-to-use
> if there's some mistakes at user side apprication.
>
> Thanks.
>
> P.S. I can't reproduce lockup on 367th-fork() with your latest patch.
I've sent that patch to Guillaume and upstream, hopefully it will be
integrated into next -mm release.
> Guillaume Thouvenin wrote:
> > ChangeLog:
> >
> > - Add parenthesis around sizeof(struct cn_msg) + CN_FORK_INFO_SIZE
> > in the CN_FORK_MSG_SIZE macro
> > - fork_cn_lock is declareed with DEFINE_SPINLOCK()
> > - fork_cn_lock is defined as static and local to fork_connector()
> > - Create a specific module cn_fork.c in drivers/connector to
> > register the callback.
> > - Improve the callback that turns on/off the fork connector
> >
> > I also run the lmbench and results are send in response to another
> > thread "A common layer for Accounting packages". When fork connector is
> > turned off the overhead is negligible. This patch works with another
> > small patch that fix a problem in the connector. Without it, there is a
> > message that says "skb does not have enough length". It will be fix in
> > the next -mm tree I think.
> >
> >
> > Thanks everyone for the comments,
> > Guillaume
>
> --
> Linux Promotion Center, NEC
> KaiGai Kohei <[email protected]>
> #include <stdio.h>
> #include <stdlib.h>
> #include <string.h>
> #include <asm/types.h>
> #include <sys/types.h>
> #include <sys/socket.h>
> #include <linux/netlink.h>
>
> void usage(){
> puts("usage: fclisten <on|off>");
> puts(" Default -> listening fork-connector");
> puts(" on -> fork-connector Enable");
> puts(" off -> fork-connector Disable");
> exit(0);
> }
>
> #define MODE_LISTEN (1)
> #define MODE_ENABLE (2)
> #define MODE_DISABLE (3)
>
> struct cb_id
> {
> __u32 idx;
> __u32 val;
> };
>
> struct cn_msg
> {
> struct cb_id id;
> __u32 seq;
> __u32 ack;
> __u32 len; /* Length of the following data */
> __u8 data[0];
> };
>
>
> int main(int argc, char *argv[]){
> char buf[4096];
> int mode, sockfd, len;
> struct sockaddr_nl ad;
> struct nlmsghdr *hdr = (struct nlmsghdr *)buf;
> struct cn_msg *msg = (struct cn_msg *)(buf+sizeof(struct nlmsghdr));
>
> switch(argc){
> case 1:
> mode = MODE_LISTEN;
> break;
> case 2:
> if (strcasecmp("on",argv[1])==0) {
> mode = MODE_ENABLE;
> }else if (strcasecmp("off",argv[1])==0){
> mode = MODE_DISABLE;
> }else{
> usage();
> }
> break;
> default:
> usage();
> break;
> }
>
> if( (sockfd=socket(PF_NETLINK, SOCK_RAW, NETLINK_NFLOG)) < 0 ){
> fprintf(stderr, "Fault on socket().\n");
> return( 1 );
> }
> ad.nl_family = AF_NETLINK;
> ad.nl_pad = 0;
> ad.nl_pid = getpid();
> ad.nl_groups = -1;
Group should be CN_FORK_IDX to receive only fork's messages.
> if( bind(sockfd, (struct sockaddr *)&ad, sizeof(ad)) ){
> fprintf(stderr, "Fault on bind to netlink.\n");
> return( 2 );
> }
>
> if (mode==MODE_LISTEN) {
> while(-1){
> len = recvfrom(sockfd, buf, 4096, 0, NULL, NULL);
> printf("%d-byte recv Seq=%d\n", len, hdr->nlmsg_seq);
> }
> }else{
> ad.nl_family = AF_NETLINK;
> ad.nl_pad = 0;
> ad.nl_pid = 0;
> ad.nl_groups = 1;
>
> hdr->nlmsg_len = sizeof(struct nlmsghdr) + sizeof(struct cn_msg) + sizeof(int);
> hdr->nlmsg_type = 0;
> hdr->nlmsg_flags = 0;
> hdr->nlmsg_seq = 0;
> hdr->nlmsg_pid = getpid();
> msg->id.idx = 0xfeed;
> msg->id.val = 0xbeef;
> msg->seq = msg->ack = 0;
> msg->len = sizeof(int);
>
> if (mode==MODE_ENABLE){
> (*(int *)(msg->data)) = 1;
> } else {
> (*(int *)(msg->data)) = 0;
> }
> sendto(sockfd, buf, sizeof(struct nlmsghdr)+sizeof(struct cn_msg)+sizeof(int),
> 0, (struct sockaddr *)&ad, sizeof(ad));
> }
> }
Later today I will post finished connector.c with the all pending
patches in, and simple test program for anyone, who wants
to test fork() performace with and without fork's connector enabled.
Since Guillaume is busy, I will test it in my 2-way (1+1HT) CPU system.
--
Evgeniy Polyakov ( s0mbre )
Simple program to test fork() performance.
#include <sys/signal.h>
#include <sys/time.h>
int main(int argc, char *argv[])
{
int pid;
int i = 0, max = 100000;
struct timeval tv0, tv1;
struct timezone tz;
long diff;
if (argc >= 2)
max = atoi(argv[1]);
signal(SIGCHLD, SIG_IGN);
gettimeofday(&tv0, &tz);
while (i++ < max) {
pid = fork();
if (pid == 0) {
sleep(1);
exit (0);
}
}
gettimeofday(&tv1, &tz);
diff = (tv1.tv_sec - tv0.tv_sec)*1000000 + (tv1.tv_usec - tv0.tv_usec);
printf("Average per process fork+exit time is %ld usecs [diff=%lu, max=%d].\n", diff/max, diff, max);
return 0;
}
Creating 10k forks 100 times.
Results on 2-way SMP(1+1HT) Xeon for one fork()+exit():
2.6.11-rc4-mm1 494 usec
2.6.11-rc4-mm1-fork-connector-no_userspace 509 usec
2.6.11-rc4-mm1-fork-connector-userspace 520 usec
5% fork() degradation(connector with userspace vs. vanilla) with fork() connector.
On my test system global fork lock does not cost anything
(tested both with and without userspace listener), but it is only 2-way(pseudo).
--
Evgeniy Polyakov
Crash is better than data corruption -- Arthur Grabowski
On Thu, 2005-03-03 at 14:51 +0300, Evgeniy Polyakov wrote:
> Simple program to test fork() performance.
...
In a bit more advanced version it checks for error value,
but it never happend.
It can also have more fine grained measurment,
but IMHO the picture is clear for small systems.
> Creating 10k forks 100 times.
> Results on 2-way SMP(1+1HT) Xeon for one fork()+exit():
>
> 2.6.11-rc4-mm1 494 usec
Actually sometimes it drops to 480 usecs.
> 2.6.11-rc4-mm1-fork-connector-no_userspace 509 usec
> 2.6.11-rc4-mm1-fork-connector-userspace 520 usec
>
> 5% fork() degradation(connector with userspace vs. vanilla) with fork() connector.
> On my test system global fork lock does not cost anything
> (tested both with and without userspace listener), but it is only 2-way(pseudo).
connector.c used in experiments is attached.
If fork connector analysis will show that global fork lock is a big
bottleneck,
than seq counter can be replaced with per-cpu counter, but then inner
header should
include cpu id to properly distinguish messages.
But it is totaly fork's connector area, so I will not break things.
--
Evgeniy Polyakov
Crash is better than data corruption -- Arthur Grabowski
Hello,
This patch cannot be apply on a 2.6.11-mm1 because connector is
missing in this release. The connector module should be back in the next
kernel release. That's why it applies on a 2.6.11-rc4-mm1 tree.
Also, there is a problem with the drivers/connector/connector.c file.
The test
if (msg->len != nlh->nlmsg_len - sizeof(*msg) - sizeof(*nlh))
must be replaced by
if (NLMSG_SPACE(msg->len + sizeof(*msg)) != nlh->nlmsg_len)
Without this change, there is a problem with the size of messages. It
should be fix in future version of the connector module.
ChangeLog:
- Add parenthesis around sizeof(struct cn_msg) + CN_FORK_INFO_SIZE
in the CN_FORK_MSG_SIZE macro
- fork_cn_lock is declared with DEFINE_SPINLOCK()
- fork_cn_lock is defined as static and local to fork_connector()
- Create a specific module cn_fork.c in drivers/connector to
register the callback
- Improve the callback that turns on/off the fork connector. Now
to enable the fork connector you need to send a message with
the length of the data equal to sizeof(int) and data is '1' to
enable (or '0' to disable).
TODO:
- Run the lmbench with the user space application that manages
group of processus. if fork connector is not used the only
overhead is a test in the do_fork() routine.
Thank you everyone for your comments,
Best regards,
Guillaume
Signed-off by: Guillaume Thouvenin <[email protected]>
---
drivers/connector/Kconfig | 11 +++++
drivers/connector/Makefile | 1
drivers/connector/cn_fork.c | 85 ++++++++++++++++++++++++++++++++++++++++++++
include/linux/connector.h | 4 ++
kernel/fork.c | 44 ++++++++++++++++++++++
5 files changed, 145 insertions(+)
diff -uprN -X dontdiff linux-2.6.11-rc4-mm1/drivers/connector/cn_fork.c linux-2.6.11-rc4-mm1-cnfork/drivers/connector/cn_fork.c
--- linux-2.6.11-rc4-mm1/drivers/connector/cn_fork.c 1970-01-01 01:00:00.000000000 +0100
+++ linux-2.6.11-rc4-mm1-cnfork/drivers/connector/cn_fork.c 2005-03-01 13:13:05.000000000 +0100
@@ -0,0 +1,85 @@
+/*
+ * cn_fork.c
+ *
+ * 2005 Copyright (c) Guillaume Thouvenin <[email protected]>
+ * All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/init.h>
+
+#include <linux/connector.h>
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Guillaume Thouvenin <[email protected]>");
+MODULE_DESCRIPTION("Enable or disable the usage of the fork connector");
+
+int cn_fork_enable = 0;
+struct cb_id cb_fork_id = { CN_IDX_FORK, CN_VAL_FORK };
+
+/**
+ * cn_fork_callback - enable or disable the fork connector
+ * @data: message send by the connector
+ *
+ * The callback allows to enable or disable the sending of information
+ * about fork in the do_fork() routine. To enable the fork, the user
+ * space application must send the integer 1 in the data part of the
+ * message. To disable the fork connector, it must send the integer 0.
+ */
+static void cn_fork_callback(void *data)
+{
+ struct cn_msg *msg = (struct cn_msg *)data;
+
+ if (cn_already_initialized && (msg->len == sizeof(cn_fork_enable)))
+ memcpy(&cn_fork_enable, msg->data, sizeof(cn_fork_enable));
+}
+
+/**
+ * cn_fork_init - initialization entry point
+ *
+ * This routine will be run at kernel boot time because this driver is
+ * built in the kernel. It adds the connector callback to the connector
+ * driver.
+ */
+static int cn_fork_init(void)
+{
+ int err;
+
+ err = cn_add_callback(&cb_fork_id, "cn_fork", &cn_fork_callback);
+ if (err) {
+ printk(KERN_WARNING "Failed to register cn_fork\n");
+ return -EINVAL;
+ }
+
+ printk(KERN_NOTICE "cn_fork is registered\n");
+ return 0;
+}
+
+/**
+ * cn_fork_exit - exit entry point
+ *
+ * As this driver is always statically compiled into the kernel the
+ * cn_fork_exit has no effect.
+ */
+static void cn_fork_exit(void)
+{
+ cn_del_callback(&cb_fork_id);
+}
+
+module_init(cn_fork_init);
+module_exit(cn_fork_exit);
diff -uprN -X dontdiff linux-2.6.11-rc4-mm1/drivers/connector/Kconfig linux-2.6.11-rc4-mm1-cnfork/drivers/connector/Kconfig
--- linux-2.6.11-rc4-mm1/drivers/connector/Kconfig 2005-02-23 11:12:15.000000000 +0100
+++ linux-2.6.11-rc4-mm1-cnfork/drivers/connector/Kconfig 2005-02-24 10:29:11.000000000 +0100
@@ -10,4 +10,15 @@ config CONNECTOR
Connector support can also be built as a module. If so, the module
will be called cn.ko.
+config FORK_CONNECTOR
+ bool "Enable fork connector"
+ depends on CONNECTOR=y
+ default y
+ ---help---
+ It adds a connector in kernel/fork.c:do_fork() function. When a fork
+ occurs, netlink is used to transfer information about the parent and
+ its child. This information can be used by a user space application.
+ The fork connector can be enable/disable by sending a message to the
+ connector with the corresponding group id.
+
endmenu
diff -uprN -X dontdiff linux-2.6.11-rc4-mm1/drivers/connector/Makefile linux-2.6.11-rc4-mm1-cnfork/drivers/connector/Makefile
--- linux-2.6.11-rc4-mm1/drivers/connector/Makefile 2005-02-23 11:12:15.000000000 +0100
+++ linux-2.6.11-rc4-mm1-cnfork/drivers/connector/Makefile 2005-02-25 13:49:57.000000000 +0100
@@ -1,2 +1,3 @@
obj-$(CONFIG_CONNECTOR) += cn.o
+obj-$(CONFIG_FORK_CONNECTOR) += cn_fork.o
cn-objs := cn_queue.o connector.o
diff -uprN -X dontdiff linux-2.6.11-rc4-mm1/include/linux/connector.h linux-2.6.11-rc4-mm1-cnfork/include/linux/connector.h
--- linux-2.6.11-rc4-mm1/include/linux/connector.h 2005-02-23 11:12:17.000000000 +0100
+++ linux-2.6.11-rc4-mm1-cnfork/include/linux/connector.h 2005-03-01 12:44:50.000000000 +0100
@@ -28,6 +28,8 @@
#define CN_VAL_KOBJECT_UEVENT 0x0000
#define CN_IDX_SUPERIO 0xaabb /* SuperIO subsystem */
#define CN_VAL_SUPERIO 0xccdd
+#define CN_IDX_FORK 0xfeed /* fork events */
+#define CN_VAL_FORK 0xbeef
#define CONNECTOR_MAX_MSG_SIZE 1024
@@ -133,6 +135,8 @@ struct cn_dev
};
extern int cn_already_initialized;
+extern int cn_fork_enable;
+extern struct cb_id cb_fork_id;
int cn_add_callback(struct cb_id *, char *, void (* callback)(void *));
void cn_del_callback(struct cb_id *);
diff -uprN -X dontdiff linux-2.6.11-rc4-mm1/kernel/fork.c linux-2.6.11-rc4-mm1-cnfork/kernel/fork.c
--- linux-2.6.11-rc4-mm1/kernel/fork.c 2005-02-23 11:12:17.000000000 +0100
+++ linux-2.6.11-rc4-mm1-cnfork/kernel/fork.c 2005-03-01 08:39:13.000000000 +0100
@@ -41,6 +41,7 @@
#include <linux/profile.h>
#include <linux/rmap.h>
#include <linux/acct.h>
+#include <linux/connector.h>
#include <asm/pgtable.h>
#include <asm/pgalloc.h>
@@ -63,6 +64,47 @@ DEFINE_PER_CPU(unsigned long, process_co
EXPORT_SYMBOL(tasklist_lock);
+#ifdef CONFIG_FORK_CONNECTOR
+
+#define CN_FORK_INFO_SIZE 64
+#define CN_FORK_MSG_SIZE (sizeof(struct cn_msg) + CN_FORK_INFO_SIZE)
+
+static inline void fork_connector(pid_t parent, pid_t child)
+{
+ static DEFINE_SPINLOCK(cn_fork_lock);
+ static __u32 seq; /* used to test if message is lost */
+
+ if (cn_fork_enable) {
+ struct cn_msg *msg;
+
+ __u8 buffer[CN_FORK_MSG_SIZE];
+
+ msg = (struct cn_msg *)buffer;
+
+ memcpy(&msg->id, &cb_fork_id, sizeof(msg->id));
+ spin_lock(&cn_fork_lock);
+ msg->seq = seq++;
+ spin_unlock(&cn_fork_lock);
+ msg->ack = 0; /* not used */
+ /*
+ * size of data is the number of characters
+ * printed plus one for the trailing '\0'
+ */
+ /* just fill the data part with '\0' */
+ memset(msg->data, '\0', CN_FORK_INFO_SIZE);
+ msg->len = scnprintf(msg->data, CN_FORK_INFO_SIZE-1,
+ "%i %i", parent, child) + 1;
+
+ cn_netlink_send(msg, CN_IDX_FORK);
+ }
+}
+#else
+static inline void fork_connector(pid_t parent, pid_t child)
+{
+ return;
+}
+#endif
+
int nr_processes(void)
{
int cpu;
@@ -1238,6 +1280,8 @@ long do_fork(unsigned long clone_flags,
if (unlikely (current->ptrace & PT_TRACE_VFORK_DONE))
ptrace_notify ((PTRACE_EVENT_VFORK_DONE << 8) | SIGTRAP);
}
+
+ fork_connector(current->pid, p->pid);
} else {
free_pidmap(pid);
pid = PTR_ERR(p);
On Tue, 2005-03-08 at 08:29 +0100, Guillaume Thouvenin wrote:
> TODO:
> - Run the lmbench with the user space application that manages
> group of processus. if fork connector is not used the only
> overhead is a test in the do_fork() routine.
For information here are some results when using the process creation
tests "lat_proc fork". Test was run ten times thus the average is
computed with the ten metrics.
with a kernel 2.6.11-rc4-mm1
max value = 164.0588 msec
min value = 159.8571 msec
average = 161.7012 msec
with a kernel 2.6.11-rc4-mm1 and the cnfork (cn_fork_enable == 0)
max value = 163.3438 msec
min value = 159.8857 msec
average = 160.9447 msec
with a kernel 2.6.11-rc4-mm1 and the cnfork (cn_fork_enable == 1)
max value = 177.6885 msec
min value = 170.9057 msec
average = 173.7675 msec
So we see that when the fork connector is disable the time it takes to
split a process into two copies is the same (and it's normal) and when
the fork connector is enable, it's around 7.5% slower.
Best,
Guillaume