Re: [patch] ipv4: initialize arp_tbl rw lock
Tried to figure out what is causing the delays I experienced when I replaced module_init() in af_inet.c with fs_initcall(). After all it turned out that synchronize_net() which is basicically nothing else than synchronize_rcu() sometimes takes several seconds to complete?! No idea why that is... callchain: inet_init() - inet_register_protosw() - synchronize_net() The problem can't be rcu_init(), that gets done very early in init/main.c Maybe it's some timer or something else specific to s390? It could also be that there's perhaps nothing to context switch to, thus the RCU takes forever to happen. Yes, it's more or less s390 specific. What happens is the following: synchronize_rcu() enqueues an RCU callback on cpu 0. Later on cpu 0 handles a bunch of RCU batches, but without handling this specific request (it's in rdp-curlist). Since this cpu has nothing else to do it enters cpu_idle() (it's a nohz idle, therefore it might be quite a long time in idle state). While cpu 0 is in idle state cpu 2 calls cpu_quiet() which in turn will call rcu_start_batch(). If cpu 0 would run now, it would notice rdp-curlist moved to rdp-donelist and that there is something to do. Unfortunately it doesn't get notified from rcu_start_batch(). That's why I ended up waiting several seconds until finally some interrupt arrived at cpu 0 which made things go on finally. Avoiding this could be done if we look at rdp-curlist before going into a nohz idle wait, or we could send an interprocessor interrupt to idle cpus. Sending an interrupt while looking only on nohz_cpu_mask seems to be a bit racy, since other cpus might have entered cpu idle after nohz_cpu_mask has been read... At least the initcall change for inet_init() can go in, since it just revealed a problem that we have anyway. - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch] ipv4: initialize arp_tbl rw lock
As spinlock debugging still does not work with the qeth driver I want to pick up the discussion. Does something like the patch below work? But this all begs the question, what happens if you want to dig into the internals of a protocol which is built modular and hasn't been loaded yet? diff --git a/include/linux/init.h b/include/linux/init.h index 93dcbe1..8169f25 100644 --- a/include/linux/init.h +++ b/include/linux/init.h @@ -95,8 +95,9 @@ #define postcore_initcall(fn) __define_ #define arch_initcall(fn)__define_initcall(3,fn) #define subsys_initcall(fn) __define_initcall(4,fn) #define fs_initcall(fn) __define_initcall(5,fn) -#define device_initcall(fn) __define_initcall(6,fn) -#define late_initcall(fn)__define_initcall(7,fn) +#define net_initcall(fn) __define_initcall(6,fn) +#define device_initcall(fn) __define_initcall(7,fn) +#define late_initcall(fn)__define_initcall(8,fn) #define __initcall(fn) device_initcall(fn) diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c index dc206f1..9803a57 100644 --- a/net/ipv4/af_inet.c +++ b/net/ipv4/af_inet.c @@ -1257,7 +1257,7 @@ out_unregister_udp_proto: goto out; } -module_init(inet_init); +net_initcall(inet_init); That's exactly the same thing that I tried to. It didn't work for me since I saw sometimes the described rcu_update latencies. Today I was able to boot the machine 30 times and just saw it once... Not very helpful for debugging this :( Btw.: I guess the linker scripts need an update too, so that the new .initcall8.init section doesn't get discarded. - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch] ipv4: initialize arp_tbl rw lock
As spinlock debugging still does not work with the qeth driver I want to pick up the discussion. On Saturday 08 April 2006 12:12, Andrew Morton wrote: Heiko Carstens [EMAIL PROTECTED] wrote: [...] -vmlinux-main := $(core-y) $(libs-y) $(drivers-y) $(net-y) +vmlinux-main := $(core-y) $(libs-y) $(net-y) $(drivers-y) wonders what this will break What about putting this patch into mm and find out? I have a bad feeling that one day we're going to come unstuck with this practice. Is there anything which dictates that the linker has to lay sections out in .o-file-order? Perhaps net initcalls should be using something higher priority than device_initcall(). I agree that the initcall order offers a lot of room for improvement (like dependencies). Is anybody aware of any work into this direction? -- Mit freundlichen Grüßen / Best Regards Christian Borntraeger Linux Software Engineer zSeries Linux Virtualization - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch] ipv4: initialize arp_tbl rw lock
From: Christian Borntraeger [EMAIL PROTECTED] Date: Wed, 19 Apr 2006 12:45:48 +0200 As spinlock debugging still does not work with the qeth driver I want to pick up the discussion. Does something like the patch below work? But this all begs the question, what happens if you want to dig into the internals of a protocol which is built modular and hasn't been loaded yet? diff --git a/include/linux/init.h b/include/linux/init.h index 93dcbe1..8169f25 100644 --- a/include/linux/init.h +++ b/include/linux/init.h @@ -95,8 +95,9 @@ #define postcore_initcall(fn) __define_ #define arch_initcall(fn) __define_initcall(3,fn) #define subsys_initcall(fn)__define_initcall(4,fn) #define fs_initcall(fn)__define_initcall(5,fn) -#define device_initcall(fn)__define_initcall(6,fn) -#define late_initcall(fn) __define_initcall(7,fn) +#define net_initcall(fn) __define_initcall(6,fn) +#define device_initcall(fn)__define_initcall(7,fn) +#define late_initcall(fn) __define_initcall(8,fn) #define __initcall(fn) device_initcall(fn) diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c index dc206f1..9803a57 100644 --- a/net/ipv4/af_inet.c +++ b/net/ipv4/af_inet.c @@ -1257,7 +1257,7 @@ out_unregister_udp_proto: goto out; } -module_init(inet_init); +net_initcall(inet_init); /* */ - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch] ipv4: initialize arp_tbl rw lock
Ok, so the problem seems to be that inet_init gets called after qeth_init. Looking at the top level Makefile this seems to be true for all network drivers in drivers/net/ and drivers/s390/net/ since we have vmlinux-main := $(core-y) $(libs-y) $(drivers-y) $(net-y) The patch below works for me... I guess there must be a better way to make sure that any networking driver's initcall is made before inet_init? Signed-off-by: Heiko Carstens [EMAIL PROTECTED] We could make inet_init() a subsystem init but I vaguely recall that we were doing that at one point and it broke things for some reason. Perhaps fs_initcall() would work better. Or if that causes problems we could create a net_initcall() that sits between fs_initcall() and device_initcall(). Or any other ideas? Tried to figure out what is causing the delays I experienced when I replaced module_init() in af_inet.c with fs_initcall(). After all it turned out that synchronize_net() which is basicically nothing else than synchronize_rcu() sometimes takes several seconds to complete?! No idea why that is... callchain: inet_init() - inet_register_protosw() - synchronize_net() - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch] ipv4: initialize arp_tbl rw lock
From: Heiko Carstens [EMAIL PROTECTED] Date: Sat, 15 Apr 2006 09:27:45 +0200 Tried to figure out what is causing the delays I experienced when I replaced module_init() in af_inet.c with fs_initcall(). After all it turned out that synchronize_net() which is basicically nothing else than synchronize_rcu() sometimes takes several seconds to complete?! No idea why that is... callchain: inet_init() - inet_register_protosw() - synchronize_net() The problem can't be rcu_init(), that gets done very early in init/main.c Maybe it's some timer or something else specific to s390? It could also be that there's perhaps nothing to context switch to, thus the RCU takes forever to happen. - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch] ipv4: initialize arp_tbl rw lock
callchain: inet_init() - inet_register_protosw() - synchronize_net() The problem can't be rcu_init(), that gets done very early in init/main.c Maybe it's some timer or something else specific to s390? It could also be that there's perhaps nothing to context switch to, thus the RCU takes forever to happen. Changing inet_init to fs_initcall() moves it way up the chain... There are quite a few __initcall()s (way is this mapped to device_initcall()?) and module_init()s in places where I would never have expected them (e.g. kernel/). After all the dependencies are anything but obvious to me. The only obvious solution which fixes my problem would be to convert qeth's module_init() to late_initcall(). - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch] ipv4: initialize arp_tbl rw lock
The qeth driver makes use of the arp_tbl rw lock. CONFIG_DEBUG_SPINLOCK detects that this lock is not initialized as it is supposed to be. This is a initialization order problem then, because: arp_init neigh_table_init rwlock_init does the initialization already. So fix the initialization sequence of the qeth driver or you will have other problems. My impression was the -rt folks wanted all lock initializations t be done at runtime not compile time. Ok, so the problem seems to be that inet_init gets called after qeth_init. Looking at the top level Makefile this seems to be true for all network drivers in drivers/net/ and drivers/s390/net/ since we have vmlinux-main := $(core-y) $(libs-y) $(drivers-y) $(net-y) The patch below works for me... I guess there must be a better way to make sure that any networking driver's initcall is made before inet_init? Signed-off-by: Heiko Carstens [EMAIL PROTECTED] --- Makefile |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index b401942..c5cea07 100644 --- a/Makefile +++ b/Makefile @@ -567,7 +567,7 @@ # # System.map is generated to document addresses of all kernel symbols vmlinux-init := $(head-y) $(init-y) -vmlinux-main := $(core-y) $(libs-y) $(drivers-y) $(net-y) +vmlinux-main := $(core-y) $(libs-y) $(net-y) $(drivers-y) vmlinux-all := $(vmlinux-init) $(vmlinux-main) vmlinux-lds := arch/$(ARCH)/kernel/vmlinux.lds - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch] ipv4: initialize arp_tbl rw lock
From: Heiko Carstens [EMAIL PROTECTED] Date: Sat, 8 Apr 2006 12:02:13 +0200 Ok, so the problem seems to be that inet_init gets called after qeth_init. Looking at the top level Makefile this seems to be true for all network drivers in drivers/net/ and drivers/s390/net/ since we have vmlinux-main := $(core-y) $(libs-y) $(drivers-y) $(net-y) The patch below works for me... I guess there must be a better way to make sure that any networking driver's initcall is made before inet_init? Signed-off-by: Heiko Carstens [EMAIL PROTECTED] We could make inet_init() a subsystem init but I vaguely recall that we were doing that at one point and it broke things for some reason. Perhaps fs_initcall() would work better. Or if that causes problems we could create a net_initcall() that sits between fs_initcall() and device_initcall(). Or any other ideas? - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch] ipv4: initialize arp_tbl rw lock
Heiko Carstens [EMAIL PROTECTED] wrote: Ok, so the problem seems to be that inet_init gets called after qeth_init. Looking at the top level Makefile this seems to be true for all network drivers in drivers/net/ and drivers/s390/net/ since we have vmlinux-main := $(core-y) $(libs-y) $(drivers-y) $(net-y) The patch below works for me... I guess there must be a better way to make sure that any networking driver's initcall is made before inet_init? Signed-off-by: Heiko Carstens [EMAIL PROTECTED] --- Makefile |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index b401942..c5cea07 100644 --- a/Makefile +++ b/Makefile @@ -567,7 +567,7 @@ # # System.map is generated to document addresses of all kernel symbols vmlinux-init := $(head-y) $(init-y) -vmlinux-main := $(core-y) $(libs-y) $(drivers-y) $(net-y) +vmlinux-main := $(core-y) $(libs-y) $(net-y) $(drivers-y) wonders what this will break I have a bad feeling that one day we're going to come unstuck with this practice. Is there anything which dictates that the linker has to lay sections out in .o-file-order? Perhaps net initcalls should be using something higher priority than device_initcall(). - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch] ipv4: initialize arp_tbl rw lock
We could make inet_init() a subsystem init but I vaguely recall that we were doing that at one point and it broke things for some reason. Perhaps fs_initcall() would work better. Or if that causes problems we could create a net_initcall() that sits between fs_initcall() and device_initcall(). Or any other ideas? Just tried fs_initcall() and net_initcall(). Both seem to have some side effects: Symptom is that console output sometimes hangs for several seconds at: NET: Registered protocol family 2 while all cpus are in cpu_idle(). Heiko - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch] ipv4: initialize arp_tbl rw lock
On Sat, Apr 08, 2006 at 03:14:04AM -0700, David S. Miller wrote: Perhaps fs_initcall() would work better. Or if that causes problems we could create a net_initcall() that sits between fs_initcall() and device_initcall(). fs_initcall() seems to be used mainly for init after subsystem stuff. Within fs/ only pipe.c uses fs_initcall(). If we are going to overload the usage of fs_initcall() even more then we should maybe try to rename it? Sam - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch] ipv4: initialize arp_tbl rw lock
From: Heiko Carstens [EMAIL PROTECTED]
The qeth driver makes use of the arp_tbl rw lock. CONFIG_DEBUG_SPINLOCK
detects that this lock is not initialized as it is supposed to be.
Signed-off-by: Heiko Carstens [EMAIL PROTECTED]
---
net/ipv4/arp.c |1 +
1 file changed, 1 insertion(+)
diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c
index 041dadd..ea54216 100644
--- a/net/ipv4/arp.c
+++ b/net/ipv4/arp.c
@@ -202,6 +202,7 @@ struct neigh_table arp_tbl = {
.gc_thresh1 = 128,
.gc_thresh2 = 512,
.gc_thresh3 = 1024,
+ .lock = RW_LOCK_UNLOCKED,
};
int arp_mc_map(u32 addr, u8 *haddr, struct net_device *dev, int dir)
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch] ipv4: initialize arp_tbl rw lock
From: Heiko Carstens [EMAIL PROTECTED] Date: Fri, 7 Apr 2006 10:15:33 +0200 From: Heiko Carstens [EMAIL PROTECTED] The qeth driver makes use of the arp_tbl rw lock. CONFIG_DEBUG_SPINLOCK detects that this lock is not initialized as it is supposed to be. Signed-off-by: Heiko Carstens [EMAIL PROTECTED] As Stephen Hemminger pointed out this change is bogus, the lock is initialized at run time by the ARP layer. - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch] ipv4: initialize arp_tbl rw lock
On Fri, 7 Apr 2006 10:15:33 +0200 Heiko Carstens [EMAIL PROTECTED] wrote: From: Heiko Carstens [EMAIL PROTECTED] The qeth driver makes use of the arp_tbl rw lock. CONFIG_DEBUG_SPINLOCK detects that this lock is not initialized as it is supposed to be. Signed-off-by: Heiko Carstens [EMAIL PROTECTED] --- This is a initialization order problem then, because: arp_init neigh_table_init rwlock_init does the initialization already. So fix the initialization sequence of the qeth driver or you will have other problems. My impression was the -rt folks wanted all lock initializations t be done at runtime not compile time. - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
