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