netlink vs kobject_uevent ordering

netlink vs kobject_uevent ordering

[Anton Blanchard]

Hi,

I noticed kobject_uevent was failing to init on my box. It looks like
both netlink and kobject_uevent are marked core_initcall and we may not
do them in the correct order.

I guess the recent changes to netlink caused this:

http://linux.bkbits.net:8080/linux-2.5/cset@41896b1dIiNgXpwhgimeurIqPpofbw?nav=index.html|ChangeSet@-2d

Anton

Re: netlink vs kobject_uevent ordering

[Greg KH]

On Fri, Nov 05, 2004 at 02:43:17AM +1100, Anton Blanchard wrote:
> 
> Hi,
> 
> I noticed kobject_uevent was failing to init on my box. It looks like
> both netlink and kobject_uevent are marked core_initcall and we may not
> do them in the correct order.
> 
> I guess the recent changes to netlink caused this:
> 
> http://linux.bkbits.net:8080/linux-2.5/cset@41896b1dIiNgXpwhgimeurIqPpofbw?nav=index.html|ChangeSet@-2d

Hm, I don't think that patch caused the reversal, it seems like we've
always linked this in the opposite order as my System.map before this
patch went in shows:
	c04b016c t __initcall_kobject_uevent_init
	c04b0170 t __initcall_netlink_proto_init

So, Robert and Kay, any thoughts as to how this has ever worked at boot
time in the past?

thanks,

greg k-h

Re: netlink vs kobject_uevent ordering

[Robert Love]

On Thu, 2004-11-04 at 10:05 -0800, Greg KH wrote:

> So, Robert and Kay, any thoughts as to how this has ever worked at boot
> time in the past?

Weird.  I don't have a clue, but clearly it did work.

Similar thing here:

	c04b60cc t __initcall_kobject_uevent_init
	c04b60d0 t __initcall_netlink_proto_init

Dunno why it ever worked, but it at least used to.

	Robert Love

Re: netlink vs kobject_uevent ordering

[David S. Miller]

On Thu, 04 Nov 2004 13:13:53 -0500
Robert Love <rml@novell.com> wrote:

> On Thu, 2004-11-04 at 10:05 -0800, Greg KH wrote:
> 
> > So, Robert and Kay, any thoughts as to how this has ever worked at boot
> > time in the past?
> 
> Weird.  I don't have a clue, but clearly it did work.

It might have really failing recently because nl_table in
net/netlink/af_netlink.c has become dynamically allocated
memory setup by netlink_proto_init().

[patch] kobject_uevent: fix init ordering

[Robert Love]

Greg!

Looks like kobject_uevent_init is executed before netlink_proto_init and
consequently always fails.  Not cool.

Attached patch switches the initialization over from core_initcall (init
level 1) to postcore_initcall (init level 2).  Netlink's initialization
is done in core_initcall, so this should fix the problem.  We should be
fine waiting until postcore_initcall.

Also a couple white space changes mixed in, because I am anal.

	Robert Love


fix kobject_uevent init ordering

 lib/kobject_uevent.c |    8 +++-----
 1 files changed, 3 insertions(+), 5 deletions(-)

diff -urN linux-2.6.10-rc1/lib/kobject_uevent.c linux/lib/kobject_uevent.c
--- linux-2.6.10-rc1/lib/kobject_uevent.c	2004-10-25 16:17:09.000000000 -0400
+++ linux/lib/kobject_uevent.c	2004-11-04 13:20:32.731836880 -0500
@@ -54,7 +54,7 @@
  * gfp_mask:
  */
 static int send_uevent(const char *signal, const char *obj, const void *buf,
-			int buflen, int gfp_mask)
+		       int buflen, int gfp_mask)
 {
 	struct sk_buff *skb;
 	char *pos;
@@ -105,9 +105,8 @@
 		sprintf(attrpath, "%s/%s", path, attr->name);
 		rc = send_uevent(signal, attrpath, NULL, 0, gfp_mask);
 		kfree(attrpath);
-	} else {
+	} else
 		rc = send_uevent(signal, path, NULL, 0, gfp_mask);
-	}
 
 exit:
 	kfree(path);
@@ -133,7 +132,6 @@
 {
 	return do_kobject_uevent(kobj, action, attr, GFP_ATOMIC);
 }
-
 EXPORT_SYMBOL_GPL(kobject_uevent_atomic);
 
 static int __init kobject_uevent_init(void)
@@ -149,7 +147,7 @@
 	return 0;
 }
 
-core_initcall(kobject_uevent_init);
+postcore_initcall(kobject_uevent_init);
 
 #else
 static inline int send_uevent(const char *signal, const char *obj,

Re: [patch] kobject_uevent: fix init ordering

[Kay Sievers]

On Thu, 2004-11-04 at 13:27 -0500, Robert Love wrote:
> Greg!
> 
> Looks like kobject_uevent_init is executed before netlink_proto_init and
> consequently always fails.  Not cool.
> 
> Attached patch switches the initialization over from core_initcall (init
> level 1) to postcore_initcall (init level 2).  Netlink's initialization
> is done in core_initcall, so this should fix the problem.  We should be
> fine waiting until postcore_initcall.

Looks good. Don't know why this never failed on any kernel I used.
Does the failure happens on a SMP kernel?

>  static int send_uevent(const char *signal, const char *obj, const void *buf,
> -			int buflen, int gfp_mask)
> +		       int buflen, int gfp_mask)
                         ^^^^^^^^^^
This has changed and will not apply.

Best,
Kay

Re: [patch] kobject_uevent: fix init ordering

[Robert Love]

On Thu, 2004-11-04 at 20:04 +0100, Kay Sievers wrote:

Hey, Kay.

> Looks good. Don't know why this never failed on any kernel I used.
> Does the failure happens on a SMP kernel?

In the original patches, I had the initialization done as module_init(),
which is done very late in the init ordering.  At some point it was
changed to core_initcall(), which is the very first things initialized.

> >  static int send_uevent(const char *signal, const char *obj, const void *buf,
> > -			int buflen, int gfp_mask)
> > +		       int buflen, int gfp_mask)

Ugh.  I am sure Greg can sort it out, but following patch has just the
init call ordering change.

	Robert Love


fix kobject_uevent init ordering

 lib/kobject_uevent.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff -urN linux-2.6.10-rc1/lib/kobject_uevent.c linux/lib/kobject_uevent.c
--- linux-2.6.10-rc1/lib/kobject_uevent.c	2004-10-25 16:17:09.000000000 -0400
+++ linux/lib/kobject_uevent.c	2004-11-04 13:20:32.731836880 -0500
@@ -149,7 +147,7 @@
 	return 0;
 }
 
-core_initcall(kobject_uevent_init);
+postcore_initcall(kobject_uevent_init);
 
 #else
 static inline int send_uevent(const char *signal, const char *obj,

Re: [patch] kobject_uevent: fix init ordering

[Robert Love]

On Thu, 2004-11-04 at 13:27 -0500, Robert Love wrote:

> Looks like kobject_uevent_init is executed before netlink_proto_init and
> consequently always fails.  Not cool.
> 
> Attached patch switches the initialization over from core_initcall (init
> level 1) to postcore_initcall (init level 2).  Netlink's initialization
> is done in core_initcall, so this should fix the problem.  We should be
> fine waiting until postcore_initcall.
> 
> Also a couple white space changes mixed in, because I am anal.

Greg, sir, here is a patch rediff'ed off current BK.

	Robert Love


fix init call ordering for kobject_uevent

Signed-Off-By: Robert Love <rml@novell.com>

 lib/kobject_uevent.c |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)

diff -urN linux-2.6.10-rc1-bk14/lib/kobject_uevent.c linux/lib/kobject_uevent.c
--- linux-2.6.10-rc1-bk14/lib/kobject_uevent.c	2004-11-04 14:20:10.431943792 -0500
+++ linux/lib/kobject_uevent.c	2004-11-04 14:26:54.056583520 -0500
@@ -120,9 +120,8 @@
 		sprintf(attrpath, "%s/%s", path, attr->name);
 		rc = send_uevent(signal, attrpath, NULL, gfp_mask);
 		kfree(attrpath);
-	} else {
+	} else
 		rc = send_uevent(signal, path, NULL, gfp_mask);
-	}
 
 exit:
 	kfree(path);
@@ -148,7 +147,6 @@
 {
 	return do_kobject_uevent(kobj, action, attr, GFP_ATOMIC);
 }
-
 EXPORT_SYMBOL_GPL(kobject_uevent_atomic);
 
 static int __init kobject_uevent_init(void)
@@ -164,7 +162,7 @@
 	return 0;
 }
 
-core_initcall(kobject_uevent_init);
+postcore_initcall(kobject_uevent_init);
 
 #else
 static inline int send_uevent(const char *signal, const char *obj,

Re: [patch] kobject_uevent: fix init ordering

[Greg KH]

On Thu, Nov 04, 2004 at 02:28:52PM -0500, Robert Love wrote:
> On Thu, 2004-11-04 at 13:27 -0500, Robert Love wrote:
> 
> > Looks like kobject_uevent_init is executed before netlink_proto_init and
> > consequently always fails.  Not cool.
> > 
> > Attached patch switches the initialization over from core_initcall (init
> > level 1) to postcore_initcall (init level 2).  Netlink's initialization
> > is done in core_initcall, so this should fix the problem.  We should be
> > fine waiting until postcore_initcall.
> > 
> > Also a couple white space changes mixed in, because I am anal.
> 
> Greg, sir, here is a patch rediff'ed off current BK.

"Sir"?  Geesh, I'm not that old looking am I?  :)

applied, thanks.

greg k-h