2005-02-07 14:28:23

by Guillaume Thouvenin

[permalink] [raw]
Subject: [RFC][PATCH 2.6.11-rc3-mm1] Relay Fork Module

Hello,

This module sends a signal to one or several processes (in user
space) when a fork occurs in the kernel. It relays information about
forks (parent and child pid) to a user space application. The relay fork
module adds a hook in the do_fork() routine that can be used by other
modules and it uses the sysfs file system. The main directory is called
"/sys/relayfork" and it contains two files. One keeps the number of the
signal that is send when a fork occurred and the other is a list of
processes that are registered. Those files are readable and writable.
The tree structure is as follow:
sys/
|
--> relayfork/
|
---> processes
---> signal

If you want to register a process, just "echoing" its PID in the file
"processes". Example:
- Register process 123
echo 123 > /sys/relayfork/processes
- Un-Register process 123
echo -123 > /sys/relayfork/processes

By default the module sends the RT signal "33". We use a RT signal
because if a signal occurs when an another one is still in the queue it
is added. If it's not a RT signal, the signal is lost. The administrator
is free to change it.
echo XX > /sys/relayfork/signal

I compile a kernel 2.6.10 to see performances impact:

# time sh -c 'make bzImage && make modules'

with a vanilla kernel: real 8m10.797s
user 7m29.652s
sys 0m49.275s

with kernel + patch : real 8m11.137s
user 7m30.034s
sys 0m49.994s

with kernel + patch + ELSA(*) : real 8m12.162s
user 7m30.835s
sys 0m50.082s

(*) ELSA uses the patch in order to perform per-group of processes
accounting. It's available at http://elsa.sf.net

You can compile it as a module or directly in the kernel.

This patch is used by the Enhanced Linux System Accounting tool that
can be downloaded from http://elsa.sf.net

Regards,
Guillaume

$ diffstat linux-2.6.11-rc3-mm1-relayfork.patch
include/linux/generic_hooks.h | 9
init/Kconfig | 25 ++
kernel/Makefile | 1
kernel/fork.c | 27 ++
kernel/relay_fork.c | 477 ++++++++....++++++++++++++
5 files changed, 539 insertions(+)

Signed-off-by: Guillaume Thouvenin <[email protected]>


diff -uprN -X dontdiff linux-2.6.11-rc3-mm1/include/linux/generic_hooks.h linux-2.6.11-rc3-mm1-relayfork/include/linux/generic_hooks.h
--- linux-2.6.11-rc3-mm1/include/linux/generic_hooks.h 1970-01-01 01:00:00.000000000 +0100
+++ linux-2.6.11-rc3-mm1-relayfork/include/linux/generic_hooks.h 2005-02-07 10:43:41.000000000 +0100
@@ -0,0 +1,9 @@
+#ifndef GENERIC_HOOKS_H
+#define GENERIC_HOOKS_H
+
+struct do_fork_cb {
+ struct list_head list;
+ void (* handler)(struct task_struct *, struct task_struct *);
+};
+
+#endif
diff -uprN -X dontdiff linux-2.6.11-rc3-mm1/init/Kconfig linux-2.6.11-rc3-mm1-relayfork/init/Kconfig
--- linux-2.6.11-rc3-mm1/init/Kconfig 2005-02-07 10:16:38.000000000 +0100
+++ linux-2.6.11-rc3-mm1-relayfork/init/Kconfig 2005-02-07 10:45:06.000000000 +0100
@@ -199,6 +199,31 @@ config KOBJECT_UEVENT
Say Y, unless you are building a system requiring minimal memory
consumption.

+config RELAY_FORK
+ tristate "Relay Fork Event"
+ depends on SYSFS
+ default n
+ ---help---
+ This module sends a signal to one or several processes when a fork
+ occurs in the kernel. It relays information about forks.
+
+ The relay fork module is configured by using the sysfs. The main
+ directory is called "/sys/relayfork" and it contains two files. One
+ keeps the number of the signal that is send when a fork occurred and
+ the other is a list of processes that are registered. Those files
+ are writable. The tree structure is as follow:
+ sys/
+ sys/relayfork/
+ sys/relayfork/processes
+ sys/relayfork/signal
+
+ If you want to register a process, just "echoing" its PID in the
+ file "processes". Example:
+ - Register process 123
+ echo 123 > /sys/relayfork/processes
+ - Un-Register process 123
+ echo -123 > /sys/relayfork/processes
+
config IKCONFIG
bool "Kernel .config support"
---help---
diff -uprN -X dontdiff linux-2.6.11-rc3-mm1/kernel/fork.c linux-2.6.11-rc3-mm1-relayfork/kernel/fork.c
--- linux-2.6.11-rc3-mm1/kernel/fork.c 2005-02-07 10:16:38.000000000 +0100
+++ linux-2.6.11-rc3-mm1-relayfork/kernel/fork.c 2005-02-07 10:48:20.000000000 +0100
@@ -41,6 +41,7 @@
#include <linux/profile.h>
#include <linux/rmap.h>
#include <linux/acct.h>
+#include <linux/generic_hooks.h>

#include <asm/pgtable.h>
#include <asm/pgalloc.h>
@@ -63,6 +64,26 @@ DEFINE_PER_CPU(unsigned long, process_co

EXPORT_SYMBOL(tasklist_lock);

+static LIST_HEAD(do_fork_cb_list);
+static spinlock_t do_fork_cb_spinlock = SPIN_LOCK_UNLOCKED;
+
+void do_fork_cb_register(struct do_fork_cb *p)
+{
+ spin_lock(&do_fork_cb_spinlock);
+ list_add(&p->list, &do_fork_cb_list);
+ spin_unlock(&do_fork_cb_spinlock);
+}
+EXPORT_SYMBOL(do_fork_cb_register);
+
+void do_fork_cb_unregister(struct do_fork_cb *p)
+{
+ spin_lock(&do_fork_cb_spinlock);
+ list_del(&p->list);
+ spin_unlock(&do_fork_cb_spinlock);
+}
+EXPORT_SYMBOL(do_fork_cb_unregister);
+
+
int nr_processes(void)
{
int cpu;
@@ -1209,6 +1230,7 @@ long do_fork(unsigned long clone_flags,
*/
if (!IS_ERR(p)) {
struct completion vfork;
+ struct do_fork_cb *cb;

if (clone_flags & CLONE_VFORK) {
p->vfork_done = &vfork;
@@ -1238,6 +1260,11 @@ long do_fork(unsigned long clone_flags,
if (unlikely (current->ptrace & PT_TRACE_VFORK_DONE))
ptrace_notify ((PTRACE_EVENT_VFORK_DONE << 8) | SIGTRAP);
}
+
+ spin_lock(&do_fork_cb_spinlock);
+ list_for_each_entry(cb, &do_fork_cb_list,list)
+ cb->handler(current, p);
+ spin_unlock(&do_fork_cb_spinlock);
} else {
free_pidmap(pid);
pid = PTR_ERR(p);
diff -uprN -X dontdiff linux-2.6.11-rc3-mm1/kernel/Makefile linux-2.6.11-rc3-mm1-relayfork/kernel/Makefile
--- linux-2.6.11-rc3-mm1/kernel/Makefile 2005-02-07 10:16:38.000000000 +0100
+++ linux-2.6.11-rc3-mm1-relayfork/kernel/Makefile 2005-02-07 10:48:43.000000000 +0100
@@ -30,6 +30,7 @@ obj-$(CONFIG_SYSFS) += ksysfs.o
obj-$(CONFIG_DETECT_SOFTLOCKUP) += softlockup.o
obj-$(CONFIG_GENERIC_HARDIRQS) += irq/
obj-$(CONFIG_CRASH_DUMP) += crash_dump.o
+obj-$(CONFIG_RELAY_FORK) += relay_fork.o

ifneq ($(CONFIG_IA64),y)
# According to Alan Modra <[email protected]>, the -fno-omit-frame-pointer is
diff -uprN -X dontdiff linux-2.6.11-rc3-mm1/kernel/relay_fork.c linux-2.6.11-rc3-mm1-relayfork/kernel/relay_fork.c
--- linux-2.6.11-rc3-mm1/kernel/relay_fork.c 1970-01-01 01:00:00.000000000 +0100
+++ linux-2.6.11-rc3-mm1-relayfork/kernel/relay_fork.c 2005-02-07 10:49:02.000000000 +0100
@@ -0,0 +1,477 @@
+/*
+ * Relay fork interface
+ *
+ * Author: Guillaume Thouvenin <[email protected]>
+ *
+ * This module is a relay for fork monitoring. You use the
+ * /sys/relayfork/processes to register your process or to
+ * display registered processes.
+ * The interface send a signal (default is a the RT signal 33)
+ * to all registered processes when a fork occurs in the kernel.
+ * The signal can be change by using /sys/relayfork/signal.
+ *
+ * Conventions:
+ * structures are prefixed by rfork_
+ * functions are prefixed by rfd_
+ * variable are prefixed by relayf_
+ */
+
+#include <linux/module.h> /* for all modules */
+#include <linux/kernel.h> /* for KERN_ALERT */
+#include <linux/init.h> /* for the macros */
+
+#include <linux/spinlock.h> /* for spinlock */
+#include <linux/sched.h> /* for task_struct */
+#include <linux/kobject.h>
+#include <linux/generic_hooks.h>
+
+#define RELAY_FORK_SIGNAL 33
+
+MODULE_AUTHOR("Guillaume Thouvenin <[email protected]>");
+MODULE_DESCRIPTION("sysfs interface to relay fork device");
+MODULE_LICENSE("GPL");
+
+/* available if the relay-fork patch is applied */
+extern void do_fork_cb_register(struct do_fork_cb *p);
+extern void do_fork_cb_unregister(struct do_fork_cb *p);
+
+/*******************
+ * local definition
+ *******************/
+
+
+struct rfork_device {
+ struct kobject kobj;
+ struct list_head proclist; /* registered processes */
+ unsigned long signal; /* signal to send to processes */
+};
+
+struct rfork_attribute {
+ struct attribute attr;
+ ssize_t(*show) (struct rfork_device * dev, char *buf);
+ ssize_t(*store) (struct rfork_device * dev, const char *buf,
+ size_t count);
+};
+
+struct rfork_proclist {
+ struct list_head list;
+ unsigned long pid;
+
+};
+
+/*****************************
+ * header for local functions
+ *****************************/
+
+
+ssize_t rfd_attr_show(struct kobject *kobj, struct attribute *attr,
+ char *buf);
+ssize_t rfd_attr_store(struct kobject *kobj, struct attribute *attr,
+ const char *buf, size_t size);
+
+ssize_t rfd_proclist_show(struct rfork_device *dev, char *buf);
+ssize_t rfd_proclist_store(struct rfork_device *dev, const char *buf,
+ size_t size);
+ssize_t rfd_signal_show(struct rfork_device *dev, char *buf);
+ssize_t rfd_signal_store(struct rfork_device *dev, const char *buf,
+ size_t size);
+
+
+/*****************
+ * local variable
+ *****************/
+
+static spinlock_t relayf_lock = SPIN_LOCK_UNLOCKED; /* protect relayf */
+
+struct sysfs_ops relayf_sysfs_ops = {
+ .show = rfd_attr_show,
+ .store = rfd_attr_store,
+};
+
+struct kobj_type relayf_kobj_type = {
+ .sysfs_ops = &relayf_sysfs_ops,
+};
+
+static struct rfork_device relayf;
+
+static struct do_fork_cb fork_hdler;
+
+#define RELAYF_ATTR(_name,_mode,_show,_store) \
+struct rfork_attribute relayf_attr_##_name = { \
+ .attr = {.name = __stringify(_name) , .mode = _mode }, \
+ .show = _show, \
+ .store = _store, \
+};
+/* give write access only to root, otherwise it's a security hole */
+static RELAYF_ATTR(processes, 0644, rfd_proclist_show, rfd_proclist_store);
+static RELAYF_ATTR(signal, 0644, rfd_signal_show, rfd_signal_store);
+
+
+/**************************
+ * body of local functions
+ **************************/
+
+
+/**
+ * rfd_strtoi - convert a string to an unsigned long
+ * @name: the string to convert
+ *
+ * Convert a string to an unsigned long. When we found
+ * a character that is not a number, we return immediatly.
+ * The first character can be '-' but if it is, we just
+ * ignore it and return the positive value.
+ */
+static unsigned long rfd_strtoi(const char *name)
+{
+ unsigned long val = 0;
+
+ /* If it's a negative value we just forgot the '-' */
+ if (*name == '-')
+ name++;
+
+ for (;; name++) {
+ switch (*name) {
+ case '0'...'9':
+ val = 10 * val + (*name - '0');
+ break;
+ default:
+ return val;
+ }
+ }
+}
+
+/**
+ * rfd_nbdigits - return the number of digits of an integer
+ * @i: the integer
+ *
+ * example: For "0" it returns 1
+ * For "324" it returns 3
+ * For "-324" it returns 3
+ */
+
+static inline int rfd_nbdigits(int i)
+{
+ int res = 0;
+
+ if (i < 0)
+ i *= -1;
+
+ do {
+ i = i / 10;
+ res++;
+ } while (i != 0);
+
+ return res;
+}
+
+#define KOBJ_TO_RFORK(obj) container_of(obj, struct rfork_device, kobj)
+#define ATTR_TO_RFORK(obj) container_of(obj, struct rfork_attribute, attr)
+
+/**
+ * rfd_attr_show - method called when a file is read
+ * @kobj: the kobject
+ * @attr: the attribute
+ * @buf: the buffer
+ *
+ * Translate the generic struct kobject and struct attribute
+ * pointers to the appropriate pointer types, and calls the
+ * associated methods.
+ */
+ssize_t rfd_attr_show(struct kobject * kobj, struct attribute * attr,
+ char *buf)
+{
+ struct rfork_device *rf_dev = KOBJ_TO_RFORK(kobj);
+ struct rfork_attribute *rf_attr = ATTR_TO_RFORK(attr);
+ ssize_t ret = 0;
+
+ if (rf_attr->show)
+ ret = rf_attr->show(rf_dev, buf);
+
+ return ret;
+}
+
+/**
+ * rfd_attr_store - method called when a file is written
+ * @kobj: the kobject
+ * @attr: the attribute
+ * @buf: the buffer
+ *
+ * Translate the generic struct kobject and struct attribute
+ * pointers to the appropriate pointer types, and calls the
+ * associated methods.
+ */
+ssize_t rfd_attr_store(struct kobject * kobj, struct attribute * attr,
+ const char *buf, size_t size)
+{
+ struct rfork_device *rf_dev = KOBJ_TO_RFORK(kobj);
+ struct rfork_attribute *rf_attr = ATTR_TO_RFORK(attr);
+ ssize_t ret = 0;
+
+ if (rf_attr->store)
+ ret = rf_attr->store(rf_dev, buf, size);
+
+ return ret;
+}
+
+/**
+ * rfd_proclist_show - method called when processes attribute is read
+ * @dev: the relay fork device
+ * @buf: the buffer
+ *
+ * The buffer is a buffer of size PAGE_SIZE (due to sysfs implementation).
+ * As the method is called only once for a read, the show() method should
+ * fill the entire buffer. Buffer is filled with a list of processes that
+ * are registered. As the size of the buffer is limited, we also limit
+ * how many processes are displayed.
+ * Method returns the number of bytes printed into the buffer. If a bad
+ * value comes through, it returns an error.
+ */
+ssize_t rfd_proclist_show(struct rfork_device * dev, char *buf)
+{
+ struct rfork_proclist *p;
+ struct list_head *pos;
+ struct list_head *next;
+ int char_print;
+ char tmp[PAGE_SIZE];
+
+ char_print = 0;
+
+ spin_lock_irq(&relayf_lock);
+ list_for_each_safe(pos, next, &dev->proclist) {
+ p = container_of(pos, struct rfork_proclist, list);
+ if (char_print + rfd_nbdigits(p->pid) + 1 < PAGE_SIZE - 1) {
+ memset(tmp, 0, PAGE_SIZE);
+ char_print += sprintf(tmp, "%lu ", p->pid);
+ strcat(buf, tmp);
+ } else
+ /*
+ * buf is full enough, we don't need to go through
+ * the list
+ */
+ break;
+ }
+ spin_unlock_irq(&relayf_lock);
+ strncat(buf, "\n", 1);
+ char_print++;
+ return char_print;
+}
+
+/**
+ * rfd_proclist_store - method called when processes attribute is written
+ * @dev: the relay fork device
+ * @buf: the buffer
+ * @size:
+ *
+ * As the method is called only once for a write, the store() method should
+ * fill the entire buffer. Methods should return the number of bytes used
+ * from the buffer. If a bad value comes through, we return an error.
+ */
+ssize_t rfd_proclist_store(struct rfork_device * dev, const char *buf,
+ size_t size)
+{
+ struct rfork_proclist *p;
+ struct list_head *pos; /* use as a loop counter */
+ struct list_head *next; /* temporary storage */
+ int pid; /* process to add or remove */
+
+ pid = rfd_strtoi(buf);
+ if (pid == 0)
+ /* nothing to do */
+ return strlen(buf);
+
+ switch (*buf) {
+ case '-':
+ /* remove the process */
+ spin_lock_irq(&relayf_lock);
+ list_for_each_safe(pos, next, &dev->proclist) {
+ p = container_of(pos, struct rfork_proclist, list);
+ if (p->pid == pid) {
+ list_del(&p->list);
+ spin_unlock_irq(&relayf_lock);
+ kfree(p);
+ return strlen(buf);
+ }
+ }
+ spin_unlock_irq(&relayf_lock);
+ break;
+ default:
+ /* add the process. We check if it is already in the list */
+ spin_lock_irq(&relayf_lock);
+ list_for_each_safe(pos, next, &dev->proclist) {
+ p = container_of(pos, struct rfork_proclist, list);
+ if (p->pid == pid) {
+ spin_unlock_irq(&relayf_lock);
+ return strlen(buf);
+ }
+ }
+
+ /* check if it is a valid pid */
+ read_lock(&tasklist_lock);
+ if (!find_task_by_pid(pid)) {
+ read_unlock(&tasklist_lock);
+ spin_unlock_irq(&relayf_lock);
+ return strlen(buf);
+ }
+ read_unlock(&tasklist_lock);
+ spin_unlock_irq(&relayf_lock);
+
+ p = kmalloc(sizeof(struct rfork_proclist), GFP_KERNEL);
+ if (!p)
+ return -ENOMEM;
+
+ INIT_LIST_HEAD(&p->list);
+ p->pid = rfd_strtoi(buf);
+ spin_lock_irq(&relayf_lock);
+ list_add(&p->list, &dev->proclist);
+ spin_unlock_irq(&relayf_lock);
+ }
+
+ return strlen(buf);
+}
+
+/**
+ * rfd_signal_show - method called when signal attribute is read
+ * @dev: the relay fork device
+ * @buf: the buffer
+ *
+ * Buffer is filled by an integer which is the RT signal used by
+ * the relay fork interface to relay fork information to processes.
+ */
+ssize_t rfd_signal_show(struct rfork_device * dev, char *buf)
+{
+ return sprintf(buf, "%lu\n", dev->signal);
+}
+
+/**
+ * rfd_signal_store - method called when signal attribute is written
+ * @dev: the relay fork device
+ * @buf: the buffer
+ * @size:
+ *
+ * Read the string in the buffer and convert it to an unsigned long
+ * integer. This value is used to set the new RT signal. A RT signal
+ * must be between SIGRTMIN and SIGRTMAX.
+ */
+ssize_t rfd_signal_store(struct rfork_device * dev, const char *buf,
+ size_t size)
+{
+ unsigned long rtsignal = rfd_strtoi(buf);
+
+ spin_lock_irq(&relayf_lock);
+ if ((rtsignal > SIGRTMIN) && (rtsignal < SIGRTMAX))
+ dev->signal = rfd_strtoi(buf);
+ spin_unlock_irq(&relayf_lock);
+
+ return strlen(buf);
+}
+
+/**
+ * rfd_relay_fork_info - send information to all registered processes
+ * @parent: PID of the parent
+ * @child: PID of the child
+ *
+ * Send information to all registered processes when a fork
+ * occurs in the kernel.
+ */
+void rfd_relay_fork_info(struct task_struct *parent, struct task_struct *child)
+{
+ struct list_head *pos;
+ struct list_head *next;
+ struct siginfo sinfo;
+
+ /*pr_info("FORK: parent = %d, child = %d\n", parent, child); */
+
+ spin_lock_irq(&relayf_lock);
+ list_for_each_safe(pos, next, &relayf.proclist) {
+ struct task_struct *taskp;
+ struct rfork_proclist *p =
+ container_of(pos, struct rfork_proclist, list);
+
+ read_lock(&tasklist_lock);
+ taskp = find_task_by_pid(p->pid);
+ if (taskp) {
+ sinfo.si_signo = relayf.signal;
+ sinfo.si_errno = 0;
+ sinfo.si_code = SI_ASYNCIO;
+ /*
+ * should be the sender ID but in our case, it is set
+ * to the pid of the process that initiates the fork.
+ */
+ sinfo.si_pid = parent->pid;
+ /*
+ * process created during the fork. This information
+ * is needed to keep jobs coherent.
+ */
+ sinfo.si_value.sival_int = child->pid;
+
+ send_sig_info(relayf.signal, &sinfo, taskp);
+ } else {
+ /* we can remove it from proclist */
+ list_del(&p->list);
+ kfree(p);
+ }
+ read_unlock(&tasklist_lock);
+ }
+ spin_unlock_irq(&relayf_lock);
+}
+
+/**
+ * Modules start and stop
+ **/
+
+/**
+ * rfd_init -
+ */
+static int __init rfd_init(void)
+{
+ int retval;
+
+ INIT_LIST_HEAD(&fork_hdler.list);
+ fork_hdler.handler = &rfd_relay_fork_info;
+
+ do_fork_cb_register(&fork_hdler);
+
+ /* Initialization of relayf */
+ kobject_set_name(&relayf.kobj, "relayfork");
+ relayf.kobj.ktype = &relayf_kobj_type;
+ INIT_LIST_HEAD(&relayf.proclist);
+ relayf.signal = RELAY_FORK_SIGNAL;
+
+ /*
+ * Register the kobject. As our object doesn't have a parent
+ * or a dominant kset, a directory is created at the top-level
+ * of the sysfs partition.
+ */
+ retval = kobject_register(&relayf.kobj);
+ sysfs_create_file(&relayf.kobj, &relayf_attr_processes.attr);
+ sysfs_create_file(&relayf.kobj, &relayf_attr_signal.attr);
+
+ return retval;
+}
+
+/**
+ * rfd_cleanup -
+ */
+static void __exit rfd_cleanup(void)
+{
+ struct list_head *pos; /* use as a loop counter */
+ struct list_head *next; /* temporary storage */
+
+ do_fork_cb_register(&fork_hdler);
+
+ /* release entry that was dynamically allocated */
+ list_for_each_safe(pos, next, &relayf.proclist) {
+ struct rfork_proclist *p =
+ container_of(pos, struct rfork_proclist, list);
+ list_del(&p->list);
+ kfree(p);
+ }
+
+ sysfs_remove_file(&relayf.kobj, &relayf_attr_processes.attr);
+ sysfs_remove_file(&relayf.kobj, &relayf_attr_signal.attr);
+ kobject_unregister(&relayf.kobj);
+}
+
+
+module_init(rfd_init);
+module_exit(rfd_cleanup);



2005-02-07 23:41:50

by Andrew Morton

[permalink] [raw]
Subject: Re: [RFC][PATCH 2.6.11-rc3-mm1] Relay Fork Module

Guillaume Thouvenin <[email protected]> wrote:
>
> Hello,
>
> This module sends a signal to one or several processes (in user
> space) when a fork occurs in the kernel. It relays information about
> forks (parent and child pid) to a user space application.
>
> ...
> This patch is used by the Enhanced Linux System Accounting tool that
> can be downloaded from http://elsa.sf.net

So this permits ELSA to maintain a complete picture of the process/thread
hierarchy? I guess that fits into the "do it in userspace" mantra -
certainly hooking into fork() is a minimal way of doing this, although I
wonder what the limitations are.

Implementation-wise: there's a lot of code there and the interface is a bit
awkward. Why not just feed that kobject you have there into
kobject_uevent()?

2005-02-08 08:35:39

by Guillaume Thouvenin

[permalink] [raw]
Subject: Re: [RFC][PATCH 2.6.11-rc3-mm1] Relay Fork Module

On Mon, 2005-02-07 at 15:46 -0800, Andrew Morton wrote:
> Guillaume Thouvenin <[email protected]> wrote:
> >
> > This module sends a signal to one or several processes (in user
> > space) when a fork occurs in the kernel. It relays information about
> > forks (parent and child pid) to a user space application.
> >
> > ...
> > This patch is used by the Enhanced Linux System Accounting tool
> > that can be downloaded from http://elsa.sf.net
>
> So this permits ELSA to maintain a complete picture of the
> process/thread hierarchy?

Yes it does.

> Implementation-wise: there's a lot of code there and the interface is
> a bit
> awkward. Why not just feed that kobject you have there into
> kobject_uevent()?

There isn't good reason and as kobject_uevent is here to notify user
space application by sending event through the netlink interface we can
add a new event (KOBJ_FORK) and send the fork creation by using this
interface. I agree with your comment and I'm working on this solution.

Thank you very much for your help,
Regards,

Guillaume

2005-02-11 08:12:19

by Guillaume Thouvenin

[permalink] [raw]
Subject: Re: [RFC][PATCH 2.6.11-rc3-mm2] Relay Fork Module

On Mon, 2005-02-07 at 15:46 -0800, Andrew Morton wrote:
> Guillaume Thouvenin <[email protected]> wrote:
> >
> > Hello,
> >
> > This module sends a signal to one or several processes (in user
> > space) when a fork occurs in the kernel. It relays information about
> > forks (parent and child pid) to a user space application.
> >
> So this permits ELSA to maintain a complete picture of the process/thread
> hierarchy? I guess that fits into the "do it in userspace" mantra -
> certainly hooking into fork() is a minimal way of doing this, although I
> wonder what the limitations are.
>
> Implementation-wise: there's a lot of code there and the interface is a bit
> awkward. Why not just feed that kobject you have there into
> kobject_uevent()?

Like Andrew suggested, I wrote a new patch (tested on 2.6.11-rc3-mm2)
that notifies to user space application the creation of a new process
when kernel forks by using the kobject_uevent() routine. This funtion
sends a new event (KOBJ_FORK) through the netlink interface. This new
event needs an environment (parent pid and child pid) so I used
send_uevent() like it is done in kobject_hotplug().

I tested this patch on a 2.6.11-rc3-mm2 kernel and there is a little
overhead when I compile a Linux kernel:

#time sh -c 'make O=/home/guill/build/k2610 bzImage &&
make O=/home/guill/build/k2610 modules'

with a vanilla kernel: real 8m10.797s
user 7m29.652s
sys 0m49.275s

with the forkuevent patch : real 8m16.189s
user 7m28.841s
sys 0m49.155s

I paste the diff at the end of the mail with wrap line disabled. is it
better to wrap the patch to 80 characters or is it ok like this?

Every comments are welcome. For exemple is it better to use a
structure instead of two pid_t parameters? I also like to know if this
is useful for someone else... thus, any feedback will be appreciate.


Regards,
Guillaume

guill@karakorum $ diffstat linux-2.6.11-rc3-mm2-forkuevent.patch
include/linux/kobject.h | 2 +
include/linux/kobject_uevent.h | 1
kernel/fork.c | 13 ++++++++
lib/kobject_uevent.c | 63 ++++++++++++++...++++++++++++
4 files changed, 79 insertions(+)

Signed-off-by: Guillaume Thouvenin <[email protected]>

diff -uprN -X dontdiff linux-2.6.11-rc3-mm2/include/linux/kobject.h linux-2.6.11-rc3-mm2-forkuevent/include/linux/kobject.h
--- linux-2.6.11-rc3-mm2/include/linux/kobject.h 2005-02-03 02:54:59.000000000 +0100
+++ linux-2.6.11-rc3-mm2-forkuevent/include/linux/kobject.h 2005-02-11 08:38:25.000000000 +0100
@@ -253,5 +253,7 @@ static inline int add_hotplug_env_var(ch
{ return 0; }
#endif

+extern void kobject_fork(struct kobject *, pid_t, pid_t);
+
#endif /* __KERNEL__ */
#endif /* _KOBJECT_H_ */
diff -uprN -X dontdiff linux-2.6.11-rc3-mm2/include/linux/kobject_uevent.h linux-2.6.11-rc3-mm2-forkuevent/include/linux/kobject_uevent.h
--- linux-2.6.11-rc3-mm2/include/linux/kobject_uevent.h 2005-02-03 02:54:38.000000000 +0100
+++ linux-2.6.11-rc3-mm2-forkuevent/include/linux/kobject_uevent.h 2005-02-11 08:38:25.000000000 +0100
@@ -29,6 +29,7 @@ enum kobject_action {
KOBJ_UMOUNT = (__force kobject_action_t) 0x05, /* umount event for block devices */
KOBJ_OFFLINE = (__force kobject_action_t) 0x06, /* offline event for hotplug devices */
KOBJ_ONLINE = (__force kobject_action_t) 0x07, /* online event for hotplug devices */
+ KOBJ_FORK = (__force kobject_action_t) 0x08, /* a child process has been created */
};


diff -uprN -X dontdiff linux-2.6.11-rc3-mm2/kernel/fork.c linux-2.6.11-rc3-mm2-forkuevent/kernel/fork.c
--- linux-2.6.11-rc3-mm2/kernel/fork.c 2005-02-11 08:37:48.000000000 +0100
+++ linux-2.6.11-rc3-mm2-forkuevent/kernel/fork.c 2005-02-11 08:39:33.000000000 +0100
@@ -41,6 +41,7 @@
#include <linux/profile.h>
#include <linux/rmap.h>
#include <linux/acct.h>
+#include <linux/kobject.h>

#include <asm/pgtable.h>
#include <asm/pgalloc.h>
@@ -57,6 +58,9 @@ int nr_threads; /* The idle threads do

int max_threads; /* tunable limit on nr_threads */

+/* kobject used to notify user space application when a fork occurs */
+struct kobject fork_kobj;
+
DEFINE_PER_CPU(unsigned long, process_counts) = 0;

__cacheline_aligned DEFINE_RWLOCK(tasklist_lock); /* outer */
@@ -148,6 +152,13 @@ void __init fork_init(unsigned long memp

init_task.signal->rlim[RLIMIT_NPROC].rlim_cur = max_threads/2;
init_task.signal->rlim[RLIMIT_NPROC].rlim_max = max_threads/2;
+
+ /*
+ * We init the fork_kobj which is the event sends when a fork
+ * occurs.
+ */
+ kobject_init(&fork_kobj);
+ kobject_set_name(&fork_kobj, "fork_kobj");
}

static struct task_struct *dup_task_struct(struct task_struct *orig)
@@ -1238,6 +1249,8 @@ long do_fork(unsigned long clone_flags,
if (unlikely (current->ptrace & PT_TRACE_VFORK_DONE))
ptrace_notify ((PTRACE_EVENT_VFORK_DONE << 8) | SIGTRAP);
}
+
+ kobject_fork(&fork_kobj, current->pid, p->pid);
} else {
free_pidmap(pid);
pid = PTR_ERR(p);
diff -uprN -X dontdiff linux-2.6.11-rc3-mm2/lib/kobject_uevent.c linux-2.6.11-rc3-mm2-forkuevent/lib/kobject_uevent.c
--- linux-2.6.11-rc3-mm2/lib/kobject_uevent.c 2005-02-11 08:37:48.000000000 +0100
+++ linux-2.6.11-rc3-mm2-forkuevent/lib/kobject_uevent.c 2005-02-11 08:38:25.000000000 +0100
@@ -46,6 +46,8 @@ static char *action_to_string(enum kobje
return "offline";
case KOBJ_ONLINE:
return "online";
+ case KOBJ_FORK:
+ return "fork";
default:
return NULL;
}
@@ -433,3 +435,64 @@ int add_hotplug_env_var(char **envp, int
EXPORT_SYMBOL(add_hotplug_env_var);

#endif /* CONFIG_HOTPLUG */
+
+/*
+ * The buffer will contain 1 integer which has 20 digits for
+ * 64 bits integer. So a size of 32 is large enough...
+ */
+#define FORK_BUFFER_SIZE 32
+
+/*
+ * number of env pointers is set to FORK_BUFFER_NB
+ * env[0] - Not used
+ * env[1] - Not used
+ * env[2] - parent pid
+ * env[3] - child pid
+ * env[4] - NULL
+ */
+#define FORK_BUFFER_NB 5
+
+/**
+ * kobject_fork - notify userspace when forking
+ *
+ * @kobj: struct kobject that the action is happening to
+ */
+void kobject_fork(struct kobject *kobj, pid_t parent, pid_t child)
+{
+ char *kobj_path = NULL;
+ char *action_string = NULL;
+ char **envp = NULL;
+ char ppid_string[FORK_BUFFER_SIZE];
+ char cpid_string[FORK_BUFFER_SIZE];
+
+ action_string = action_to_string(KOBJ_FORK);
+ if (!action_string)
+ return;
+
+ kobj_path = kobject_get_path(kobj, GFP_KERNEL);
+ if (!kobj_path)
+ return;
+
+ envp = kmalloc(FORK_BUFFER_NB * sizeof (char *), GFP_KERNEL);
+ if (!envp) {
+ kfree(kobj_path);
+ return;
+ }
+ memset (envp, 0x0, FORK_BUFFER_NB * sizeof (char *));
+
+ snprintf(ppid_string, FORK_BUFFER_SIZE, "%i", parent);
+ snprintf(cpid_string, FORK_BUFFER_SIZE, "%i", child);
+
+ envp[0] = "Not used";
+ envp[1] = "Not used";
+ envp[2] = ppid_string;
+ envp[3] = cpid_string;
+ envp[4] = NULL;
+
+ send_uevent(action_string, kobj_path, envp, GFP_KERNEL);
+
+ kfree(envp);
+ kfree(kobj_path);
+ return;
+}
+EXPORT_SYMBOL(kobject_fork);


2005-02-11 08:56:19

by Andrew Morton

[permalink] [raw]
Subject: Re: [RFC][PATCH 2.6.11-rc3-mm2] Relay Fork Module

Guillaume Thouvenin <[email protected]> wrote:
>
> On Mon, 2005-02-07 at 15:46 -0800, Andrew Morton wrote:
> > Guillaume Thouvenin <[email protected]> wrote:
> > >
> > > Hello,
> > >
> > > This module sends a signal to one or several processes (in user
> > > space) when a fork occurs in the kernel. It relays information about
> > > forks (parent and child pid) to a user space application.
> > >
> > So this permits ELSA to maintain a complete picture of the process/thread
> > hierarchy? I guess that fits into the "do it in userspace" mantra -
> > certainly hooking into fork() is a minimal way of doing this, although I
> > wonder what the limitations are.
> >
> > Implementation-wise: there's a lot of code there and the interface is a bit
> > awkward. Why not just feed that kobject you have there into
> > kobject_uevent()?
>
> Like Andrew suggested, I wrote a new patch (tested on 2.6.11-rc3-mm2)
> that notifies to user space application the creation of a new process
> when kernel forks by using the kobject_uevent() routine.

A bit smaller than the old one ;)

> This funtion
> sends a new event (KOBJ_FORK) through the netlink interface. This new
> event needs an environment (parent pid and child pid) so I used
> send_uevent() like it is done in kobject_hotplug().
>
> I tested this patch on a 2.6.11-rc3-mm2 kernel and there is a little
> overhead when I compile a Linux kernel:
>
> #time sh -c 'make O=/home/guill/build/k2610 bzImage &&
> make O=/home/guill/build/k2610 modules'
>
> with a vanilla kernel: real 8m10.797s
> user 7m29.652s
> sys 0m49.275s
>
> with the forkuevent patch : real 8m16.189s
> user 7m28.841s
> sys 0m49.155s

Was that when some process was monitoring the netlink socket?

We do need to find some way of avoiding all that setup work, just to drop
the constructed message on the floor because nobody was listening for it.

You could just do what send_uevent() does:

kobject_fork()
{
if (!uevent_sock)
return;

...
}

but we should expect that there will be something on the other end of the
uevent socket all the time.

Is there some simple way in which we can record whether there is actually
someone who will be interested in these messages? I can't immediately
think of one - if we rely on userspace to send on/off messages, then buggy
userspace will leave us in the wrong state.

Still, that's not a catastrophe. Perhaps invent some simple on/off
messaging on the netlink receive side? Or use a separate socket, perhaps,
if there's some quick way of working out if any process if prepared to
receive from it.


> I paste the diff at the end of the mail with wrap line disabled. is it
> better to wrap the patch to 80 characters or is it ok like this?

The patch came through OK. Wordwrapping it would be doubleplusungood.

> +/* kobject used to notify user space application when a fork occurs */
> +struct kobject fork_kobj;
> +

fork_kobj can have static scope.

> + */
> +void kobject_fork(struct kobject *kobj, pid_t parent, pid_t child)
> +{
> + char *kobj_path = NULL;
> + char *action_string = NULL;
> + char **envp = NULL;
> + char ppid_string[FORK_BUFFER_SIZE];
> + char cpid_string[FORK_BUFFER_SIZE];
> +
> + action_string = action_to_string(KOBJ_FORK);
> + if (!action_string)
> + return;
> +
> + kobj_path = kobject_get_path(kobj, GFP_KERNEL);
> + if (!kobj_path)
> + return;
> +
> + envp = kmalloc(FORK_BUFFER_NB * sizeof (char *), GFP_KERNEL);
> + if (!envp) {
> + kfree(kobj_path);
> + return;
> + }
> + memset (envp, 0x0, FORK_BUFFER_NB * sizeof (char *));
> +
> + snprintf(ppid_string, FORK_BUFFER_SIZE, "%i", parent);
> + snprintf(cpid_string, FORK_BUFFER_SIZE, "%i", child);
> +
> + envp[0] = "Not used";
> + envp[1] = "Not used";
> + envp[2] = ppid_string;
> + envp[3] = cpid_string;
> + envp[4] = NULL;
> +
> + send_uevent(action_string, kobj_path, envp, GFP_KERNEL);
> +
> + kfree(envp);
> + kfree(kobj_path);
> + return;
> +}
> +EXPORT_SYMBOL(kobject_fork);

hmm, I have a feeling that the above code is getting duplicated all over
the place. If there's some consolidation opportunity, that would be a nice
separate patch.

You should move your code inside the #ifdef CONFIG_KOBJECT_UEVENT section,
because it'll just waste space otherwise. Then make sure that the
appropriate stubs are in place in kobject.h so that the kernel still
compiles if !CONFIG_KOBJECT_UEVENT.

(It seems strange to use a "kobject" here. What's objective about a
fork()? I guess it'll do though).

2005-02-11 12:37:11

by Guillaume Thouvenin

[permalink] [raw]
Subject: Re: [RFC][PATCH 2.6.11-rc3-mm2] Relay Fork Module

On Fri, 2005-02-11 at 00:54 -0800, Andrew Morton wrote:
> > I tested this patch on a 2.6.11-rc3-mm2 kernel and there is a little
> > overhead when I compile a Linux kernel:
> >
> > #time sh -c 'make O=/home/guill/build/k2610 bzImage &&
> > make O=/home/guill/build/k2610 modules'
> >
> > with a vanilla kernel: real 8m10.797s
> > user 7m29.652s
> > sys 0m49.275s
> >
> > with the forkuevent patch : real 8m16.189s
> > user 7m28.841s
> > sys 0m49.155s
>
> Was that when some process was monitoring the netlink socket?

The test was done without monitoring. I ran another one with
monitoring and the result is:

real 8m12.747s
user 7m30.761s
sys 0.51.414s

As I only tested each case only once, I'm going to run the same test
five times to have a more accurate results.

Thank you very much for your comments, I'm carefully looking all of
them. I will send comments next week.

Regards,
Guillaume

2005-02-11 15:14:35

by Guillaume Thouvenin

[permalink] [raw]
Subject: Re: [RFC][PATCH 2.6.11-rc3-mm2] Relay Fork Module

On Fri, 2005-02-11 at 00:54 -0800, Andrew Morton wrote:

> > I tested this patch on a 2.6.11-rc3-mm2 kernel and there is a little
> > overhead when I compile a Linux kernel:
> >
> > #time sh -c 'make O=/home/guill/build/k2610 bzImage &&
> > make O=/home/guill/build/k2610 modules'
> >
> > with a vanilla kernel: real 8m10.797s
> > user 7m29.652s
> > sys 0m49.275s
> >
> > with the forkuevent patch : real 8m16.189s
> > user 7m28.841s
> > sys 0m49.155s
>
> Was that when some process was monitoring the netlink socket?

I used another patch for running those tests and here are the changes:

- struct kobject fork_kobjet is now static
- add a test that drop the construction of the message nobody is
listening on the netlink.
- put the code in a #ifdef CONFIG_KOBJECT_UEVENT section but I
directly add the #ifdef in the code instead of moving the code in
the existing #ifdef CONFIG_KOBJECT_UEVENT section.

There are still some open discussions like the duplicate code and the
use of the kobject here. I continue to work on that.

Here are results about performance impact. The test is the compilation
of 2.6.10 kernel. For monitoring the KOBJ_FORK event I'm using the
uevent_listen.c written by Kay Sievers. Three situation was tested:

1) Vanilla 2.6.11-rc3-mm2 kernel
2) 2.6.11-rc3-mm2 + patch (without monitoring)
3) 2.6.11-rc3-mm2 + patch + monitoring

command used :
# time sh -c 'make O=/home/guill/build/k2610 oldconfig && \
make O=/home/guill/build/k2610 bzImage && \
make O=/home/guill/build/k2610 modules'

Between each test the directory /home/guill/build/k2610 was destroyed,
a new one was created, a .config file was copied in the directory and
file system buffers are flushed with 'sync'. Here are the results:

| Real User Sys |
-----------|-----------------------------------|
Vanilla | 8m06.808s 7m32.044s 0m51.647s |
| 8m07.144s 7m32.804s 0m51.308s |
| 8m06.856s 7m31.414s 0m51.711s |
| 8m06.875s 7m32.082s 0m51.885s |
| 8m07.205s 7m32.140s 0m51.766s |
-----------|-----------------------------------|
Kernel + | 8m15.884s 7m31.838s 0m50.616s |
Patch | 8m08.506s 7m34.154s 0m51.005s |
| 8m07.972s 7m33.761s 0m51.416s |
| 8m08.006s 7m33.683s 0m51.206s |
| 8m09.138s 7m34.212s 0m51.087s |
-----------|-----------------------------------|
Kernel + | 8m08.149s 7m33.590s 0m51.527s |
Patch + | 8m08.595s 7m33.674s 0m51.380s |
Monitoring | 8m08.374s 7m34.011s 0m51.282s |
| 8m09.317s 7m34.563s 0m51.259s |
| 8m08.121s 7m35.412s 0m51.069s |
-----------------------------------------------/


Regards,
Guillaume

Signed-off-by: Guillaume Thouvenin <[email protected]>

diff -uprN -X dontdiff linux-2.6.11-rc3-mm2/include/linux/kobject.h linux-2.6.11-rc3-mm2-forkuevent/include/linux/kobject.h
--- linux-2.6.11-rc3-mm2/include/linux/kobject.h 2005-02-03 02:54:59.000000000 +0100
+++ linux-2.6.11-rc3-mm2-forkuevent/include/linux/kobject.h 2005-02-11 11:03:13.000000000 +0100
@@ -253,5 +253,7 @@ static inline int add_hotplug_env_var(ch
{ return 0; }
#endif

+extern void kobject_fork(struct kobject *, pid_t, pid_t);
+
#endif /* __KERNEL__ */
#endif /* _KOBJECT_H_ */
diff -uprN -X dontdiff linux-2.6.11-rc3-mm2/include/linux/kobject_uevent.h linux-2.6.11-rc3-mm2-forkuevent/include/linux/kobject_uevent.h
--- linux-2.6.11-rc3-mm2/include/linux/kobject_uevent.h 2005-02-03 02:54:38.000000000 +0100
+++ linux-2.6.11-rc3-mm2-forkuevent/include/linux/kobject_uevent.h 2005-02-11 11:03:44.000000000 +0100
@@ -29,6 +29,7 @@ enum kobject_action {
KOBJ_UMOUNT = (__force kobject_action_t) 0x05, /* umount event for block devices */
KOBJ_OFFLINE = (__force kobject_action_t) 0x06, /* offline event for hotplug devices */
KOBJ_ONLINE = (__force kobject_action_t) 0x07, /* online event for hotplug devices */
+ KOBJ_FORK = (__force kobject_action_t) 0x08, /* a child process has been created */
};


diff -uprN -X dontdiff linux-2.6.11-rc3-mm2/kernel/fork.c linux-2.6.11-rc3-mm2-forkuevent/kernel/fork.c
--- linux-2.6.11-rc3-mm2/kernel/fork.c 2005-02-11 11:00:18.000000000 +0100
+++ linux-2.6.11-rc3-mm2-forkuevent/kernel/fork.c 2005-02-11 11:05:43.000000000 +0100
@@ -41,6 +41,7 @@
#include <linux/profile.h>
#include <linux/rmap.h>
#include <linux/acct.h>
+#include <linux/kobject.h>

#include <asm/pgtable.h>
#include <asm/pgalloc.h>
@@ -57,6 +58,9 @@ int nr_threads; /* The idle threads do

int max_threads; /* tunable limit on nr_threads */

+/* kobject used to notify user space application when a fork occurs */
+static struct kobject fork_kobj;
+
DEFINE_PER_CPU(unsigned long, process_counts) = 0;

__cacheline_aligned DEFINE_RWLOCK(tasklist_lock); /* outer */
@@ -148,6 +152,13 @@ void __init fork_init(unsigned long memp

init_task.signal->rlim[RLIMIT_NPROC].rlim_cur = max_threads/2;
init_task.signal->rlim[RLIMIT_NPROC].rlim_max = max_threads/2;
+
+ /*
+ * We init the fork_kobj which is the event sends when a fork
+ * occurs.
+ */
+ kobject_init(&fork_kobj);
+ kobject_set_name(&fork_kobj, "fork_kobj");
}

static struct task_struct *dup_task_struct(struct task_struct *orig)
@@ -1238,6 +1249,8 @@ long do_fork(unsigned long clone_flags,
if (unlikely (current->ptrace & PT_TRACE_VFORK_DONE))
ptrace_notify ((PTRACE_EVENT_VFORK_DONE << 8) | SIGTRAP);
}
+
+ kobject_fork(&fork_kobj, current->pid, p->pid);
} else {
free_pidmap(pid);
pid = PTR_ERR(p);
diff -uprN -X dontdiff linux-2.6.11-rc3-mm2/lib/kobject_uevent.c linux-2.6.11-rc3-mm2-forkuevent/lib/kobject_uevent.c
--- linux-2.6.11-rc3-mm2/lib/kobject_uevent.c 2005-02-11 11:00:18.000000000 +0100
+++ linux-2.6.11-rc3-mm2-forkuevent/lib/kobject_uevent.c 2005-02-11 11:08:23.000000000 +0100
@@ -46,6 +46,8 @@ static char *action_to_string(enum kobje
return "offline";
case KOBJ_ONLINE:
return "online";
+ case KOBJ_FORK:
+ return "fork";
default:
return NULL;
}
@@ -433,3 +435,69 @@ int add_hotplug_env_var(char **envp, int
EXPORT_SYMBOL(add_hotplug_env_var);

#endif /* CONFIG_HOTPLUG */
+
+/*
+ * The buffer will contain 1 integer which has 20 digits for
+ * 64 bits integer. So a size of 32 is large enough...
+ */
+#define FORK_BUFFER_SIZE 32
+
+/*
+ * number of env pointers is set to FORK_BUFFER_NB
+ * env[0] - Not used
+ * env[1] - Not used
+ * env[2] - parent pid
+ * env[3] - child pid
+ * env[4] - NULL
+ */
+#define FORK_BUFFER_NB 5
+
+/**
+ * kobject_fork - notify userspace when forking
+ *
+ * @kobj: struct kobject that the action is happening to
+ */
+void kobject_fork(struct kobject *kobj, pid_t parent, pid_t child)
+{
+#ifdef CONFIG_KOBJECT_UEVENT
+ char *kobj_path = NULL;
+ char *action_string = NULL;
+ char **envp = NULL;
+ char ppid_string[FORK_BUFFER_SIZE];
+ char cpid_string[FORK_BUFFER_SIZE];
+
+ if (!uevent_sock)
+ return;
+
+ action_string = action_to_string(KOBJ_FORK);
+ if (!action_string)
+ return;
+
+ kobj_path = kobject_get_path(kobj, GFP_KERNEL);
+ if (!kobj_path)
+ return;
+
+ envp = kmalloc(FORK_BUFFER_NB * sizeof (char *), GFP_KERNEL);
+ if (!envp) {
+ kfree(kobj_path);
+ return;
+ }
+ memset (envp, 0x0, FORK_BUFFER_NB * sizeof (char *));
+
+ snprintf(ppid_string, FORK_BUFFER_SIZE, "%i", parent);
+ snprintf(cpid_string, FORK_BUFFER_SIZE, "%i", child);
+
+ envp[0] = "Not used";
+ envp[1] = "Not used";
+ envp[2] = ppid_string;
+ envp[3] = cpid_string;
+ envp[4] = NULL;
+
+ send_uevent(action_string, kobj_path, envp, GFP_KERNEL);
+
+ kfree(envp);
+ kfree(kobj_path);
+#endif /* CONFIG_KOBJECT_UEVENT */
+ return;
+}
+EXPORT_SYMBOL(kobject_fork);


2005-02-11 19:19:37

by Greg KH

[permalink] [raw]
Subject: Re: [RFC][PATCH 2.6.11-rc3-mm2] Relay Fork Module

On Fri, Feb 11, 2005 at 04:08:40PM +0100, Guillaume Thouvenin wrote:
> +void kobject_fork(struct kobject *kobj, pid_t parent, pid_t child)
> +{
> +#ifdef CONFIG_KOBJECT_UEVENT

No, provide two different functions. In a header file make it a static
inline function that does nothing if this option is not selected, so as
to make the code just go away, and the call is never made.


> + char *kobj_path = NULL;
> + char *action_string = NULL;
> + char **envp = NULL;
> + char ppid_string[FORK_BUFFER_SIZE];
> + char cpid_string[FORK_BUFFER_SIZE];
> +
> + if (!uevent_sock)
> + return;
> +
> + action_string = action_to_string(KOBJ_FORK);
> + if (!action_string)
> + return;
> +
> + kobj_path = kobject_get_path(kobj, GFP_KERNEL);
> + if (!kobj_path)
> + return;

How is there a path for a kobject that is never registered with sysfs?

I agree with Andrew, why are you using a kobject for this? Have you
looked at the "connector" code that is in the -mm tree? That might be a
better solution for this, and it will be going into the kernel tree
after 2.6.11 is released.

> +EXPORT_SYMBOL(kobject_fork);

EXPORT_SYMBOL_GPL() for something like this please.

thanks,

greg k-h

2005-02-14 08:26:55

by Guillaume Thouvenin

[permalink] [raw]
Subject: Re: [RFC][PATCH 2.6.11-rc3-mm2] Relay Fork Module

On Fri, 2005-02-11 at 11:11 -0800, Greg KH wrote:
> > + char *kobj_path = NULL;
> > + char *action_string = NULL;
> > + char **envp = NULL;
> > + char ppid_string[FORK_BUFFER_SIZE];
> > + char cpid_string[FORK_BUFFER_SIZE];
> > +
> > + if (!uevent_sock)
> > + return;
> > +
> > + action_string = action_to_string(KOBJ_FORK);
> > + if (!action_string)
> > + return;
> > +
> > + kobj_path = kobject_get_path(kobj, GFP_KERNEL);
> > + if (!kobj_path)
> > + return;
>
> How is there a path for a kobject that is never registered with sysfs?

My kobject has a name, kobject_set_name(&fork_kobj, "fork_kobj"), and no
parent so I thought that the path returned by kobject_get_path() was
"/fork_kobj" even if the kobject is not registered with sysfs. As
send_uevent() function needs an object path, I used the
kobject_get_path() routine.

> I agree with Andrew, why are you using a kobject for this? Have you
> looked at the "connector" code that is in the -mm tree? That might be a
> better solution for this, and it will be going into the kernel tree
> after 2.6.11 is released.

I'm using kobject because it allows to notify user space application by
sending an event and as I need to send a kernel event (fork event) to a
user space application I thought about kobject. Do you think that it's
not the good solution because it's a too big mechanism for what I want
to do?

I haven't looked at the "connector" code and I will have a look now.
Thank you very much to point this.

Thank you for your comments and your help,
Regards,
Guillaume

2005-02-14 18:15:43

by Greg KH

[permalink] [raw]
Subject: Re: [RFC][PATCH 2.6.11-rc3-mm2] Relay Fork Module

On Mon, Feb 14, 2005 at 09:26:49AM +0100, Guillaume Thouvenin wrote:
>
> I'm using kobject because it allows to notify user space application by
> sending an event and as I need to send a kernel event (fork event) to a
> user space application I thought about kobject. Do you think that it's
> not the good solution because it's a too big mechanism for what I want
> to do?

Yes, I think it is too "big" of a solution, take a look at the connector
code, it is what you want to use instead.

thanks,

greg k-h