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
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
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) > > 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
> > 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
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
> > 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
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
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
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) 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
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
> > 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: 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
[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
