[patch 1/2] fork_connector: add a fork connector

[patch 1/2] fork_connector: add a fork connector

[Guillaume Thouvenin]

  This patch adds a fork connector in the do_fork() routine. It sends a
netlink datagram when enabled. The message can be read by a user space
application. By this way, the user space application is alerted when a
fork occurs.

  It uses the userspace <-> kernelspace connector that works on top of
the netlink protocol. The fork connector is enabled or disabled by
sending a message to the connector. The unique sequence number of
messages can be used to check if a message is lost.

  When the fork connector is disabled, the "lat_proc fork" test returns
a value equal to:
     Process fork+exit: 154.6944 microseconds
  
  When the fork connector is enabled, the same test returns:
     Process fork+exit: 165.6667 microseconds

  So the overhead (the construction and the sending of the message) is
around 7%.

  This patch applies to 2.6.11-mm4. Some other patches are needed that
fix problems in the connector.c file. At least, you need to apply the
patch provided in the second email.

Signed-off-by: Guillaume Thouvenin <guillaume.thouvenin@bull.net>
--- 

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

Index: linux-2.6.11-mm4-cnfork/drivers/connector/Kconfig
===================================================================
--- linux-2.6.11-mm4-cnfork.orig/drivers/connector/Kconfig	2005-03-16 14:21:46.000000000 +0100
+++ linux-2.6.11-mm4-cnfork/drivers/connector/Kconfig	2005-03-16 14:34:41.000000000 +0100
@@ -10,4 +10,15 @@
 	  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
Index: linux-2.6.11-mm4-cnfork/drivers/connector/Makefile
===================================================================
--- linux-2.6.11-mm4-cnfork.orig/drivers/connector/Makefile	2005-03-16 14:21:46.000000000 +0100
+++ linux-2.6.11-mm4-cnfork/drivers/connector/Makefile	2005-03-16 14:34:41.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
Index: linux-2.6.11-mm4-cnfork/drivers/connector/cn_fork.c
===================================================================
--- linux-2.6.11-mm4-cnfork.orig/drivers/connector/cn_fork.c	2003-01-30 11:24:37.000000000 +0100
+++ linux-2.6.11-mm4-cnfork/drivers/connector/cn_fork.c	2005-03-16 14:34:41.000000000 +0100
@@ -0,0 +1,85 @@
+/*
+ * 	cn_fork.c
+ * 
+ * 2005 Copyright (c) Guillaume Thouvenin <guillaume.thouvenin@bull.net>
+ * 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 <guillaume.thouvenin@bull.net>");
+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);
Index: linux-2.6.11-mm4-cnfork/include/linux/connector.h
===================================================================
--- linux-2.6.11-mm4-cnfork.orig/include/linux/connector.h	2005-03-16 14:21:49.000000000 +0100
+++ linux-2.6.11-mm4-cnfork/include/linux/connector.h	2005-03-16 14:34:41.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 @@
 };
 
 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 *);
Index: linux-2.6.11-mm4-cnfork/kernel/fork.c
===================================================================
--- linux-2.6.11-mm4-cnfork.orig/kernel/fork.c	2005-03-16 14:21:49.000000000 +0100
+++ linux-2.6.11-mm4-cnfork/kernel/fork.c	2005-03-16 14:34:41.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 @@
 
 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 /* CONFIG_FORK_CONNECTOR */
+
 int nr_processes(void)
 {
 	int cpu;
@@ -1253,6 +1295,8 @@
 			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);

Re: [patch 1/2] fork_connector: add a fork connector

[Jesse Barnes]

On Thursday, March 17, 2005 1:04 am, Guillaume Thouvenin wrote:
> +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);

As I mentioned before, this won't work very well on a large CPU count system.  
cn_fork_lock will be taken by each CPU everytime it does a fork, meaning that 
forks will be very slow if lots of CPUs are doing them at the same time.  Is 
there a more scalable way to ensure message delivery?

Jesse

Re: [patch 1/2] fork_connector: add a fork connector

[Evgeniy Polyakov]

On Thu, 17 Mar 2005 08:56:57 -0800
Jesse Barnes <jbarnes@engr.sgi.com> wrote:

> On Thursday, March 17, 2005 1:04 am, Guillaume Thouvenin wrote:
> > +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);
> 
> As I mentioned before, this won't work very well on a large CPU count system.  
> cn_fork_lock will be taken by each CPU everytime it does a fork, meaning that 
> forks will be very slow if lots of CPUs are doing them at the same time.  Is 

Maybe... But..., concider ppc system, 
each lock is about 10 instructions(or even less), 
increment with return is about 3-5 instructions, unlock - 
barrier() and setting.
The whole fork syscall contains too bigger number
of instruction(do_fork() itself is more than 500, 
and it is not counting number of instructions in 
functions that are called from do_fork()) 
to care about 20 idle on each CPU, 
even if there are 512 of them.

The most significant part there - is requirement to store
u32 seq in each CPU's cache and thus flush cacheline + 
invalidate/get from mem on each other cpus
each time it is accessed, which is a big price.

> there a more scalable way to ensure message delivery?

It is totally Guillaume's work - so he decides, 
I would recomend per cpu counters and processor's 
id in each message.
And of course userspace should take care of misordered
messages.
I personally prefer such mechanism.

Guillaume?
 
> Jesse


	Evgeniy Polyakov

Only failure makes us experts. -- Theo de Raadt

Re: [patch 1/2] fork_connector: add a fork connector

[Jesse Barnes]

On Thursday, March 17, 2005 1:38 pm, Evgeniy Polyakov wrote:
> The most significant part there - is requirement to store
> u32 seq in each CPU's cache and thus flush cacheline +
> invalidate/get from mem on each other cpus
> each time it is accessed, which is a big price.

Same thing has to happen with the lock.  To put it simply, writing global 
variables from multiple CPUs with anything other than very low frequency is 
bad.

> It is totally Guillaume's work - so he decides,
> I would recomend per cpu counters and processor's
> id in each message.
> And of course userspace should take care of misordered
> messages.
> I personally prefer such mechanism.

Yep, I agree.  Hopefully Guillaume will too :)

Jesse

Re: [patch 1/2] fork_connector: add a fork connector

[Guillaume Thouvenin]

On Thu, 2005-03-17 at 14:05 -0800, Jesse Barnes wrote:
> On Thursday, March 17, 2005 1:38 pm, Evgeniy Polyakov wrote:
> > The most significant part there - is requirement to store
> > u32 seq in each CPU's cache and thus flush cacheline +
> > invalidate/get from mem on each other cpus
> > each time it is accessed, which is a big price.
> 
> Same thing has to happen with the lock.  To put it simply, writing global 
> variables from multiple CPUs with anything other than very low frequency is 
> bad.
> 
> > It is totally Guillaume's work - so he decides,
> > I would recomend per cpu counters and processor's
> > id in each message.
> > And of course userspace should take care of misordered
> > messages.
> > I personally prefer such mechanism.
> 
> Yep, I agree.  Hopefully Guillaume will too :)

Sure, I agree and I will implement the per cpu counters solution with a
processor id in each message. 

Thank you very much Jesse and Evgeniy for your great help,
Best regards,

Guillaume

Re: [patch 1/2] fork_connector: add a fork connector

[Guillaume Thouvenin]

ChangeLog:

  - Remove the global cn_fork_lock and replace it by a per CPU 
    counter. 
  - The processor ID has been added in the data part of the message.
    Thus datas sent in a message are: "CPU_ID PARENT_PID CHILD_PID"

  Those modifications were done to be more scalable because, as
mentioned by Jesse Barnes, the global cn_fork_lock won't work well on a
large CPU system.

  This patch applies to 2.6.11-mm4.

Regards,
Guillaume

---

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


Index: linux-2.6.11-mm4-cnfork/drivers/connector/Kconfig
===================================================================
--- linux-2.6.11-mm4-cnfork.orig/drivers/connector/Kconfig	2005-03-16 14:21:46.000000000 +0100
+++ linux-2.6.11-mm4-cnfork/drivers/connector/Kconfig	2005-03-16 14:34:41.000000000 +0100
@@ -10,4 +10,15 @@
 	  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
Index: linux-2.6.11-mm4-cnfork/drivers/connector/Makefile
===================================================================
--- linux-2.6.11-mm4-cnfork.orig/drivers/connector/Makefile	2005-03-16 14:21:46.000000000 +0100
+++ linux-2.6.11-mm4-cnfork/drivers/connector/Makefile	2005-03-16 14:34:41.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
Index: linux-2.6.11-mm4-cnfork/drivers/connector/cn_fork.c
===================================================================
--- linux-2.6.11-mm4-cnfork.orig/drivers/connector/cn_fork.c	2003-01-30 11:24:37.000000000 +0100
+++ linux-2.6.11-mm4-cnfork/drivers/connector/cn_fork.c	2005-03-16 14:34:41.000000000 +0100
@@ -0,0 +1,85 @@
+/*
+ * 	cn_fork.c
+ * 
+ * 2005 Copyright (c) Guillaume Thouvenin <guillaume.thouvenin@bull.net>
+ * 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 <guillaume.thouvenin@bull.net>");
+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);
Index: linux-2.6.11-mm4-cnfork/include/linux/connector.h
===================================================================
--- linux-2.6.11-mm4-cnfork.orig/include/linux/connector.h	2005-03-16 14:21:49.000000000 +0100
+++ linux-2.6.11-mm4-cnfork/include/linux/connector.h	2005-03-16 14:34:41.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 @@
 };
 
 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 *);
Index: linux-2.6.11-mm4-cnfork/kernel/fork.c
===================================================================
--- linux-2.6.11-mm4-cnfork.orig/kernel/fork.c	2005-03-16 14:21:49.000000000 +0100
+++ linux-2.6.11-mm4-cnfork/kernel/fork.c	2005-03-21 13:13:25.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 @@
 
 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 DEFINE_PER_CPU(unsigned long, fork_counts);
+
+static inline void fork_connector(pid_t parent, pid_t child)
+{
+	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));
+		
+		msg->ack = 0; /* not used */
+		msg->seq = get_cpu_var(fork_counts)++;
+		
+		/* 
+		 * size of data is the number of characters 
+		 * printed plus one for the trailing '\0'
+		 */
+		memset(msg->data, '\0', CN_FORK_INFO_SIZE);
+		msg->len = scnprintf(msg->data, CN_FORK_INFO_SIZE-1, 
+				    "%i %i %i", 
+				    smp_processor_id(), parent, child) + 1;
+		
+		put_cpu_var(fork_counts);
+
+		cn_netlink_send(msg, CN_IDX_FORK);
+	}
+}
+#else
+static inline void fork_connector(pid_t parent, pid_t child) 
+{
+	return; 
+}
+#endif /* CONFIG_FORK_CONNECTOR */
+
 int nr_processes(void)
 {
 	int cpu;
@@ -1253,6 +1295,8 @@
 			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);

Re: [patch 1/2] fork_connector: add a fork connector

[Ram]

On Mon, 2005-03-21 at 04:48, Guillaume Thouvenin wrote:
> ChangeLog:
> 
>   - Remove the global cn_fork_lock and replace it by a per CPU 
>     counter. 
>   - The processor ID has been added in the data part of the message.
>     Thus datas sent in a message are: "CPU_ID PARENT_PID CHILD_PID"
> 
>   Those modifications were done to be more scalable because, as
> mentioned by Jesse Barnes, the global cn_fork_lock won't work well on a
> large CPU system.
> 
>   This patch applies to 2.6.11-mm4.
Guillaume,

     If a bunch of applications are listening for fork events, 
     your patch allows any application to turn off the 
     fork event notification?  Is this the right behavior?

     Should'nt it turn off the fork-event notification when 
     the number of listeners become zero?
  
RP


 

Re: [patch 1/2] fork_connector: add a fork connector

[Evgeniy Polyakov]

[-- Attachment #1: Type: text/plain, Size: 1341 bytes --]

On Mon, 2005-03-21 at 12:52 -0800, Ram wrote:
> On Mon, 2005-03-21 at 04:48, Guillaume Thouvenin wrote:
> > ChangeLog:
> > 
> >   - Remove the global cn_fork_lock and replace it by a per CPU 
> >     counter. 
> >   - The processor ID has been added in the data part of the message.
> >     Thus datas sent in a message are: "CPU_ID PARENT_PID CHILD_PID"
> > 
> >   Those modifications were done to be more scalable because, as
> > mentioned by Jesse Barnes, the global cn_fork_lock won't work well on a
> > large CPU system.
> > 
> >   This patch applies to 2.6.11-mm4.
> Guillaume,
> 
>      If a bunch of applications are listening for fork events, 
>      your patch allows any application to turn off the 
>      fork event notification?  Is this the right behavior?
> 
>      Should'nt it turn off the fork-event notification when 
>      the number of listeners become zero?

There is no number of listeners - netlink sockets provide multicast
dataflow.
[Although one can obtain that number].

As far as I can see, Guillaume's application is main management utility
- 
it can turn on or off some feature, like "ip" can turn on or off
interfaces 
without waiting when bounded processes decide to exit.

> RP

-- 
        Evgeniy Polyakov

Crash is better than data corruption -- Arthur Grabowski

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [patch 1/2] fork_connector: add a fork connector

[Ram]

On Mon, 2005-03-21 at 20:36, Evgeniy Polyakov wrote:
> On Mon, 2005-03-21 at 12:52 -0800, Ram wrote:
> > On Mon, 2005-03-21 at 04:48, Guillaume Thouvenin wrote:
> > > ChangeLog:
> > > 
> > >   - Remove the global cn_fork_lock and replace it by a per CPU 
> > >     counter. 
> > >   - The processor ID has been added in the data part of the message.
> > >     Thus datas sent in a message are: "CPU_ID PARENT_PID CHILD_PID"
> > > 
> > >   Those modifications were done to be more scalable because, as
> > > mentioned by Jesse Barnes, the global cn_fork_lock won't work well on a
> > > large CPU system.
> > > 
> > >   This patch applies to 2.6.11-mm4.
> > Guillaume,
> > 
> >      If a bunch of applications are listening for fork events, 
> >      your patch allows any application to turn off the 
> >      fork event notification?  Is this the right behavior?
> > 
> >      Should'nt it turn off the fork-event notification when 
> >      the number of listeners become zero?
> 
> There is no number of listeners - netlink sockets provide multicast
> dataflow.
> [Although one can obtain that number].
> 
> As far as I can see, Guillaume's application is main management utility
> - 

Yes. agreed. But again nothing stops one of the many application
listening on the multicast events from stopping the fork-events.

Even though Guillame's application claims itself the main management
utility, nothing stops another application from assuming the management
role.

I think if the the connector infrastructure provides a administrative
kind of channel, (the one I mentioned in the reply to Guillame) that
should help get better control on the management aspects of the
event stream.

RP
> 
> it can turn on or off some feature, like "ip" can turn on or off
> interfaces 
> without waiting when bounded processes decide to exit.


> > RP

Re: [patch 1/2] fork_connector: add a fork connector

[Guillaume Thouvenin]

On Mon, 2005-03-21 at 12:52 -0800, Ram wrote:
>      If a bunch of applications are listening for fork events, 
>      your patch allows any application to turn off the 
>      fork event notification?  Is this the right behavior?

Yes it is. The main management is done by application so, if several
applications are listening for fork events you need to choose which one
will turn off the fork connector. 

I want to keep this turn on/off mechanism simple but if it's needed I
can manage the variable "cn_fork_enable" as a counter. Thus the callback
could be something like:

static void cn_fork_callback(void *data)
{
  int start; 
  struct cn_msg *msg = (struct cn_msg *)data;

  if (cn_already_initialized && (msg->len == sizeof(cn_fork_enable))) {
    memcpy(&start, msg->data, sizeof(cn_fork_enable));
    if (start)
      cn_fork_enable++;
    else
      cn_fork_enable > 0 ? cn_fork_enable-- : 0;
  }
}


What do you think about this implementation? 

Guillaume

Re: [patch 1/2] fork_connector: add a fork connector

[Jay Lan]

Guillaume Thouvenin wrote:
> On Mon, 2005-03-21 at 12:52 -0800, Ram wrote:
> 
>>     If a bunch of applications are listening for fork events, 
>>     your patch allows any application to turn off the 
>>     fork event notification?  Is this the right behavior?
> 
> 
> Yes it is. The main management is done by application so, if several
> applications are listening for fork events you need to choose which one
> will turn off the fork connector. 

It is not practical. One listener never know who else are listening
to the fork connector.

We also need to protect yet another global variable "cn_fork_enable".

Also, in order to implement "cn_fork_enable" as a counter, we need
some sort of registration to prevent an application from sending
repeated "off" notification. One can only turn off its own switch.

Thanks,
  - jay

> 
> I want to keep this turn on/off mechanism simple but if it's needed I
> can manage the variable "cn_fork_enable" as a counter. Thus the callback
> could be something like:
> 
> static void cn_fork_callback(void *data)
> {
>   int start; 
>   struct cn_msg *msg = (struct cn_msg *)data;
> 
>   if (cn_already_initialized && (msg->len == sizeof(cn_fork_enable))) {
>     memcpy(&start, msg->data, sizeof(cn_fork_enable));
>     if (start)
>       cn_fork_enable++;
>     else
>       cn_fork_enable > 0 ? cn_fork_enable-- : 0;
>   }
> }
> 
> 
> What do you think about this implementation? 
> 
> Guillaume

Re: [patch 1/2] fork_connector: add a fork connector

[Guillaume Thouvenin]

On Tue, 2005-03-22 at 10:15 -0800, Jay Lan wrote:
> Guillaume Thouvenin wrote:
> > On Mon, 2005-03-21 at 12:52 -0800, Ram wrote:
> > 
> >>     If a bunch of applications are listening for fork events, 
> >>     your patch allows any application to turn off the 
> >>     fork event notification?  Is this the right behavior?
> > 
> > 
> > Yes it is. The main management is done by application so, if several
> > applications are listening for fork events you need to choose which one
> > will turn off the fork connector. 
> 
> It is not practical. One listener never know who else are listening
> to the fork connector.
> 
> We also need to protect yet another global variable "cn_fork_enable".
> 
> Also, in order to implement "cn_fork_enable" as a counter, we need
> some sort of registration to prevent an application from sending
> repeated "off" notification. One can only turn off its own switch.

  I agree with the Evgeniy's ip example. I can provide a fork connector
interface utility to enable or disable the fork connector. Thus,
listeners don't have to start and stop the fork connector and they don't
have to worry about who else are listening the fork connector. It also
removes the need of registration.

  I copy at the end of this email the code of the fork connector
controller application (called fcctl).

  I need to add the possibility to get the state of the fork connector.
I also need to extend the fork connector to send to everyone information
when it is turned off/on.


Regards,
Guillaume


/*
 * fcctl.c - Fork connector interface utility
 *
 * Used to turn on/off the fork connector
 */

#include <stdio.h>
#include <string.h>
#include <unistd.h>

#include <sys/socket.h>
#include <sys/types.h>

#include <linux/connector.h>
#include <linux/netlink.h>

#define MESSAGE_SIZE    (sizeof(struct nlmsghdr) + \
                         sizeof(struct cn_msg)   + \
                         sizeof(int))

#define FORK_CN_DISABLE	0
#define FORK_CN_ENABLE	1
#define FORK_CN_UNKNOWN	2

void show_help()
{
	printf("usage: fcctl {on|off}\n\n");
}

int main(int argc, char **argv)
{
	int sk_nl;
	int err;
	int action = FORK_CN_UNKNOWN;
	struct sockaddr_nl sa_nl;	/* info for the netlink interface */
	char buff[128];		/* must be > MESSAGE_SIZE */
	struct nlmsghdr *hdr;
	struct cn_msg *mesg;

	if (getuid() != 0) {
		printf("Only root can start/stop the fork connector\n");
		return 0;
	}

	if (argc != 2) {
		show_help();
		return 0;
	}

	if (strcmp(argv[1], "on") == 0)
		action = FORK_CN_ENABLE;
	else if (strcmp(argv[1], "off") == 0)
		action = FORK_CN_DISABLE;

	if (action == FORK_CN_UNKNOWN) {
		show_help();
		return 0;
	}

	/*
	 * Create an endpoint for communication. Use the kernel user
	 * interface device (PF_NETLINK) which is a datagram oriented
	 * service (SOCK_DGRAM). The protocol used is the netfilter/iptables 
	 * ULOG protocol (NETLINK_NFLOG)
	 */
	sk_nl = socket(PF_NETLINK, SOCK_DGRAM, NETLINK_NFLOG);
	if (sk_nl == -1) {
		printf("socket sk_nl error");
		return -1;
	}

	sa_nl.nl_family = AF_NETLINK;
	sa_nl.nl_groups = CN_IDX_FORK;
	sa_nl.nl_pid = getpid();

	err = bind(sk_nl, (struct sockaddr *)&sa_nl, sizeof(sa_nl));
	if (err == -1) {
		printf("binding sk_nl error");
		close(sk_nl);
		return -1;
	}

	/* Clear the buffer */
	memset(buff, '\0', sizeof(buff));

	/* fill the message header */
	hdr = (struct nlmsghdr *)buff;

	hdr->nlmsg_len = MESSAGE_SIZE;
	hdr->nlmsg_type = NLMSG_DONE;
	hdr->nlmsg_flags = 0;
	hdr->nlmsg_seq = 0;
	hdr->nlmsg_pid = getpid();

	/* the message */
	mesg = (struct cn_msg *)NLMSG_DATA(hdr);
	mesg->id.idx = CN_IDX_FORK;
	mesg->id.val = CN_VAL_FORK;
	mesg->seq = 0;
	mesg->ack = 0;
	mesg->len = sizeof(int);
	mesg->data[0] = action;

	send(sk_nl, hdr, hdr->nlmsg_len, 0);

	close(sk_nl);

	return 0;
}
   

Re: [patch 1/2] fork_connector: add a fork connector

[Ram]

On Mon, 2005-03-21 at 23:07, Guillaume Thouvenin wrote:
> On Mon, 2005-03-21 at 12:52 -0800, Ram wrote:
> >      If a bunch of applications are listening for fork events, 
> >      your patch allows any application to turn off the 
> >      fork event notification?  Is this the right behavior?
> 
> Yes it is. The main management is done by application so, if several
> applications are listening for fork events you need to choose which one
> will turn off the fork connector. 
> 
> I want to keep this turn on/off mechanism simple but if it's needed I
> can manage the variable "cn_fork_enable" as a counter. Thus the callback
> could be something like:
> 
> static void cn_fork_callback(void *data)
> {
>   int start; 
>   struct cn_msg *msg = (struct cn_msg *)data;
> 
>   if (cn_already_initialized && (msg->len == sizeof(cn_fork_enable))) {
>     memcpy(&start, msg->data, sizeof(cn_fork_enable));
>     if (start)
>       cn_fork_enable++;
>     else
>       cn_fork_enable > 0 ? cn_fork_enable-- : 0;
>   }
> }

I think a better way is:

   Providing a different connector channel called the administrator 
   channel which can be used only by a super-user, and gives you
   the ability to switch on or off any connector channel including the
   fork-connector channel.
 
   For lack of better term I am using the word 'channel' to mean
   something that carries events of particular type through the
   connector-infrastructure.
 
RP


> 
> 
> What do you think about this implementation? 
> 
> Guillaume
> 

Re: [patch 1/2] fork_connector: add a fork connector

[Evgeniy Polyakov]

On Tue, 22 Mar 2005 10:26:19 -0800
Ram <linuxram@us.ibm.com> wrote:

> On Mon, 2005-03-21 at 23:07, Guillaume Thouvenin wrote:
> > On Mon, 2005-03-21 at 12:52 -0800, Ram wrote:
> > >      If a bunch of applications are listening for fork events, 
> > >      your patch allows any application to turn off the 
> > >      fork event notification?  Is this the right behavior?
> > 
> > Yes it is. The main management is done by application so, if several
> > applications are listening for fork events you need to choose which one
> > will turn off the fork connector. 
> > 
> > I want to keep this turn on/off mechanism simple but if it's needed I
> > can manage the variable "cn_fork_enable" as a counter. Thus the callback
> > could be something like:
> > 
> > static void cn_fork_callback(void *data)
> > {
> >   int start; 
> >   struct cn_msg *msg = (struct cn_msg *)data;
> > 
> >   if (cn_already_initialized && (msg->len == sizeof(cn_fork_enable))) {
> >     memcpy(&start, msg->data, sizeof(cn_fork_enable));
> >     if (start)
> >       cn_fork_enable++;
> >     else
> >       cn_fork_enable > 0 ? cn_fork_enable-- : 0;
> >   }
> > }
> 
> I think a better way is:
> 
>    Providing a different connector channel called the administrator 
>    channel which can be used only by a super-user, and gives you
>    the ability to switch on or off any connector channel including the
>    fork-connector channel.

Only super-user can bind netlink socket to multicast group.

>    For lack of better term I am using the word 'channel' to mean
>    something that carries events of particular type through the
>    connector-infrastructure.

I still do not see why it is needed.
Super-user can run ip command and turn network interface off
not waiting while apache or named exits or unbind.

In theory I can create some kind of userspace registration mechanism,
when userspace application reports it's pid to the connector, 
and then it sends data to the specified pids, but does not 
allow controlling from userspace.
But I really do not think it is a good idea to permit
non-priviledged userspace processes to know about deep
kernel internals through connector's messages.

> RP
> 
> 
> > 
> > 
> > What do you think about this implementation? 
> > 
> > Guillaume
> > 


	Evgeniy Polyakov

Only failure makes us experts. -- Theo de Raadt

Re: [patch 1/2] fork_connector: add a fork connector

[Ram]

On Tue, 2005-03-22 at 11:22, Evgeniy Polyakov wrote:
> On Tue, 22 Mar 2005 10:26:19 -0800
> Ram <linuxram@us.ibm.com> wrote:
> 
> > On Mon, 2005-03-21 at 23:07, Guillaume Thouvenin wrote:
> > > On Mon, 2005-03-21 at 12:52 -0800, Ram wrote:
> > > >      If a bunch of applications are listening for fork events, 
> > > >      your patch allows any application to turn off the 
> > > >      fork event notification?  Is this the right behavior?
> > > 
> > > Yes it is. The main management is done by application so, if several
> > > applications are listening for fork events you need to choose which one
> > > will turn off the fork connector. 
> > > 
> > > I want to keep this turn on/off mechanism simple but if it's needed I
> > > can manage the variable "cn_fork_enable" as a counter. Thus the callback
> > > could be something like:
> > > 
> > > static void cn_fork_callback(void *data)
> > > {
> > >   int start; 
> > >   struct cn_msg *msg = (struct cn_msg *)data;
> > > 
> > >   if (cn_already_initialized && (msg->len == sizeof(cn_fork_enable))) {
> > >     memcpy(&start, msg->data, sizeof(cn_fork_enable));
> > >     if (start)
> > >       cn_fork_enable++;
> > >     else
> > >       cn_fork_enable > 0 ? cn_fork_enable-- : 0;
> > >   }
> > > }
> > 
> > I think a better way is:
> > 
> >    Providing a different connector channel called the administrator 
> >    channel which can be used only by a super-user, and gives you
> >    the ability to switch on or off any connector channel including the
> >    fork-connector channel.
> 
> Only super-user can bind netlink socket to multicast group.

ok. I did not realize that.

> 
> >    For lack of better term I am using the word 'channel' to mean
> >    something that carries events of particular type through the
> >    connector-infrastructure.
> 
> I still do not see why it is needed.
> Super-user can run ip command and turn network interface off
> not waiting while apache or named exits or unbind.
> 
> In theory I can create some kind of userspace registration mechanism,
> when userspace application reports it's pid to the connector, 
> and then it sends data to the specified pids, but does not 
> allow controlling from userspace.
> But I really do not think it is a good idea to permit
> non-priviledged userspace processes to know about deep
> kernel internals through connector's messages.

Yes. non-priviledged userspace processes should not know
any deep kernel internals through connector events.

I think what I am driving at is, an application that is critically
dependent on the fork-notification, suddenly stops receiving such
notification because some other application has switched off the 
service without its notice. 

the reason I am concerned is I am planning to feed this fork-events
to my in-kernel module. Side note: I would really like support for
in-kernel listners through connector infrastructure.

RP

> 
> > RP
> > 
> > 
> > > 
> > > 
> > > What do you think about this implementation? 
> > > 
> > > Guillaume
> > > 
> 
> 
> 	Evgeniy Polyakov
> 
> Only failure makes us experts. -- Theo de Raadt

Re: [patch 1/2] fork_connector: add a fork connector

[Evgeniy Polyakov]

On Tue, 22 Mar 2005 11:18:07 -0800
Ram <linuxram@us.ibm.com> wrote:

> > I still do not see why it is needed.
> > Super-user can run ip command and turn network interface off
> > not waiting while apache or named exits or unbind.
> > 
> > In theory I can create some kind of userspace registration mechanism,
> > when userspace application reports it's pid to the connector, 
> > and then it sends data to the specified pids, but does not 
> > allow controlling from userspace.
> > But I really do not think it is a good idea to permit
> > non-priviledged userspace processes to know about deep
> > kernel internals through connector's messages.
> 
> Yes. non-priviledged userspace processes should not know
> any deep kernel internals through connector events.
> 
> I think what I am driving at is, an application that is critically
> dependent on the fork-notification, suddenly stops receiving such
> notification because some other application has switched off the 
> service without its notice. 
> 
> the reason I am concerned is I am planning to feed this fork-events
> to my in-kernel module. Side note: I would really like support for
> in-kernel listners through connector infrastructure.

fork connector can be extended to send to the rest of the world
information that it was turned off or on.

The easiest way to listen inside the kernel for connector's 
notifications is creating appropriate socket in userspace
and then obtain it through the following code which must be
run in context of the process that created above socket:

	struct file *file;
	struct inode *inode;
	int err = 0;
	
	file = fget(userspace_socket_number_in_the_current_process_context);
	if (!file) {
		return -ENODEV;
	}
	inode = file->f_dentry->d_inode;
	if (inode->i_sock) {
		sock = SOCKET_I(inode);
		err = 0;
	} else {
		fput(file);
		err = -ENODEV;
	}

sock is struct sock, which then can be used for read/write
operations using 
kernel_recvmsg(sock, &msg, &iov, 1, size, 0);
kernel_sendmsg(sock, &msg, &iov, 1, size);

with the following initialisation:

sock->sk->sk_allocation |= GFP_NOIO;
iov.iov_base = buf;
iov.iov_len = size;
msg.msg_name = NULL;
msg.msg_namelen = 0;
msg.msg_control = NULL;
msg.msg_controllen = 0;
msg.msg_namelen = 0;
msg.msg_flags = msg_flags | MSG_NOSIGNAL;


Of course it can be done directly from kernelspace without
touching process's structures, but this way is easier.

> RP

	Evgeniy Polyakov

Only failure makes us experts. -- Theo de Raadt

Re: [patch 1/2] fork_connector: add a fork connector

[Ram]

On Tue, 2005-03-22 at 12:25, Evgeniy Polyakov wrote:
> On Tue, 22 Mar 2005 11:18:07 -0800
> Ram <linuxram@us.ibm.com> wrote:
> 
> > > I still do not see why it is needed.
> > > Super-user can run ip command and turn network interface off
> > > not waiting while apache or named exits or unbind.
> > > 
> > > In theory I can create some kind of userspace registration mechanism,
> > > when userspace application reports it's pid to the connector, 
> > > and then it sends data to the specified pids, but does not 
> > > allow controlling from userspace.
> > > But I really do not think it is a good idea to permit
> > > non-priviledged userspace processes to know about deep
> > > kernel internals through connector's messages.
> > 
> > Yes. non-priviledged userspace processes should not know
> > any deep kernel internals through connector events.
> > 
> > I think what I am driving at is, an application that is critically
> > dependent on the fork-notification, suddenly stops receiving such
> > notification because some other application has switched off the 
> > service without its notice. 
> > 
> > the reason I am concerned is I am planning to feed this fork-events
> > to my in-kernel module. Side note: I would really like support for
> > in-kernel listners through connector infrastructure.
> 
> fork connector can be extended to send to the rest of the world
> information that it was turned off or on.

This will support the paradigm: "Let me know if this is switched off,
and I don't mind any arbitrary process switching it off".  No
applications can seriously rely on this service, because there is this
*arbritray-application-being-able-to-control* component to it.

A ideal paradigm would be: "Don't switch it off, without my consent".
Essentially this will give applications enough confidence to rely on it
seriously.

However an inbetween paradigm that would acceptable is:
"Let me know if this is swtiched off provided only application that I
can trust-on has the power to manage it".

To me 2nd or 3rd paradigm seems acceptable, but not the 1st.

> 
> The easiest way to listen inside the kernel for connector's 
> notifications is creating appropriate socket in userspace
> and then obtain it through the following code which must be
> run in context of the process that created above socket:
> 
> 	struct file *file;
> 	struct inode *inode;
> 	int err = 0;
> 	
> 	file = fget(userspace_socket_number_in_the_current_process_context);
> 	if (!file) {
> 		return -ENODEV;
> 	}
> 	inode = file->f_dentry->d_inode;
> 	if (inode->i_sock) {
> 		sock = SOCKET_I(inode);
> 		err = 0;
> 	} else {
> 		fput(file);
> 		err = -ENODEV;
> 	}
> 
> sock is struct sock, which then can be used for read/write
> operations using 
> kernel_recvmsg(sock, &msg, &iov, 1, size, 0);
> kernel_sendmsg(sock, &msg, &iov, 1, size);
> 
> with the following initialisation:
> 
> sock->sk->sk_allocation |= GFP_NOIO;
> iov.iov_base = buf;
> iov.iov_len = size;
> msg.msg_name = NULL;
> msg.msg_namelen = 0;
> msg.msg_control = NULL;
> msg.msg_controllen = 0;
> msg.msg_namelen = 0;
> msg.msg_flags = msg_flags | MSG_NOSIGNAL;
> 

right. thanks for the code snippet. I should be coding something
around these lines.
> 
> Of course it can be done directly from kernelspace without
> touching process's structures, but this way is easier.

Thanks,
RP

> 
> > RP
> 
> 	Evgeniy Polyakov
> 
> Only failure makes us experts. -- Theo de Raadt

Re: [patch 1/2] fork_connector: add a fork connector

[Evgeniy Polyakov]

[-- Attachment #1: Type: text/plain, Size: 3909 bytes --]

On Tue, 2005-03-22 at 12:42 -0800, Ram wrote:
> On Tue, 2005-03-22 at 12:25, Evgeniy Polyakov wrote:
> > On Tue, 22 Mar 2005 11:18:07 -0800
> > Ram <linuxram@us.ibm.com> wrote:
> > 
> > > > I still do not see why it is needed.
> > > > Super-user can run ip command and turn network interface off
> > > > not waiting while apache or named exits or unbind.
> > > > 
> > > > In theory I can create some kind of userspace registration mechanism,
> > > > when userspace application reports it's pid to the connector, 
> > > > and then it sends data to the specified pids, but does not 
> > > > allow controlling from userspace.
> > > > But I really do not think it is a good idea to permit
> > > > non-priviledged userspace processes to know about deep
> > > > kernel internals through connector's messages.
> > > 
> > > Yes. non-priviledged userspace processes should not know
> > > any deep kernel internals through connector events.
> > > 
> > > I think what I am driving at is, an application that is critically
> > > dependent on the fork-notification, suddenly stops receiving such
> > > notification because some other application has switched off the 
> > > service without its notice. 
> > > 
> > > the reason I am concerned is I am planning to feed this fork-events
> > > to my in-kernel module. Side note: I would really like support for
> > > in-kernel listners through connector infrastructure.
> > 
> > fork connector can be extended to send to the rest of the world
> > information that it was turned off or on.
> 
> This will support the paradigm: "Let me know if this is switched off,
> and I don't mind any arbitrary process switching it off".  No
> applications can seriously rely on this service, because there is this
> *arbritray-application-being-able-to-control* component to it.

It is fork connector who will inform all it's listeners that it is not
working any more.
And it is only super-user who can turn it off.

> A ideal paradigm would be: "Don't switch it off, without my consent".
> Essentially this will give applications enough confidence to rely on it
> seriously.

Does ip command wait until apache or bind exited when trying to turn 
network interface off? No. One can even unload network driver.
Because super-user must have ability to control the whole system.

> However an inbetween paradigm that would acceptable is:
> "Let me know if this is swtiched off provided only application that I
> can trust-on has the power to manage it".

I suppose fork connector [kernel module] is trusted enough application
that can inform external listeners about it's status.
But super-user still can turn it off by any program,
like one can turn network interface off using ip, ifconfig, your own 
application that can, btw, do it using netlink socket.

> To me 2nd or 3rd paradigm seems acceptable, but not the 1st.

Ok, fork module can implement following controlling mechanism:
on insert time super-user provides pid of the controlling 
program, later in fork callback it searches for the given pid, 
sends some signal to it, and if it receives 
magic message [with signal number for example] after it, then it is ok
to perform desired action.
I believe there are plenty of auth techniques that can be applied
here, but I strongly object against it.

What would happen with the system, if only one controlling daemon 
is allowed to turn networking off, or change hard drive work modes,
or super-user could not be allowed to remove user's file using usual
rm comman, but not specially authentificated daemon?
Or if it should ask all user's application, do they agree
to remove user's files?

> Thanks,
> RP
> 
> > 
> > > RP
> > 
> > 	Evgeniy Polyakov
> > 
> > Only failure makes us experts. -- Theo de Raadt
-- 
        Evgeniy Polyakov

Crash is better than data corruption -- Arthur Grabowski

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [patch 1/2] fork_connector: add a fork connector

[Jay Lan]

Ram wrote:
> On Tue, 2005-03-22 at 11:22, Evgeniy Polyakov wrote:
> 
>>On Tue, 22 Mar 2005 10:26:19 -0800
>>Ram <linuxram@us.ibm.com> wrote:
>>
>>
>>>On Mon, 2005-03-21 at 23:07, Guillaume Thouvenin wrote:
>>>
>>>>On Mon, 2005-03-21 at 12:52 -0800, Ram wrote:
>>>>
>>>>>     If a bunch of applications are listening for fork events, 
>>>>>     your patch allows any application to turn off the 
>>>>>     fork event notification?  Is this the right behavior?
>>>>
>>>>Yes it is. The main management is done by application so, if several
>>>>applications are listening for fork events you need to choose which one
>>>>will turn off the fork connector. 
>>>>
>>>>I want to keep this turn on/off mechanism simple but if it's needed I
>>>>can manage the variable "cn_fork_enable" as a counter. Thus the callback
>>>>could be something like:
>>>>
>>>>static void cn_fork_callback(void *data)
>>>>{
>>>>  int start; 
>>>>  struct cn_msg *msg = (struct cn_msg *)data;
>>>>
>>>>  if (cn_already_initialized && (msg->len == sizeof(cn_fork_enable))) {
>>>>    memcpy(&start, msg->data, sizeof(cn_fork_enable));
>>>>    if (start)
>>>>      cn_fork_enable++;
>>>>    else
>>>>      cn_fork_enable > 0 ? cn_fork_enable-- : 0;
>>>>  }
>>>>}
>>>
>>>I think a better way is:
>>>
>>>   Providing a different connector channel called the administrator 
>>>   channel which can be used only by a super-user, and gives you
>>>   the ability to switch on or off any connector channel including the
>>>   fork-connector channel.
>>
>>Only super-user can bind netlink socket to multicast group.
> 
> 
> ok. I did not realize that.
> 
> 
>>>   For lack of better term I am using the word 'channel' to mean
>>>   something that carries events of particular type through the
>>>   connector-infrastructure.
>>
>>I still do not see why it is needed.
>>Super-user can run ip command and turn network interface off
>>not waiting while apache or named exits or unbind.
>>
>>In theory I can create some kind of userspace registration mechanism,
>>when userspace application reports it's pid to the connector, 
>>and then it sends data to the specified pids, but does not 
>>allow controlling from userspace.
>>But I really do not think it is a good idea to permit
>>non-priviledged userspace processes to know about deep
>>kernel internals through connector's messages.
> 
> 
> Yes. non-priviledged userspace processes should not know
> any deep kernel internals through connector events.
> 
> I think what I am driving at is, an application that is critically
> dependent on the fork-notification, suddenly stops receiving such
> notification because some other application has switched off the 
> service without its notice. 
> 
> the reason I am concerned is I am planning to feed this fork-events
> to my in-kernel module. Side note: I would really like support for
> in-kernel listners through connector infrastructure.

Listners in the kernel? Sound like a PAGG thing again!

Guillaume's stuff was originally an in-kernel module. CSA/Job
are kernel modules also. We all use hook management feature of PAGG,
which offer event notification to other kernel components.

Guillaume moved to connector approach and i am moving to that
direction too because Andrew likes to see in-kernel modules moved
to user space.

It is kind of funny to see someone trying to get a notification
through a connector and then feed it back to the kernel! This
does not sound right to me!

- jay

> 
> RP
> 
> 
>>>RP
>>>
>>>
>>>
>>>>
>>>>What do you think about this implementation? 
>>>>
>>>>Guillaume
>>>>
>>
>>
>>	Evgeniy Polyakov
>>
>>Only failure makes us experts. -- Theo de Raadt

Re: [patch 1/2] fork_connector: add a fork connector

[Jay Lan]

Evgeniy Polyakov wrote:
> On Tue, 22 Mar 2005 10:26:19 -0800
> Ram <linuxram@us.ibm.com> wrote:
> 
> 
>>On Mon, 2005-03-21 at 23:07, Guillaume Thouvenin wrote:
>>
>>>On Mon, 2005-03-21 at 12:52 -0800, Ram wrote:
>>>
>>>>     If a bunch of applications are listening for fork events, 
>>>>     your patch allows any application to turn off the 
>>>>     fork event notification?  Is this the right behavior?
>>>
>>>Yes it is. The main management is done by application so, if several
>>>applications are listening for fork events you need to choose which one
>>>will turn off the fork connector. 
>>>
>>>I want to keep this turn on/off mechanism simple but if it's needed I
>>>can manage the variable "cn_fork_enable" as a counter. Thus the callback
>>>could be something like:
>>>
>>>static void cn_fork_callback(void *data)
>>>{
>>>  int start; 
>>>  struct cn_msg *msg = (struct cn_msg *)data;
>>>
>>>  if (cn_already_initialized && (msg->len == sizeof(cn_fork_enable))) {
>>>    memcpy(&start, msg->data, sizeof(cn_fork_enable));
>>>    if (start)
>>>      cn_fork_enable++;
>>>    else
>>>      cn_fork_enable > 0 ? cn_fork_enable-- : 0;
>>>  }
>>>}
>>
>>I think a better way is:
>>
>>   Providing a different connector channel called the administrator 
>>   channel which can be used only by a super-user, and gives you
>>   the ability to switch on or off any connector channel including the
>>   fork-connector channel.
> 
> 
> Only super-user can bind netlink socket to multicast group.
> 
> 
>>   For lack of better term I am using the word 'channel' to mean
>>   something that carries events of particular type through the
>>   connector-infrastructure.
> 
> 
> I still do not see why it is needed.
> Super-user can run ip command and turn network interface off
> not waiting while apache or named exits or unbind.

You can turn off a network interface and turn it back on without
closing a network application and it will continue to work.

> 
> In theory I can create some kind of userspace registration mechanism,
> when userspace application reports it's pid to the connector, 
> and then it sends data to the specified pids, but does not 
> allow controlling from userspace.
> But I really do not think it is a good idea to permit
> non-priviledged userspace processes to know about deep
> kernel internals through connector's messages.

I see this issue less a case of bad guys vs. good guys. I see it
as various components doing system related work, but there is
no mechanism of knowing who is on who is off by today's patch. A
service listening to the fork connector can request to turn off
cn_fork_enable on exit and inadquately affect other services/daemons
listening to the same connector. It is not acceptable in my opinion.

The idea of implementing fork connector enabling/disabling was so
that the kernel does not waste time writing to the sockets if no
application listening. If implementing such a feature costs
more than it saves, maybe the fork connector should simply always
send?

Thanks,
- jay


> 
> 
>>RP
>>
>>
>>
>>>
>>>What do you think about this implementation? 
>>>
>>>Guillaume
>>>
> 
> 
> 
> 	Evgeniy Polyakov
> 
> Only failure makes us experts. -- Theo de Raadt

Re: [patch 1/2] fork_connector: add a fork connector

[Evgeniy Polyakov]

[-- Attachment #1: Type: text/plain, Size: 2776 bytes --]

On Tue, 2005-03-22 at 15:51 -0800, Jay Lan wrote:

> >>I think a better way is:
> >>
> >>   Providing a different connector channel called the administrator 
> >>   channel which can be used only by a super-user, and gives you
> >>   the ability to switch on or off any connector channel including the
> >>   fork-connector channel.
> > 
> > 
> > Only super-user can bind netlink socket to multicast group.
> > 
> > 
> >>   For lack of better term I am using the word 'channel' to mean
> >>   something that carries events of particular type through the
> >>   connector-infrastructure.
> > 
> > 
> > I still do not see why it is needed.
> > Super-user can run ip command and turn network interface off
> > not waiting while apache or named exits or unbind.
> 
> You can turn off a network interface and turn it back on without
> closing a network application and it will continue to work.

And connector's listeners will continue to listen it's events.
But there will not be any event.
Like application will continue to be bound to the IP and listen
for incomming connection, but there will not be any connection.

> > 
> > In theory I can create some kind of userspace registration mechanism,
> > when userspace application reports it's pid to the connector, 
> > and then it sends data to the specified pids, but does not 
> > allow controlling from userspace.
> > But I really do not think it is a good idea to permit
> > non-priviledged userspace processes to know about deep
> > kernel internals through connector's messages.
> 
> I see this issue less a case of bad guys vs. good guys. I see it
> as various components doing system related work, but there is
> no mechanism of knowing who is on who is off by today's patch. A
> service listening to the fork connector can request to turn off
> cn_fork_enable on exit and inadquately affect other services/daemons
> listening to the same connector. It is not acceptable in my opinion.

It is super-user who only is allowed to turn it off and even listen for
events,
since only super-user is allowed to bind socket to multicast netlink
group.
Super-user should not be allowed to control it's system?

> The idea of implementing fork connector enabling/disabling was so
> that the kernel does not waste time writing to the sockets if no
> application listening. If implementing such a feature costs
> more than it saves, maybe the fork connector should simply always
> send?

With CBUS it costs nothing to always send notifications,
but I still do not see a point of disallowing super-user to stop
fork notifications using arbitrary application.

> Thanks,
> - jay

-- 
        Evgeniy Polyakov

Crash is better than data corruption -- Arthur Grabowski

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [patch 1/2] fork_connector: add a fork connector

[Ram]

On Tue, 2005-03-22 at 21:51, Evgeniy Polyakov wrote:
> On Wed, 2005-03-23 at 08:01 +0300, Evgeniy Polyakov wrote:
> > On Tue, 2005-03-22 at 15:51 -0800, Jay Lan wrote:
> > 
> 
> > > I see this issue less a case of bad guys vs. good guys. I see it
> > > as various components doing system related work, but there is
> > > no mechanism of knowing who is on who is off by today's patch. A
> > > service listening to the fork connector can request to turn off
> > > cn_fork_enable on exit and inadquately affect other services/daemons
> > > listening to the same connector. It is not acceptable in my opinion.
> > 
> > It is super-user who only is allowed to turn it off and even listen for
> > events,
> > since only super-user is allowed to bind socket to multicast netlink
> > group.
> > Super-user should not be allowed to control it's system?
> 
> BTW, super-user can unload fork connector module, and none listener
> will even know about it, it just stops to receive notification.

I see your point. Since the application has to be super-user to turn it
off, and since super-user applications are trusted not to mis-behave,
the current mechanism is relatively safe. I guess its the amount of
checks you put in place,  to prevent inadvertent shooting-in-the-foot.

There is nothing one can do if the fork_connector module is yanked out.
However there is something one can do, to prevent any arbitrary
application from shutting down the fork-event stream. I think I can live
with the current mechanism, under the assumption that no fork-event
listner has a legitate reason to shut down the fork-event-stream. 

RP

[patch 1/2] fork_connector: add a fork connector

[Guillaume Thouvenin]

  This patch adds a fork connector in the do_fork() routine. It sends a
netlink datagram when enabled. The message can be read by a user space
application. By this way, the user space application is alerted when a
fork occurs.

  It uses the userspace <-> kernelspace connector that works on top of
the netlink protocol. The fork connector is enabled or disabled by
sending a message to the connector. This operation should be done by
only one application. Such application can be downloaded from
http://cvs.sourceforge.net/viewcvs.py/elsa/elsa_project/utils/fcctl.c

  The unique sequence number of messages can be used to check if a
message is lost. This sequence number is relative to a CPU.

  The lmbench shows that the overhead (the construction and the sending
of the message) in the fork() routine is around 7%.

  This patch applies to 2.6.12-rc1-mm3. Some other patches are needed
that fix problems in the connector.c file. At least, you need to apply
the patch provided in the second email. 

Signed-off-by: Guillaume Thouvenin <guillaume.thouvenin@bull.net>
--- 

 drivers/connector/Kconfig   |   11 ++++
 drivers/connector/Makefile  |    1
 drivers/connector/cn_fork.c |  104 ++++++++++++++++++++++++++++++++++++++++++++
 include/linux/connector.h   |    8 +++
 kernel/fork.c               |   44 ++++++++++++++++++
 5 files changed, 168 insertions(+)


Index: linux-2.6.12-rc1-mm3-cnfork/drivers/connector/Kconfig
===================================================================
--- linux-2.6.12-rc1-mm3-cnfork.orig/drivers/connector/Kconfig	2005-03-25 09:47:09.000000000 +0100
+++ linux-2.6.12-rc1-mm3-cnfork/drivers/connector/Kconfig	2005-03-25 10:14:21.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
Index: linux-2.6.12-rc1-mm3-cnfork/drivers/connector/Makefile
===================================================================
--- linux-2.6.12-rc1-mm3-cnfork.orig/drivers/connector/Makefile	2005-03-25 09:47:09.000000000 +0100
+++ linux-2.6.12-rc1-mm3-cnfork/drivers/connector/Makefile	2005-03-25 10:14:21.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
Index: linux-2.6.12-rc1-mm3-cnfork/drivers/connector/cn_fork.c
===================================================================
--- linux-2.6.12-rc1-mm3-cnfork.orig/drivers/connector/cn_fork.c	2003-01-30 11:24:37.000000000 +0100
+++ linux-2.6.12-rc1-mm3-cnfork/drivers/connector/cn_fork.c	2005-03-25 10:14:21.000000000 +0100
@@ -0,0 +1,104 @@
+/*
+ * 	cn_fork.c
+ * 
+ * 2005 Copyright (c) Guillaume Thouvenin <guillaume.thouvenin@bull.net>
+ * 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 <guillaume.thouvenin@bull.net>");
+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 };
+
+static inline void cn_fork_send_status(void)
+{
+	/* TODO */
+	printk(KERN_INFO "cn_fork_enable == %d\n", cn_fork_enable);
+}
+
+/**
+ * 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;
+	int action;
+
+	if (cn_already_initialized && (msg->len == sizeof(cn_fork_enable))) {
+		memcpy(&action, msg->data, sizeof(cn_fork_enable));
+		switch(action) {
+			case FORK_CN_START:
+				cn_fork_enable = 1;
+				break;
+			case FORK_CN_STOP:
+				cn_fork_enable = 0;
+				break;
+			case FORK_CN_STATUS:
+				cn_fork_send_status();
+				break;
+		}
+	}
+}
+
+/**
+ * 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);
Index: linux-2.6.12-rc1-mm3-cnfork/include/linux/connector.h
===================================================================
--- linux-2.6.12-rc1-mm3-cnfork.orig/include/linux/connector.h	2005-03-25 09:47:11.000000000 +0100
+++ linux-2.6.12-rc1-mm3-cnfork/include/linux/connector.h	2005-03-25 10:14:21.000000000 +0100
@@ -28,10 +28,16 @@
 #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
 
+#define FORK_CN_STOP	0
+#define FORK_CN_START	1
+#define FORK_CN_STATUS	2
+
 struct cb_id
 {
 	__u32			idx;
@@ -133,6 +139,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 *);
Index: linux-2.6.12-rc1-mm3-cnfork/kernel/fork.c
===================================================================
--- linux-2.6.12-rc1-mm3-cnfork.orig/kernel/fork.c	2005-03-25 09:47:11.000000000 +0100
+++ linux-2.6.12-rc1-mm3-cnfork/kernel/fork.c	2005-03-25 10:14:21.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 DEFINE_PER_CPU(unsigned long, fork_counts);
+
+static inline void fork_connector(pid_t parent, pid_t child)
+{
+	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));
+		
+		msg->ack = 0; /* not used */
+		msg->seq = get_cpu_var(fork_counts)++;
+		
+		/* 
+		 * size of data is the number of characters 
+		 * printed plus one for the trailing '\0'
+		 */
+		memset(msg->data, '\0', CN_FORK_INFO_SIZE);
+		msg->len = scnprintf(msg->data, CN_FORK_INFO_SIZE-1, 
+				    "%i %i %i", 
+				    smp_processor_id(), parent, child) + 1;
+		
+		put_cpu_var(fork_counts);
+
+		cn_netlink_send(msg, CN_IDX_FORK);
+	}
+}
+#else
+static inline void fork_connector(pid_t parent, pid_t child) 
+{
+	return; 
+}
+#endif /* CONFIG_FORK_CONNECTOR */
+
 int nr_processes(void)
 {
 	int cpu;
@@ -1253,6 +1295,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);

Re: [patch 1/2] fork_connector: add a fork connector

[dean gaudet]

On Fri, 25 Mar 2005, Guillaume Thouvenin wrote:

...
>   The lmbench shows that the overhead (the construction and the sending
> of the message) in the fork() routine is around 7%.
...
> +		/* 
> +		 * size of data is the number of characters 
> +		 * printed plus one for the trailing '\0'
> +		 */
> +		memset(msg->data, '\0', CN_FORK_INFO_SIZE);
> +		msg->len = scnprintf(msg->data, CN_FORK_INFO_SIZE-1, 
> +				    "%i %i %i", 
> +				    smp_processor_id(), parent, child) + 1;

i'm certain that if you used a struct {} and filled in 3 fields rather 
than zeroing 64 bytes of memory, and doing 3 conversions to decimal ascii 
then you'd see a marked decrease in the overhead of this.  it's not clear 
to me why you need ascii here -- the rest of the existing bsd accounting 
code is not ascii (i'm assuming the purpose of the fork connector is for 
accounting).

-dean

Re: [patch 1/2] fork_connector: add a fork connector

[Paul Jackson]

Guillaume wrote:
>   The lmbench shows that the overhead (the construction and the sending
> of the message) in the fork() routine is around 7%.

Thanks for including the numbers.  The 7% seems a bit costly, for a bit
more accounting information.  Perhaps dean's suggestion, to not use
ascii, will help.  I hope so, though I doubt it will make a huge
difference.  Was this 7% loss with or without a user level program
consuming the sent messages?  I would think that the number of interest
would include a minimal consumer task.

I don't see a good reason to make fork_connector() inline.  Since it
calls other subroutines and is not just a few lines, perhaps better to
make it a real routine, so we can see it in "nm --print-size" output and
debug stacks.

Having the "#ifdef CONFIG_FORK_CONNECTOR" chunk of code right in fork.c
seems unfortunate.  Can the real fork_connector() be put elsewhere, and
the ifdef put in a header file that makes it a no-op if not configured,
or simply a function declaration, if configured?

What's the status of the connector driver patch?  I perhaps wasn't
paying close enough attention, but all I see of it now is a couple of
patches sent to lkml, from Evgeniy Polyakov, in September and January.
I don't see it in my copies of *-mm or recent Linus bk trees.  Am I
missing something?

This still seems to me like more apparatus than is desirable, just to
get another form of session id, as best as I can figure it.  However
we've already been there, and apparently my concerns were not
persuasive.  If one does go down this path, then using this connector
patch is a good an alternative as any I know of.  Well, that or relayfs.
My uneducated assumption is that relayfs might at least batch data
packets up into big buffer chunks better, but someone more knowledgeable
than me needs to consider that.

It's a little sad, when almost all the required accounting information
comes out in packed 64 byte records, carefully buffered and sent in
big chunks, to minimize per-task costs.  Then this one extra detail,
of <parent-pid, child-pid> requires an entire netlink packet of
its own of what size -- another 50 or 100 bytes?  Is this packet
received as a separate data packet, on its own recv(2) system call,
by the user task, not in a big block of packets?  The efficiency
of getting this one extra <parent-pid, child-pid> out of the kernel
seems to be one or two orders of magnitude worse than the rest of
the accounting data.

===

Hmmm ... perhaps one could add a _second_ accounting file, cutting and
pasting code in kernel/acct.c and enabling writing additional
information to that second file, using the same mechanisms as now used
for the primary file.  Use a more extensible record format for the
second file (say start each record with a magic cookie, a byte record
type and a byte record length, then that many bytes).  That way, we have
an escape valve for adding additional record types in the future.
And that way we can efficiently write short records, with just say
a couple of interesting values, and minimal overhead.

Don't worry if the magic cookie appears as part of the raw data.  If one
has to resync such a data stream, one can look for a series of records,
each starting with the magic cookie, sensible record type byte, and a
length that ends right at the next such valid record.  The occassional
duplication of the same cookie within the data stream would not thwart a
resync for long.  And the main purpose of the magic cookie is to make
sure you are still in sync, not reverting to garbage-in, garbage-out,
mode.  Almost any magic value other than 0x0000 will suffice for that
purpose.

I just ran a silly little test on my PC desktop Linux box, scanning
/proc/kcore.  The _least_ common 2 byte word seen was 0x2B91, with 31
instances in a half-billion words scanned, so I nominate that value for
the magic cookie ;).

The key reason that it might make sense here to adapt the existing
accounting file direct write mechanism, rather than using "connector" or
"relayfs", is that we really do want to get this data to disk initially.
Relayfs is optimized for getting alot of data to a user daemon, and the
connector for sending smaller packets of data to a user daemon.  But
accounting processing is sometimes done out of a cron job off-hours.
During the day (the busy hours) you might just want to stash the stuff
with as little performance impact is possible.  If one can avoid _any_
other task having to context switch in, in order to get this data on its
way, that is a huge win.

-- 
                  I won't rest till it's the best ...
                  Programmer, Linux Scalability
                  Paul Jackson <pj@engr.sgi.com> 1.650.933.1373, 1.925.600.0401

Re: [patch 1/2] fork_connector: add a fork connector

[Evgeniy Polyakov]

[-- Attachment #1: Type: text/plain, Size: 6040 bytes --]

On Mon, 2005-03-28 at 13:42 -0800, Paul Jackson wrote:
> Guillaume wrote:
> >   The lmbench shows that the overhead (the construction and the sending
> > of the message) in the fork() routine is around 7%.
> 
> Thanks for including the numbers.  The 7% seems a bit costly, for a bit
> more accounting information.  Perhaps dean's suggestion, to not use
> ascii, will help.  I hope so, though I doubt it will make a huge
> difference.  Was this 7% loss with or without a user level program
> consuming the sent messages?  I would think that the number of interest
> would include a minimal consumer task.

There is no overhead at all using CBUS.
On my old P2/256mb SMP machine it took about 950 usec 
to create+exit process both with fork connector turned on and 
without it even compiled.
Direct connector's method call took about 1000-1100 usec.
Current fork connector does not use CBUS [yet, I hope].

> I don't see a good reason to make fork_connector() inline.  Since it
> calls other subroutines and is not just a few lines, perhaps better to
> make it a real routine, so we can see it in "nm --print-size" output and
> debug stacks.
> 
> Having the "#ifdef CONFIG_FORK_CONNECTOR" chunk of code right in fork.c
> seems unfortunate.  Can the real fork_connector() be put elsewhere, and
> the ifdef put in a header file that makes it a no-op if not configured,
> or simply a function declaration, if configured?
> 
> What's the status of the connector driver patch?  I perhaps wasn't
> paying close enough attention, but all I see of it now is a couple of
> patches sent to lkml, from Evgeniy Polyakov, in September and January.
> I don't see it in my copies of *-mm or recent Linus bk trees.  Am I
> missing something?

It was dropped from -mm tree, since bk tree where it lives 
was in maintenance mode.
I think connector will be appeared in the next -mm release.

> This still seems to me like more apparatus than is desirable, just to
> get another form of session id, as best as I can figure it.  However
> we've already been there, and apparently my concerns were not
> persuasive.  If one does go down this path, then using this connector
> patch is a good an alternative as any I know of.  Well, that or relayfs.
> My uneducated assumption is that relayfs might at least batch data
> packets up into big buffer chunks better, but someone more knowledgeable
> than me needs to consider that.
> 
> It's a little sad, when almost all the required accounting information
> comes out in packed 64 byte records, carefully buffered and sent in
> big chunks, to minimize per-task costs.  Then this one extra detail,
> of <parent-pid, child-pid> requires an entire netlink packet of
> its own of what size -- another 50 or 100 bytes?  Is this packet
> received as a separate data packet, on its own recv(2) system call,
> by the user task, not in a big block of packets?  The efficiency
> of getting this one extra <parent-pid, child-pid> out of the kernel
> seems to be one or two orders of magnitude worse than the rest of
> the accounting data.

It can be easily changed.
One may send kernel/acct.c acct_t structure out of the kernel - 
overhead will be the same: kmalloc probably will get new area from the
same 256-bytes pool, skb is still in cache.

> ===
> 
> Hmmm ... perhaps one could add a _second_ accounting file, cutting and
> pasting code in kernel/acct.c and enabling writing additional
> information to that second file, using the same mechanisms as now used
> for the primary file.  Use a more extensible record format for the
> second file (say start each record with a magic cookie, a byte record
> type and a byte record length, then that many bytes).  That way, we have
> an escape valve for adding additional record types in the future.
> And that way we can efficiently write short records, with just say
> a couple of interesting values, and minimal overhead.
> 
> Don't worry if the magic cookie appears as part of the raw data.  If one
> has to resync such a data stream, one can look for a series of records,
> each starting with the magic cookie, sensible record type byte, and a
> length that ends right at the next such valid record.  The occassional
> duplication of the same cookie within the data stream would not thwart a
> resync for long.  And the main purpose of the magic cookie is to make
> sure you are still in sync, not reverting to garbage-in, garbage-out,
> mode.  Almost any magic value other than 0x0000 will suffice for that
> purpose.
> 
> I just ran a silly little test on my PC desktop Linux box, scanning
> /proc/kcore.  The _least_ common 2 byte word seen was 0x2B91, with 31
> instances in a half-billion words scanned, so I nominate that value for
> the magic cookie ;).
> 
> The key reason that it might make sense here to adapt the existing
> accounting file direct write mechanism, rather than using "connector" or
> "relayfs", is that we really do want to get this data to disk initially.
> Relayfs is optimized for getting alot of data to a user daemon, and the
> connector for sending smaller packets of data to a user daemon.  But
> accounting processing is sometimes done out of a cron job off-hours.
> During the day (the busy hours) you might just want to stash the stuff
> with as little performance impact is possible.  If one can avoid _any_
> other task having to context switch in, in order to get this data on its
> way, that is a huge win.

File writing accounting [kernel/acct.c] is slower, it takes global
locks 
and requires process' context to work with system calls.
realyfs is interesting project, but it has different aims, 
as far as I can see, - it is created for transferring huge amounts of
data,
and it is succeded in it, while connector is purely
control/notification 
mechanism, for example, for gathering short-living per-process
accounting data.

-- 
        Evgeniy Polyakov

Crash is better than data corruption -- Arthur Grabowski

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [patch 1/2] fork_connector: add a fork connector

[Greg KH]

On Tue, Mar 29, 2005 at 11:04:16AM +0400, Evgeniy Polyakov wrote:
> On Mon, 2005-03-28 at 13:42 -0800, Paul Jackson wrote:
> > I don't see it in my copies of *-mm or recent Linus bk trees.  Am I
> > missing something?
> 
> It was dropped from -mm tree, since bk tree where it lives 
> was in maintenance mode.
> I think connector will be appeared in the next -mm release.

Should have been in the last -mm release.  If not, please let me know.

thanks,

greg k-h

Re: [patch 1/2] fork_connector: add a fork connector

[Evgeniy Polyakov]

[-- Attachment #1: Type: text/plain, Size: 766 bytes --]

On Mon, 2005-03-28 at 23:02 -0800, Greg KH wrote:
> On Tue, Mar 29, 2005 at 11:04:16AM +0400, Evgeniy Polyakov wrote:
> > On Mon, 2005-03-28 at 13:42 -0800, Paul Jackson wrote:
> > > I don't see it in my copies of *-mm or recent Linus bk trees.  Am I
> > > missing something?
> > 
> > It was dropped from -mm tree, since bk tree where it lives 
> > was in maintenance mode.
> > I think connector will be appeared in the next -mm release.
> 
> Should have been in the last -mm release.  If not, please let me know.

Thank you.
If you are not going to sleep right now I will recreate rejected
NLMSGSPACE
patch in a several minutes.

> thanks,
> 
> greg k-h
-- 
        Evgeniy Polyakov

Crash is better than data corruption -- Arthur Grabowski

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [patch 1/2] fork_connector: add a fork connector

[Paul Jackson]

Evgeniy wrote:
> There is no overhead at all using CBUS.

This is unlikely.  Very unlikely.

Please understand that I am not trying to critique CBUS or connector in
isolation, but rather trying to determine what mechanism is best suited
for getting this accounting data written to disk, which is where I
assume it has to go until some non-real time job gets around to
analyzing it.  We already have the rest of the BSD Accounting
information taking this batched up path directly to the disk.  There is
nothing (that I know of) to be gained from delivering this new fork data
with any higher quality of service, or to any other place.

>From what I can understand, correct me if I'm wrong, we have two
alternatives in front of us (ignoring relayfs for a second):

 1) Using fork_connector (presumably soon to include use of CBUS):
	- forking process enqueues data plus header for single fork
	- context switch
	- daemon process dequeues single fork data (is this a read or recv?)
	- daemon process mergers multiple fork data into single buffer
	- daemon process writes buffer for multiple forks (a write)
	- disk driver pushes buffer with data for multiple forks to disk

 2) Using a modified form of what BSD ACCOUNTING does now:
	- forking process appends single fork data to in-kernel buffer
	- disk driver pushes buffer with data for multiple forks to disk

It seems to me to be rather unlikely that (1) is cheaper than (2).  It
is no particular fault of connector or CBUS that this is so.  Even if
there were no overhead at all using CBUS (which I don't believe), (1)
still costs more, because it passes the data, with an added packet
header, into a separate process, and into user space, before it is
combined with other accounting information, and written back down into
the kernel to go to the disk.


> > ... The efficiency
> > of getting this one extra <parent-pid, child-pid> out of the kernel
> > seems to be one or two orders of magnitude worse than the rest of
> > the accounting data.
> 
> It can be easily changed.
> One may send kernel/acct.c acct_t structure out of the kernel - 
> overhead will be the same: kmalloc probably will get new area from the
> same 256-bytes pool, skb is still in cache.

I have no idea what you just said.


> File writing accounting [kernel/acct.c] is slower,

Sure file writing is slower than queuing on an internal list.  I don't
care that getting the data where I want it is slower than getting it
some other place that's only part way.


> and requires process' context to work with system calls.

For some connector uses, that might matter.  For hooks in fork,
that is no problem - we have all the process context one could
want - two of them if that helps ;).


> realyfs is interesting project, but it has different aims, 

That could well be ... I can't claim to know which of relayfs or
connector would be better here, of the two.


> while connector is purely control/notification mechanism

However connector is, in this regard, overkill.  We don't need a single
event notification mechanism here.  One of the key ways in which
accounting such as this has historically minimized system load is to
forgo any effort to provide any notification or data packet per event,
and instead immediately work to batch the data up in bulk form, with one
buffer containing the concatenated data for multiple events.  This
amortizes the cost of almost all the handling, and of all the disk i/o,
over many data collection events.  Correct me if I'm wrong, but
fork_connector doesn't do this merging of events into a consolidated
data buffer, so is at a distinct disadvantage, for this use, because the
data merging is delayed, and a separate, user level process, is required
to accomplish the merging and conversion to writable blocks of data
suitable for storing on the disk.

Nothing wrong with a good screwdriver.  But if you want to pound nails,
hammers, even rocks, work better.

-- 
                  I won't rest till it's the best ...
                  Programmer, Linux Scalability
                  Paul Jackson <pj@engr.sgi.com> 1.650.933.1373, 1.925.600.0401

Re: [patch 1/2] fork_connector: add a fork connector

[Guillaume Thouvenin]

On Tue, 2005-03-29 at 00:49 -0800, Paul Jackson wrote:
>   This
> amortizes the cost of almost all the handling, and of all the disk i/o,
> over many data collection events.  Correct me if I'm wrong, but
> fork_connector doesn't do this merging of events into a consolidated
> data buffer, so is at a distinct disadvantage, for this use, because the
> data merging is delayed, and a separate, user level process, is required
> to accomplish the merging and conversion to writable blocks of data
> suitable for storing on the disk.

  The goal of the fork connector is to inform a user space application
that a fork occurs in the kernel. This information (cpu ID, parent PID
and child PID) can be used by several user space applications. It's not
only for accounting. Accounting and fork_connector are two different
things and thus, fork_connector doesn't do the merge of any kinds of
data (and it will never do). 

  One difference with relayfs is the amount of datas that are
transfered. The relayfs is done, like Evgeniy said, for large amount of
datas. So I think that it's not suitable for what we want to achieve
with the fork connector.


I hope this help,
Regards,
Guillaume

Re: [patch 1/2] fork_connector: add a fork connector

[Paul Jackson]

Guillaume wrote:
>   The goal of the fork connector is to inform a user space application
> that a fork occurs in the kernel. This information (cpu ID, parent PID
> and child PID) can be used by several user space applications. It's not
> only for accounting. Accounting and fork_connector are two different
> things and thus, fork_connector doesn't do the merge of any kinds of
> data (and it will never do). 

Yes - it is clear that the fork_connector does this - inform user space
of fork information <cpu, parent, child>.  I'm not saying that
fork_connector should merge data; I'm observing that it doesn't, and
that this would seem to serve the needs of accounting poorly.

Out of curiosity, what are these 'several user space applications?'  The
only one I know of is this extension to bsd accounting to include
capturing parent and child pid at fork.  Probably you've mentioned some
other uses of fork_connector before here, but I missed it.

> The relayfs is done, like Evgeniy said, for large amount of
> datas. So I think that it's not suitable for what we want to achieve
> with the fork connector.

I never claimed that relayfs was appropriate for fork_connector.

I'm not trying to tape a rock to Evgeniy's screwdriver.  I'm saying that
accounting looks like a nail to me, so let us see what rocks and hammers
we have in our tool box.

-- 
                  I won't rest till it's the best ...
                  Programmer, Linux Scalability
                  Paul Jackson <pj@engr.sgi.com> 1.650.933.1373, 1.925.600.0401

Re: [patch 1/2] fork_connector: add a fork connector

[Jay Lan]

Paul Jackson wrote:
> Guillaume wrote:
> 
>>  The goal of the fork connector is to inform a user space application
>>that a fork occurs in the kernel. This information (cpu ID, parent PID
>>and child PID) can be used by several user space applications. It's not
>>only for accounting. Accounting and fork_connector are two different
>>things and thus, fork_connector doesn't do the merge of any kinds of
>>data (and it will never do). 
> 
> 
> Yes - it is clear that the fork_connector does this - inform user space
> of fork information <cpu, parent, child>.  I'm not saying that
> fork_connector should merge data; I'm observing that it doesn't, and
> that this would seem to serve the needs of accounting poorly.

Paul,

You probably can look at it this way: the accounting data being
written out by BSD are per process data and the fork connector
provides information needed to group processes into process
aggregates.

Thanks,
  - jay

> 
> Out of curiosity, what are these 'several user space applications?'  The
> only one I know of is this extension to bsd accounting to include
> capturing parent and child pid at fork.  Probably you've mentioned some
> other uses of fork_connector before here, but I missed it.
> 
> 
>>The relayfs is done, like Evgeniy said, for large amount of
>>datas. So I think that it's not suitable for what we want to achieve
>>with the fork connector.
> 
> 
> I never claimed that relayfs was appropriate for fork_connector.
> 
> I'm not trying to tape a rock to Evgeniy's screwdriver.  I'm saying that
> accounting looks like a nail to me, so let us see what rocks and hammers
> we have in our tool box.
> 

Re: [patch 1/2] fork_connector: add a fork connector

[Paul Jackson]

[ Hmmm .. the following pertains more to accounting than to fork_connector,
  as have my other remarks earlier today.  I notice just now I am on a thread
  whose Subject is "fork_connector".   Oh well.  Sorry.          - pj ]

Jay wrote:
> You probably can look at it this way: the accounting data being
> written out by BSD are per process data and the fork connector
> provides information needed to group processes into process
> aggregates.

I guess so.  Though that doesn't provide any explicit guidance as to
what the necessary dataflow must be -- who (which essential piece(s) of
software) needs the data when, to accomplish what purposes that some
Linux users will desire.

Well, maybe to someone expert in Process Aggregates, it provides
such guidance, implicitly.  That's definitely not I.

  Let me step back a minute here.

  What's needed to is work from the actual user requirements down
  to what technical pieces are needed.  There's an old saying
  that if you want something done bad enough, do it yourself.

  Or, on usenet and now on mailing lists, this has become:
  if you want something done, post a sufficiently botched
  example yourself, and someone who actually knows will become
  sufficiently annoyed to post a useful answer.

  So here goes my botched effort to work from user requirements
  down to actual technical pieces needed.  I look forward to
  being shot down in flames.

My current understanding of the 'system accounting' requirement
is that users of large shared resource servers want to determine,
after the fact, what was the usage by or for various
tasks/jobs/users/groups/time-periods of various compute
resources, in order to perform such tasks as billing and sizing
of future equipment needs, and to identify patterns of over or
under utilized system resources that might present other
opportunities for useful action, or causes for remedial action.

I am working under the assumption that there is some accounting
(of computer users and resources, not of money ;) software
(runnacct, CSA, and ELSA, for example) that runs, after the fact
in some post-processing mode, that reads records of actual usage
details from disk files and does useful stuff (like generate
reports useful to the above requirements) with what it can glean
from those records and from other configuration information
it can find about the current system (by reading other disk
files, typically).  This processing can be and often is done
in batch mode, and is often scheduled out of a cron job for some
time when the system is normally under relatively lighter load,
such as late at night.

I assume that the information needed by this accounting software
includes both the classic BSD accounting records and the
<parent_pid, child_pid> information at fork.

I am not aware of any other uses of the <parent_pid, child_pid>
information from fork, though it would not surprise me to learn
that there other such uses - you're welcome to educate me on
this matter.

I suspect that there is other information, or will be, in
addition to the specific details collected by the classic bsd
accounting kernel hooks, and in addition to the <parent_pid,
child_pid> information at fork, which will also be needed by CSA
and/or ELSA, and which also needs to be written to disk files
as the data is collected, for subsequent processing by such
accounting software as CSA and ELSA, or the classic runacct(1M)
daily accounting software and variants.

If the above is all true, then the basic problem to solve
regarding the <parent_pid, child_pid> information collected at
fork is how to get it into a disk file, with close to minimum
impact on the system.

Since the data is not needed in anything like realtime (or
if it is, I don't realize that yet) therefore there is an
opportunity to combine the data records into buffers of data,
so as to amortize some of the costs of writing the data to
disk over several records.  The classic bsd accounting hooks
do this merging aggressively, in the context of the process
doing the exit.

The classic accounting hooks may have a problem that they are not
NUMA friendly - having all the nodes in a big system trying to
simultaneously add small (64 bytes, typically) snippets to the
same shared file buffers at the same time might not scale well.
These hooks were designed over 25 years ago, when multiprocessing
was in its infancy, and may need overhaul.

The fork_connector mechanism is being proposed to get the
particular bit of information <parent_pid, child_pid> from
fork moved to what I presume is a data collector daemon user
process, which will I presume then write merged records of
this data to disk.  This may have the problem that it moves
the individual records between various contexts on the system,
more than is necessary, before it can be merged into buffers
and written.  While such data motion does not happen inline
to the fork itself, it still has to occur in near realtime
(minutes) of the fork event, so still impacts system performance
(both CPU cycles and memory footprint) during peak usage hours.
Performance impact numbers have been presented to show that
this impact is minimal, but my direct question as to whether
these numbers include the load of the data collector and writer
daemon has gone unanswered, so far as I know.

So this is what I understand the problems and the requirements
to be.

Most likely I misunderstand important parts of this - I invite
corrections.  I don't actually have a horse in this race; I'm just
doing the color commentary.  So if someone wants to rip this apart,
then go for it.  I will enjoy the show along with everyone else.

-- 
                  I won't rest till it's the best ...
                  Programmer, Linux Scalability
                  Paul Jackson <pj@engr.sgi.com> 1.650.933.1373, 1.925.600.0401

Re: [patch 1/2] fork_connector: add a fork connector

[Guillaume Thouvenin]

On Tue, 2005-03-29 at 07:23 -0800, Paul Jackson wrote:
> Guillaume wrote:
> >   The goal of the fork connector is to inform a user space application
> > that a fork occurs in the kernel. This information (cpu ID, parent PID
> > and child PID) can be used by several user space applications. It's not
> > only for accounting. Accounting and fork_connector are two different
> > things and thus, fork_connector doesn't do the merge of any kinds of
> > data (and it will never do). 
> 
> Yes - it is clear that the fork_connector does this - inform user space
> of fork information <cpu, parent, child>.  I'm not saying that
> fork_connector should merge data; I'm observing that it doesn't, and
> that this would seem to serve the needs of accounting poorly.
> 
> Out of curiosity, what are these 'several user space applications?'  The
> only one I know of is this extension to bsd accounting to include
> capturing parent and child pid at fork.  Probably you've mentioned some
> other uses of fork_connector before here, but I missed it.

During the discussion some people like Erich Focht and Ram mentioned
that this information can be useful for them. I remember that Erich had
in mind something like cluster-wide pid tracking in user space. 

When I wrote "several user space applications" it was just to say that
this fork connector is not designed only for ELSA and fork information
is available to every listeners.

Regards,
Guillaume

Re: [patch 1/2] fork_connector: add a fork connector

[Paul Jackson]

Guillaume wrote:
> When I wrote "several user space applications" it was just to say that
> this fork connector is not designed only for ELSA and fork information
> is available to every listeners.

So I suppose if fork_connector were not used to collect <parent pid,
child pid> information for accounting, then someone would have to make
the case that there were enough other uses, of sufficient value, to add
fork_connector.  We have to be a bit careful, in the kernel, to avoid
adding mechanisms until we have the immediate use in hand.  If we don't
do this, then the kernel ends up looking like the Gargoyles on a
Renaissance church - burdened with overly ornate features serving no
earthly purpose.

-- 
                  I won't rest till it's the best ...
                  Programmer, Linux Scalability
                  Paul Jackson <pj@engr.sgi.com> 1.650.933.1373, 1.925.600.0401

Re: [patch 1/2] fork_connector: add a fork connector

[Herbert Xu]

Paul Jackson <pj@engr.sgi.com> wrote:
> 
> So I suppose if fork_connector were not used to collect <parent pid,
> child pid> information for accounting, then someone would have to make
> the case that there were enough other uses, of sufficient value, to add
> fork_connector.  We have to be a bit careful, in the kernel, to avoid
> adding mechanisms until we have the immediate use in hand.  If we don't
> do this, then the kernel ends up looking like the Gargoyles on a
> Renaissance church - burdened with overly ornate features serving no
> earthly purpose.

I agree completely.  In fact the whole drivers/connector directory
looks pretty suspect.  Are there any in-kernel users of it at all?
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [patch 1/2] fork_connector: add a fork connector

[Evgeniy Polyakov]

[-- Attachment #1: Type: text/plain, Size: 990 bytes --]

On Wed, 2005-03-30 at 20:25 +1000, Herbert Xu wrote:
> Paul Jackson <pj@engr.sgi.com> wrote:
> > 
> > So I suppose if fork_connector were not used to collect <parent pid,
> > child pid> information for accounting, then someone would have to make
> > the case that there were enough other uses, of sufficient value, to add
> > fork_connector.  We have to be a bit careful, in the kernel, to avoid
> > adding mechanisms until we have the immediate use in hand.  If we don't
> > do this, then the kernel ends up looking like the Gargoyles on a
> > Renaissance church - burdened with overly ornate features serving no
> > earthly purpose.
> 
> I agree completely.  In fact the whole drivers/connector directory
> looks pretty suspect.  Are there any in-kernel users of it at all?

SuperIO subsystem.
In agenda sit w1, acrypto [but it already looks like it will not be
included :) ].

-- 
        Evgeniy Polyakov

Crash is better than data corruption -- Arthur Grabowski

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [patch 1/2] fork_connector: add a fork connector

[Guillaume Thouvenin]

On Wed, 2005-03-30 at 20:25 +1000, Herbert Xu wrote:
> Paul Jackson <pj@engr.sgi.com> wrote:
> > 
> > So I suppose if fork_connector were not used to collect <parent pid,
> > child pid> information for accounting, then someone would have to make
> > the case that there were enough other uses, of sufficient value, to add
> > fork_connector.  We have to be a bit careful, in the kernel, to avoid
> > adding mechanisms until we have the immediate use in hand.  If we don't
> > do this, then the kernel ends up looking like the Gargoyles on a
> > Renaissance church - burdened with overly ornate features serving no
> > earthly purpose.
> 
> I agree completely.  In fact the whole drivers/connector directory
> looks pretty suspect.  Are there any in-kernel users of it at all?

There is the Enhanced Linux System Accounting project 
http://elsa.sourceforge.net

Guillaume

Re: [patch 1/2] fork_connector: add a fork connector

[Drew Hess]

On Tue, 29 Mar 2005 07:23:35 -0800, Paul Jackson <pj@engr.sgi.com> wrote:

> Out of curiosity, what are these 'several user space applications?'  The
> only one I know of is this extension to bsd accounting to include
> capturing parent and child pid at fork.  Probably you've mentioned some
> other uses of fork_connector before here, but I missed it.


I have a user-space batch job scheduler that could use fork_connector
to track which processes belong to a job.  It looks perfect for what I
need.

I would also like to see a do_exit hook, but only as a convenience. I
can probably scrape the BSD accounting files in lieu of a do_exit
hook, but if I had one, I wouldn't need to touch disk for my job
accounting.

d

Re: [patch 1/2] fork_connector: add a fork connector

[Evgeniy Polyakov]

[-- Attachment #1: Type: text/plain, Size: 7782 bytes --]

On Tue, 2005-03-29 at 00:49 -0800, Paul Jackson wrote:
> Evgeniy wrote:
> > There is no overhead at all using CBUS.
> 
> This is unlikely.  Very unlikely.
> 
> Please understand that I am not trying to critique CBUS or connector in
> isolation, but rather trying to determine what mechanism is best suited
> for getting this accounting data written to disk, which is where I
> assume it has to go until some non-real time job gets around to
> analyzing it.  We already have the rest of the BSD Accounting
> information taking this batched up path directly to the disk.  There is
> nothing (that I know of) to be gained from delivering this new fork data
> with any higher quality of service, or to any other place.
> 
> From what I can understand, correct me if I'm wrong, we have two
> alternatives in front of us (ignoring relayfs for a second):
> 
>  1) Using fork_connector (presumably soon to include use of CBUS):
> 	- forking process enqueues data plus header for single fork

Here forking connector module "exits" and can handle next fork() on the
same CPU.
Different CPUs are handled independently due to per-cpu variables usage.

That is why it is very fast in "fast-path".

> 	- context switch
> 	- daemon process dequeues single fork data (is this a read or recv?)
> 	- daemon process mergers multiple fork data into single buffer
> 	- daemon process writes buffer for multiple forks (a write)
> 	- disk driver pushes buffer with data for multiple forks to disk

Not exactly.

 - context switch
 - CBUS daemon, which runs with +19 nice get a bunch [10 currently, nice
value and queue "length" are determined in experiments] of requests from
each CPU queue and send them using connector's cn_netlink_send.
 - cn_netlink_send reallocates a buffer for each message 
[skb + allocation from 256-bytes pool in kmalloc],
walks through list of registered sockets and links given skb to the
requested socket queues.
 - context switch
 - userspace daemon is awakened from recv() syscall and does what it
want with provided data.
It can write it to disk, but may process in real-time or send to the
network.

The most expensive part is cn_netlink_send()/netlink_broadcast(), 
with CBUS it is deferred to the safe time,
so fork() itself is not affected (it is only per-cpu locking + linking +
atomic allocation).
Since deferred message will be processed in "safe" time with low
priority, it should not 
affect fork() too (but can).

>  2) Using a modified form of what BSD ACCOUNTING does now:
> 	- forking process appends single fork data to in-kernel buffer

It is not as simple.
It takes global locks several times, it access bunch of shared between
CPU data.
It calls ->stat() and ->write() which may sleep.

> 	- disk driver pushes buffer with data for multiple forks to disk


Here is the same deffering as in connector, only preparation is
different.
And acct.c preparation may sleep and works with shared objects and
locks,
so it is slower, but it has an advantage - data is already written to
the 
storage.

> It seems to me to be rather unlikely that (1) is cheaper than (2).  It
> is no particular fault of connector or CBUS that this is so.  Even if
> there were no overhead at all using CBUS (which I don't believe), (1)
> still costs more, because it passes the data, with an added packet
> header, into a separate process, and into user space, before it is
> combined with other accounting information, and written back down into
> the kernel to go to the disk.

That work is deferred and does not affect in-kernel processes.
And why userspace fork connector should write data to the disk?
It can process it in-flight and write only results.
acct.c processing daemon needs to read data, i.e. transfer them from
kernelspace.
But again all that work is deferred and does not affect fork()
performance.

> 
> > > ... The efficiency
> > > of getting this one extra <parent-pid, child-pid> out of the kernel
> > > seems to be one or two orders of magnitude worse than the rest of
> > > the accounting data.
> > 
> > It can be easily changed.
> > One may send kernel/acct.c acct_t structure out of the kernel - 
> > overhead will be the same: kmalloc probably will get new area from the
> > same 256-bytes pool, skb is still in cache.
> 
> I have no idea what you just said.

Connector's overhead may come from memory allocation - 
currently it calls alloc_skb(size, GFP_ATOMIC), skb allocation
calls kmem_cache_alloc() for skb itself and kmalloc() for
size + sizeof(struct skb_shared_info), which essentially 
is allocation from the 256-bytes slab pool.

> 
> > File writing accounting [kernel/acct.c] is slower,
> 
> Sure file writing is slower than queuing on an internal list.  I don't
> care that getting the data where I want it is slower than getting it
> some other place that's only part way.

One needs to pay for speed.
In case of the acct.c price is high since writing is slow, 
but one does not need to care about receiving part.
In the case of connector, price is low, but that requires
some additional process to fetch the data.

For some tasks one may be better than other, for others it is not.

> > and requires process' context to work with system calls.
> 
> For some connector uses, that might matter.  For hooks in fork,
> that is no problem - we have all the process context one could
> want - two of them if that helps ;).
>
> > realyfs is interesting project, but it has different aims, 
> 
> That could well be ... I can't claim to know which of relayfs or
> connector would be better here, of the two.
> 
> 
> > while connector is purely control/notification mechanism
> 
> However connector is, in this regard, overkill.  We don't need a single
> event notification mechanism here.  One of the key ways in which
> accounting such as this has historically minimized system load is to
> forgo any effort to provide any notification or data packet per event,
> and instead immediately work to batch the data up in bulk form, with one
> buffer containing the concatenated data for multiple events.  This
> amortizes the cost of almost all the handling, and of all the disk i/o,
> over many data collection events.  Correct me if I'm wrong, but
> fork_connector doesn't do this merging of events into a consolidated
> data buffer, so is at a distinct disadvantage, for this use, because the
> data merging is delayed, and a separate, user level process, is required
> to accomplish the merging and conversion to writable blocks of data
> suitable for storing on the disk.

It is design decision, 
one may want to write all data even with slowdowning
the system, but later process it all at once, 
but one may want to process it in real-time in small pieces and have
direct 
vision of how the system behaves, and thus it requires very small
overhead.
Userspace fork connector daemon may send data to the network or transfer
it using some other mechanism without touching disk IO subsystem,
and it is faster than writing it disk, then reading it or may be
seeking,
and transfer again.

One instrument is better for one type of taks, 
others are suitable for different.

> Nothing wrong with a good screwdriver.  But if you want to pound nails,
> hammers, even rocks, work better.

He-he, while you lift your rock hammer, others can finish all the work
with theirs small instruments. :)

> -- 
>                   I won't rest till it's the best ...
>                   Programmer, Linux Scalability
>                   Paul Jackson <pj@engr.sgi.com> 1.650.933.1373, 1.925.600.0401
-- 
        Evgeniy Polyakov

Crash is better than data corruption -- Arthur Grabowski

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [patch 1/2] fork_connector: add a fork connector

[Paul Jackson]

Evgeniy writes:
> Here forking connector module "exits" and can handle next fork() on the
> same CPU.

Fine ... but it's not about what the fork_connector does.  It's about
getting the accounting data to disk, if I understand correctly.

> That is why it is very fast in "fast-path".

I don't care how fast a tool is.  I care how fast the job gets done.  If
a tool is only doing part of the job, then we can't decide whether to
use that tool just based on how fast that part of the job gets done.

> The most expensive part is cn_netlink_send()/netlink_broadcast(), 
> with CBUS it is deferred to the safe time,

This is "safe time" for the immediate purpose of letting the forking
process continue on its way.  But the deferred work of buffering up the
data and writing it to disk still needs to be done, pretty soon.  When
sizing a system to see how many users or jobs I can run on it at a time,
I will have to include sufficient cpu, memory and disk i/o to handle
getting this accounting data to disk, right?

> >  2) Using a modified form of what BSD ACCOUNTING does now:
> > 	- forking process appends single fork data to in-kernel buffer
> 
> It is not as simple.
> It takes global locks several times, it access bunch of shared between
> CPU data.
> It calls ->stat() and ->write() which may sleep.

Hmmm ... good points.  The mechanisms in the kernel now (and for the
last 25 years) to write out BSD ACCOUNTING data may not be numa friendly.

Perhaps there should be a per-cpu 512 byte buffer, which can gather up 8
accounting records (64 bytes each) and only call the file system write
once every 8 task exits.  Or perhaps a per-node buffer, with a spinlock
to serialize access by the CPUs on that node.  Or perhaps per-node
accounting files.  Or something like that.

Guillaume, Jay - do we (you ?) need to make classic BSD ACCOUNTING data
collection numa friendly?  Based on the various frustrated comments at
the top of kernel/acct.c, this could be a non-trivial effort to get
right.  Maybe we need it, but can't afford it.

And perhaps my proposed variable length records for supplementary
accounting, such as <parent pid, child pid> from fork, need to allow
for some way to pad out the rest of a buffer, when the next record
won't fit entirely.

> That work is deferred and does not affect in-kernel processes.

The accounting data collection cannot be deferred for long, perhaps
just a few minutes.  Not until the data hits the disk can we rest
indefinitely.  Unless, that is, I don't understand what problem is
being solved here (quite possible ;).

> And why userspace fork connector should write data to the disk?

I NEVER said it should.  I am NOT trying to redesign fork_connector.

Good grief ... how many times and ways do I have to say this ;)?

I am asking what is the best tool for accounting data collection,
which, if I understand correctly, does need to write to disk.

-- 
                  I won't rest till it's the best ...
                  Programmer, Linux Scalability
                  Paul Jackson <pj@engr.sgi.com> 1.650.933.1373, 1.925.600.0401

Re: [patch 1/2] fork_connector: add a fork connector

[Jay Lan]

Paul,

The fork_connector is not designed to solve accounting data collection
problem.

The accounting data collection must be done via a hook from do_exit().
The acct_process() hook invokes do_acct_process() to write BSD
accounting data to disk. CSA needs a similar hook off do_exit() to
collect more accounting data and write to disk in different
accounting records format. This part is not part of fork_connector.

ELSA does not care how accounting data being written to disks. However,
it needs the accounting data being reliably and accurately collected
and written to disk by BSD and/or CSA and it needs the <ppid, pid>
information to aggregate processes. It was never the fork_connector's
intention to piggy back the data to the accounting file.

Thanks,
  - jay



Paul Jackson wrote:
> Evgeniy writes:
> 
>>Here forking connector module "exits" and can handle next fork() on the
>>same CPU.
> 
> 
> Fine ... but it's not about what the fork_connector does.  It's about
> getting the accounting data to disk, if I understand correctly.
> 
> 
>>That is why it is very fast in "fast-path".
> 
> 
> I don't care how fast a tool is.  I care how fast the job gets done.  If
> a tool is only doing part of the job, then we can't decide whether to
> use that tool just based on how fast that part of the job gets done.
> 
> 
>>The most expensive part is cn_netlink_send()/netlink_broadcast(), 
>>with CBUS it is deferred to the safe time,
> 
> 
> This is "safe time" for the immediate purpose of letting the forking
> process continue on its way.  But the deferred work of buffering up the
> data and writing it to disk still needs to be done, pretty soon.  When
> sizing a system to see how many users or jobs I can run on it at a time,
> I will have to include sufficient cpu, memory and disk i/o to handle
> getting this accounting data to disk, right?
> 
> 
>>> 2) Using a modified form of what BSD ACCOUNTING does now:
>>>	- forking process appends single fork data to in-kernel buffer
>>
>>It is not as simple.
>>It takes global locks several times, it access bunch of shared between
>>CPU data.
>>It calls ->stat() and ->write() which may sleep.
> 
> 
> Hmmm ... good points.  The mechanisms in the kernel now (and for the
> last 25 years) to write out BSD ACCOUNTING data may not be numa friendly.
> 
> Perhaps there should be a per-cpu 512 byte buffer, which can gather up 8
> accounting records (64 bytes each) and only call the file system write
> once every 8 task exits.  Or perhaps a per-node buffer, with a spinlock
> to serialize access by the CPUs on that node.  Or perhaps per-node
> accounting files.  Or something like that.
> 
> Guillaume, Jay - do we (you ?) need to make classic BSD ACCOUNTING data
> collection numa friendly?  Based on the various frustrated comments at
> the top of kernel/acct.c, this could be a non-trivial effort to get
> right.  Maybe we need it, but can't afford it.
> 
> And perhaps my proposed variable length records for supplementary
> accounting, such as <parent pid, child pid> from fork, need to allow
> for some way to pad out the rest of a buffer, when the next record
> won't fit entirely.
> 
> 
>>That work is deferred and does not affect in-kernel processes.
> 
> 
> The accounting data collection cannot be deferred for long, perhaps
> just a few minutes.  Not until the data hits the disk can we rest
> indefinitely.  Unless, that is, I don't understand what problem is
> being solved here (quite possible ;).
> 
> 
>>And why userspace fork connector should write data to the disk?
> 
> 
> I NEVER said it should.  I am NOT trying to redesign fork_connector.
> 
> Good grief ... how many times and ways do I have to say this ;)?
> 
> I am asking what is the best tool for accounting data collection,
> which, if I understand correctly, does need to write to disk.
> 

Re: [patch 1/2] fork_connector: add a fork connector

[Paul Jackson]

Jay wrote:
> The fork_connector is not designed to solve accounting data collection
> problem.

I don't think I ever said it was designed for that purpose.

Indeed, I will confess to not yet knowing the 'real' purpose of its
design.


> It was never the fork_connector's
> intention to piggy back the data to the accounting file.

I am sure that was not its intention, and I can't imagine I would ever
have said it was.


> CSA needs a similar hook off do_exit() to
> collect more accounting data and write to disk in different
> accounting records format.

Aha - as I suspected - there will be more data to collect, in addition
to both the classic bsd accounting records at exit, and the <parent pid,
child pid> at fork.

-- 
                  I won't rest till it's the best ...
                  Programmer, Linux Scalability
                  Paul Jackson <pj@engr.sgi.com> 1.650.933.1373, 1.925.600.0401

Re: [patch 1/2] fork_connector: add a fork connector

[Evgeniy Polyakov]

Sorry for long delay - I was quite far from my test machines.
Here are results:

fork connector with turned off disk writes and direct connector's 
methods calls.

pcix$ ./fork_test 100000
Average per process fork+exit time is 505 usecs [diff=50567251, max=100000].
pcix$ ./fork_test 100000
Average per process fork+exit time is 512 usecs [diff=51248174, max=100000].
pcix$ ./fork_test 100000
Average per process fork+exit time is 504 usecs [diff=50469379, max=100000].

fork connector with turned on disk writes and direct connector's 
methods calls. 
Each disk write has about 80 bytes which are:
	time(&tm);
	fprintf(out,
		"%.24s : [%x.%x] [seq=%08x, ack=%08x] %s.\n",
		ctime(&tm), data->id.idx, data->id.val,
		data->seq, data->ack, (char *)data->data);

pcix$ ./fork_test 100000
Average per process fork+exit time is 539 usecs [diff=53944663, max=100000].
pcix$ ./fork_test 100000
Average per process fork+exit time is 523 usecs [diff=52378314, max=100000].
pcix$ ./fork_test 100000
Average per process fork+exit time is 540 usecs [diff=54078648, max=100000].

CBUS results.

Writing disabled:

pcix$ ./fork_test 100000
Average per process fork+exit time is 451 usecs [diff=45194377, max=100000].
pcix$ ./fork_test 100000
Average per process fork+exit time is 454 usecs [diff=45416470, max=100000].
pcix$ ./fork_test 100000
Average per process fork+exit time is 448 usecs [diff=44863153, max=100000].
pcix$ ./fork_test 100000
Average per process fork+exit time is 453 usecs [diff=45312870, max=100000].
pcix$

Writing enabled like described above:

pcix$ ./fork_test 100000
Average per process fork+exit time is 456 usecs [diff=45680384, max=100000].
pcix$ ./fork_test 100000
Average per process fork+exit time is 455 usecs [diff=45590682, max=100000].
pcix$ ./fork_test 100000
Average per process fork+exit time is 453 usecs [diff=45376436, max=100000].
pcix$


fork connector is not compiled in:

Average per process fork+exit time is 452 usecs [diff=45280538, max=100000].
pcix$ ./fork_test 100000
Average per process fork+exit time is 446 usecs [diff=44687388, max=100000].
pcix$ ./fork_test 100000
Average per process fork+exit time is 445 usecs [diff=44505999, max=100000].
pcix$ ./fork_test 100000


With 80 bytes write per fork with CBUS it takes from 0.5% to 2.5%.
So it still can be used for accounting :)

-- 
	Evgeniy Polyakov

Re: [patch 1/2] fork_connector: add a fork connector

[Paul Jackson]

> So it still can be used for accounting :)

No ... so these results don't show that it shouldn't be used for
accounting.

-- 
                  I won't rest till it's the best ...
                  Programmer, Linux Scalability
                  Paul Jackson <pj@engr.sgi.com> 1.650.933.1373, 1.925.600.0401

Re: [patch 1/2] fork_connector: add a fork connector

[dean gaudet]

On Tue, 29 Mar 2005, Jay Lan wrote:

> The fork_connector is not designed to solve accounting data collection
> problem.
> 
> The accounting data collection must be done via a hook from do_exit().

by the time do_exit() occurs the parent may have disappeared... you do 
need to record something at fork() time so that you can account to the 
correct ancestor.

an example of where this ancestry is useful would be the summation of all 
cpu time spent by children of apache, spamd, clamd, ...

-dean

Re: [patch 1/2] fork_connector: add a fork connector

[Paul Jackson]

Dean wrote:
> by the time do_exit() occurs the parent may have disappeared

I don't think Jay was disagreeing with this.  I think he agrees
that there is to be collected:
 1) the classic bsd accounting data, in do_exit
 2) the fork time <parent pid, child pid> by some mechanism at
	fork time (perhaps just not the fork_connect mechanism)
 3) some additional data to be harvested at exit time, for CSA

I suspect you two are just tripping over words to describe this.

However, this does expose another possibility.  Record the original
forking parent pid in another task_struct field at fork time (didn't
someone else have a 'bio_pid' patch to this affect?), and add that task
struct value to the list of additional items to be written out, at exit
time.

I was skeptical that CBUS could have zero impact on fork, but recording
one more word in the task struct at fork gets about as close to zero
impact as one can get on fork.

-- 
                  I won't rest till it's the best ...
                  Programmer, Linux Scalability
                  Paul Jackson <pj@engr.sgi.com> 1.650.933.1373, 1.925.600.0401

Re: [patch 1/2] fork_connector: add a fork connector

[Guillaume Thouvenin]

On Tue, 2005-03-29 at 22:06 -0800, dean gaudet wrote:
> On Tue, 29 Mar 2005, Jay Lan wrote:
> 
> > The fork_connector is not designed to solve accounting data collection
> > problem.
> > 
> > The accounting data collection must be done via a hook from do_exit().
> 
> by the time do_exit() occurs the parent may have disappeared... you do 
> need to record something at fork() time so that you can account to the 
> correct ancestor.

You're right. At fork(), the "job daemon", provided by ELSA, records
information about parent PID, child PID and also about the group of
processes they belong to. At exit(), accounting data are recorded by CSA
or BSD-like accounting. 

> an example of where this ancestry is useful would be the summation of all 
> cpu time spent by children of apache, spamd, clamd, ...

You're right. One usage can be: apache, spamd and clamd can be put in a
job (a group of processes) by using the "job daemon" and automatically,
all children will belong to the same jobs respectively. So the gaol here
is really to perform per-group of processes accounting using ELSA and
CSA accounting data.

Best Regards,
Guillaume

Re: [patch 1/2] fork_connector: add a fork connector

[Jay Lan]

The parent information ((ppid,pid) pair) is useful for process group
aggregation while do_exit() hook is needed to save per task
accounting data before the task data is disposed.

Thanks,
  - jay


dean gaudet wrote:
> On Tue, 29 Mar 2005, Jay Lan wrote:
> 
> 
>>The fork_connector is not designed to solve accounting data collection
>>problem.
>>
>>The accounting data collection must be done via a hook from do_exit().
> 
> 
> by the time do_exit() occurs the parent may have disappeared... you do 
> need to record something at fork() time so that you can account to the 
> correct ancestor.
> 
> an example of where this ancestry is useful would be the summation of all 
> cpu time spent by children of apache, spamd, clamd, ...
> 
> -dean

Re: [patch 1/2] fork_connector: add a fork connector

[Guillaume Thouvenin]

On Mon, 2005-03-28 at 13:42 -0800, Paul Jackson wrote:
> Guillaume wrote:
> >   The lmbench shows that the overhead (the construction and the sending
> > of the message) in the fork() routine is around 7%.
> 
> Thanks for including the numbers.  The 7% seems a bit costly, for a bit
> more accounting information.  Perhaps dean's suggestion, to not use
> ascii, will help.  I hope so, though I doubt it will make a huge
> difference.  Was this 7% loss with or without a user level program
> consuming the sent messages?  I would think that the number of interest
> would include a minimal consumer task.

Yes, dean's suggestion helps. The overhead is now around 4%

fork_connector disabled:
  Process fork+exit: 149.4444 microseconds

fork_connector enabled:
  Process fork+exit: 154.9167 microseconds

> Having the "#ifdef CONFIG_FORK_CONNECTOR" chunk of code right in fork.c
> seems unfortunate.  Can the real fork_connector() be put elsewhere, and
> the ifdef put in a header file that makes it a no-op if not configured,
> or simply a function declaration, if configured?

I think that it can be moved in include/linux/connector.h 

Guillaume

Re: [patch 1/2] fork_connector: add a fork connector

[Paul Jackson]

Guillaume wrote:
> Yes, dean's suggestion helps. The overhead is now around 4%

More improvement than I expected (and I see a CBUS result further
down in my inbox).

Does this include a minimal consumer task of the data that writes
it to disk?

> I think that it can be moved in include/linux/connector.h 

Good.

-- 
                  I won't rest till it's the best ...
                  Programmer, Linux Scalability
                  Paul Jackson <pj@engr.sgi.com> 1.650.933.1373, 1.925.600.0401

Re: [patch 1/2] fork_connector: add a fork connector

[Guillaume Thouvenin]

On Mon, 2005-03-28 at 13:42 -0800, Paul Jackson wrote:
> Guillaume wrote:
> >   The lmbench shows that the overhead (the construction and the sending
> > of the message) in the fork() routine is around 7%.
> 
> Thanks for including the numbers.  The 7% seems a bit costly, for a bit
> more accounting information.  Perhaps dean's suggestion, to not use
> ascii, will help.  I hope so, though I doubt it will make a huge
> difference.  Was this 7% loss with or without a user level program
> consuming the sent messages?  I would think that the number of interest
> would include a minimal consumer task.

I ran some test using the CBUS instead of the cn_netlink_send() routine
and the overhead is nearly 0%:

fork connector disabled:
    Process fork+exit: 148.1429 microseconds

fork connector enabled:
    Process fork+exit: 148.4595 microseconds

Regards,
Guillaume

Re: [patch 1/2] fork_connector: add a fork connector

[Paul Jackson]

Guillaume wrote:
> I ran some test using the CBUS instead of the cn_netlink_send() routine
> and the overhead is nearly 0%:

Overhead of what?  Does this include merging the data and getting it to
disk?

Am I even asking the right question here - is it true that this data,
when collected for accounting purposes, needs to go to disk, and that
summarizing and analyzing the data is done 'off-line', perhaps hours
later?  That's the way it was 25 years ago ... but perhaps the basic
data flow appropriate for accounting has changed since then.

And if the data flow has changed, then how to you account for the fact
that the rest of the accounting data, under the CONFIG_BSD_PROCESS_ACCT
option, is still collected the 'old fashioned way' (direct kernel write
to disk)?

-- 
                  I won't rest till it's the best ...
                  Programmer, Linux Scalability
                  Paul Jackson <pj@engr.sgi.com> 1.650.933.1373, 1.925.600.0401

Re: [patch 1/2] fork_connector: add a fork connector

[Guillaume Thouvenin]

On Tue, 2005-03-29 at 07:35 -0800, Paul Jackson wrote:
> Guillaume wrote:
> > I ran some test using the CBUS instead of the cn_netlink_send() routine
> > and the overhead is nearly 0%:
> 
> Overhead of what?  Does this include merging the data and getting it to
> disk?

I test the overhead of sending the fork information to a user space
application. The merge of the data is done later and it has nothing to
do with the fork connector...

> Am I even asking the right question here - is it true that this data,
> when collected for accounting purposes, needs to go to disk, and that
> summarizing and analyzing the data is done 'off-line', perhaps hours
> later?  That's the way it was 25 years ago ... but perhaps the basic
> data flow appropriate for accounting has changed since then.

  Accounting is another problem and, as you said previously, summarizing
and analyzing the data is done later. 

  I'm sorry but I really don't understand why you're speaking about
accounting when I present results about fork connector. I agree that
ELSA is using the fork connector but the fork connector has nothing to
do with accounting.

Regards,
Guillaume

Re: [patch 1/2] fork_connector: add a fork connector

[Paul Jackson]

Guillaume wrote:
>   I'm sorry but I really don't understand why you're speaking about
> accounting when I present results about fork connector. I agree that
> ELSA is using the fork connector but the fork connector has nothing to
> do with accounting.

True - sorry.  I kinda hijacked your thread.  I had fork_connector
associated in my mind with process accounting, so made the leap from
analyzing the fork_connector mechanism on its own merit, to analyzing
whether it was useful for collecting the new process accounting
information that was needed from forks.

In my own defense, I don't see where the motivations for fork_connector
are spelled out in the presentation to this patch, and it seems that
the other potential uses of it are less well explored at this point.

So I think my leap was a small one ;).

-- 
                  I won't rest till it's the best ...
                  Programmer, Linux Scalability
                  Paul Jackson <pj@engr.sgi.com> 1.650.933.1373, 1.925.600.0401