[2.6 patch] drivers/net/loopback.c: convert to module_init()

[2.6 patch] drivers/net/loopback.c: convert to module_init()

[Adrian Bunk]

This patch converts drivers/net/loopback.c to using module_init().

Signed-off-by: Adrian Bunk <bunk@stusta.de>

---

 drivers/net/Space.c    |   11 -----------
 drivers/net/loopback.c |    4 +++-
 2 files changed, 3 insertions(+), 12 deletions(-)

--- linux-2.6.19-mm1/drivers/net/loopback.c.old	2006-12-12 15:59:02.000000000 +0100
+++ linux-2.6.19-mm1/drivers/net/loopback.c	2006-12-12 16:02:11.000000000 +0100
@@ -229,9 +229,11 @@
 };
 
 /* Setup and register the loopback device. */
-int __init loopback_init(void)
+static int __init loopback_init(void)
 {
 	return register_netdev(&loopback_dev);
 };
 
+module_init(loopback_init);
+
 EXPORT_SYMBOL(loopback_dev);
--- linux-2.6.19-mm1/drivers/net/Space.c.old	2006-12-12 15:59:11.000000000 +0100
+++ linux-2.6.19-mm1/drivers/net/Space.c	2006-12-12 16:01:35.000000000 +0100
@@ -345,22 +345,11 @@
 #endif
 
 
-/*
- *	The loopback device is global so it can be directly referenced
- *	by the network code. Also, it must be first on device list.
- */
-extern int loopback_init(void);
-
 /*  Statically configured drivers -- order matters here. */
 static int __init net_olddevs_init(void)
 {
 	int num;
 
-	if (loopback_init()) {
-		printk(KERN_ERR "Network loopback device setup failed\n");
-	}
-
-
 #ifdef CONFIG_SBNI
 	for (num = 0; num < 8; ++num)
 		sbni_probe(num);

Re: [2.6 patch] drivers/net/loopback.c: convert to module_init()

[David Miller]

From: Adrian Bunk <bunk@stusta.de>
Date: Tue, 12 Dec 2006 17:24:35 +0100

> This patch converts drivers/net/loopback.c to using module_init().
> 
> Signed-off-by: Adrian Bunk <bunk@stusta.de>

I'm not %100 sure of this one, let's look at the comment you
are deleting:

> -/*
> - *	The loopback device is global so it can be directly referenced
> - *	by the network code. Also, it must be first on device list.
> - */
> -extern int loopback_init(void);
> -

in particular notice the part that says "it must be first on the
device list".

I'm not sure whether that is important any longer.  It probably isn't,
but we should verify it before applying such a patch.

Since module_init() effectively == device_initcall() for statically
built objects, which loopback always is, the Makefile ordering does
not seem to indicate to me that there is anything guarenteeing
this "first on the list" invariant.  At least not via object
file ordering.

So this gives some support to the idea that loopback_dev's position
on the device list no longer matters.

Re: [2.6 patch] drivers/net/loopback.c: convert to module_init()

[Adrian Bunk]

On Tue, Dec 12, 2006 at 05:17:56PM -0800, David Miller wrote:
> From: Adrian Bunk <bunk@stusta.de>
> Date: Tue, 12 Dec 2006 17:24:35 +0100
> 
> > This patch converts drivers/net/loopback.c to using module_init().
> > 
> > Signed-off-by: Adrian Bunk <bunk@stusta.de>
> 
> I'm not %100 sure of this one, let's look at the comment you
> are deleting:
> 
> > -/*
> > - *	The loopback device is global so it can be directly referenced
> > - *	by the network code. Also, it must be first on device list.
> > - */
> > -extern int loopback_init(void);
> > -
> 
> in particular notice the part that says "it must be first on the
> device list".
> 
> I'm not sure whether that is important any longer.  It probably isn't,
> but we should verify it before applying such a patch.
> 
> Since module_init() effectively == device_initcall() for statically
> built objects, which loopback always is, the Makefile ordering does
> not seem to indicate to me that there is anything guarenteeing
> this "first on the list" invariant.  At least not via object
> file ordering.
> 
> So this gives some support to the idea that loopback_dev's position
> on the device list no longer matters.

That far I already was.

Additionally:
- it works for me (but my e100 is always initialized before loop)
- I didn't spot any obvious interdependency with the other Space.c
  drivers

It could be I missed anything, but I don't see any better way to verify 
this than getting the patch into the next -mm and wait whether someone 
will scream...

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: [2.6 patch] drivers/net/loopback.c: convert to module_init()

[Thomas Graf]

* Adrian Bunk <bunk@stusta.de> 2006-12-13 14:40
> Additionally:
> - it works for me (but my e100 is always initialized before loop)
> - I didn't spot any obvious interdependency with the other Space.c
>   drivers
> 
> It could be I missed anything, but I don't see any better way to verify 
> this than getting the patch into the next -mm and wait whether someone 
> will scream...

The loopback was traditionally on ifindex 1, some userspace applications
even relied on it but that is no longer true. I think this is safe.

Re: [2.6 patch] drivers/net/loopback.c: convert to module_init()

[Stephen Hemminger]

On Tue, 12 Dec 2006 17:17:56 -0800 (PST)
David Miller <davem@davemloft.net> wrote:

> From: Adrian Bunk <bunk@stusta.de>
> Date: Tue, 12 Dec 2006 17:24:35 +0100
> 
> > This patch converts drivers/net/loopback.c to using module_init().
> > 
> > Signed-off-by: Adrian Bunk <bunk@stusta.de>
> 
> I'm not %100 sure of this one, let's look at the comment you
> are deleting:
> 
> > -/*
> > - *	The loopback device is global so it can be directly referenced
> > - *	by the network code. Also, it must be first on device list.
> > - */
> > -extern int loopback_init(void);
> > -
> 
> in particular notice the part that says "it must be first on the
> device list".
> 
> I'm not sure whether that is important any longer.  It probably isn't,
> but we should verify it before applying such a patch.
> 
> Since module_init() effectively == device_initcall() for statically
> built objects, which loopback always is, the Makefile ordering does
> not seem to indicate to me that there is anything guarenteeing
> this "first on the list" invariant.  At least not via object
> file ordering.
> 
> So this gives some support to the idea that loopback_dev's position
> on the device list no longer matters.

The dst code makes assumptions that loopback is ifindex 1 as well.
NAK on Adrian's change, it doesn't buy anything

Re: [2.6 patch] drivers/net/loopback.c: convert to module_init()

[Adrian Bunk]

On Wed, Dec 13, 2006 at 11:08:01AM -0800, Stephen Hemminger wrote:
> On Tue, 12 Dec 2006 17:17:56 -0800 (PST)
> David Miller <davem@davemloft.net> wrote:
> 
> > From: Adrian Bunk <bunk@stusta.de>
> > Date: Tue, 12 Dec 2006 17:24:35 +0100
> > 
> > > This patch converts drivers/net/loopback.c to using module_init().
> > > 
> > > Signed-off-by: Adrian Bunk <bunk@stusta.de>
> > 
> > I'm not %100 sure of this one, let's look at the comment you
> > are deleting:
> > 
> > > -/*
> > > - *	The loopback device is global so it can be directly referenced
> > > - *	by the network code. Also, it must be first on device list.
> > > - */
> > > -extern int loopback_init(void);
> > > -
> > 
> > in particular notice the part that says "it must be first on the
> > device list".
> > 
> > I'm not sure whether that is important any longer.  It probably isn't,
> > but we should verify it before applying such a patch.
> > 
> > Since module_init() effectively == device_initcall() for statically
> > built objects, which loopback always is, the Makefile ordering does
> > not seem to indicate to me that there is anything guarenteeing
> > this "first on the list" invariant.  At least not via object
> > file ordering.
> > 
> > So this gives some support to the idea that loopback_dev's position
> > on the device list no longer matters.
> 
> The dst code makes assumptions that loopback is ifindex 1 as well.
>...

But that assumption is already false for drivers not handled by Space.c:

E.g. on my computer [1], eth0 [2] has ifindex 1 and lo has ifindex 2.

cu
Adrian

[1] plain 2.6.16.36
[2] built-in e100 driver

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: [2.6 patch] drivers/net/loopback.c: convert to module_init()

[Al Viro]

On Tue, Dec 12, 2006 at 05:17:56PM -0800, David Miller wrote:
> From: Adrian Bunk <bunk@stusta.de>
> Date: Tue, 12 Dec 2006 17:24:35 +0100
> 
> > This patch converts drivers/net/loopback.c to using module_init().
> > 
> > Signed-off-by: Adrian Bunk <bunk@stusta.de>
> 
> I'm not %100 sure of this one, let's look at the comment you
> are deleting:
> 
> > -/*
> > - *	The loopback device is global so it can be directly referenced
> > - *	by the network code. Also, it must be first on device list.
> > - */
> > -extern int loopback_init(void);
> > -
> 
> in particular notice the part that says "it must be first on the
> device list".
> 
> I'm not sure whether that is important any longer.  It probably isn't,
> but we should verify it before applying such a patch.

There might be practical considerations along the lines of "we want
lookups for loopback to be fast"...

Re: [2.6 patch] drivers/net/loopback.c: convert to module_init()

[Thomas Graf]

* Al Viro <viro@ftp.linux.org.uk> 2006-12-13 20:12
> On Tue, Dec 12, 2006 at 05:17:56PM -0800, David Miller wrote:
> > I'm not sure whether that is important any longer.  It probably isn't,
> > but we should verify it before applying such a patch.
> 
> There might be practical considerations along the lines of "we want
> lookups for loopback to be fast"...

What is this discussion actually about? Since we started registering
devices directly hooked into the init process before device_initcall()
the order is random. Even the bonding device is registered before the
loopback.

Re: [2.6 patch] drivers/net/loopback.c: convert to module_init()

[Stephen Hemminger]

On Wed, 13 Dec 2006 21:49:33 +0100
Thomas Graf <tgraf@suug.ch> wrote:

> * Al Viro <viro@ftp.linux.org.uk> 2006-12-13 20:12
> > On Tue, Dec 12, 2006 at 05:17:56PM -0800, David Miller wrote:
> > > I'm not sure whether that is important any longer.  It probably isn't,
> > > but we should verify it before applying such a patch.
> > 
> > There might be practical considerations along the lines of "we want
> > lookups for loopback to be fast"...
> 
> What is this discussion actually about? Since we started registering
> devices directly hooked into the init process before device_initcall()
> the order is random. Even the bonding device is registered before the
> loopback.
> -
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Loopback should be there before protocols are started. It makes sense
to have a standard startup order.

Re: [2.6 patch] drivers/net/loopback.c: convert to module_init()

[Adrian Bunk]

On Wed, Dec 13, 2006 at 03:01:43PM -0800, Stephen Hemminger wrote:
> On Wed, 13 Dec 2006 21:49:33 +0100
> Thomas Graf <tgraf@suug.ch> wrote:
> 
> > * Al Viro <viro@ftp.linux.org.uk> 2006-12-13 20:12
> > > On Tue, Dec 12, 2006 at 05:17:56PM -0800, David Miller wrote:
> > > > I'm not sure whether that is important any longer.  It probably isn't,
> > > > but we should verify it before applying such a patch.
> > > 
> > > There might be practical considerations along the lines of "we want
> > > lookups for loopback to be fast"...
> > 
> > What is this discussion actually about? Since we started registering
> > devices directly hooked into the init process before device_initcall()
> > the order is random. Even the bonding device is registered before the
> > loopback.
> 
> Loopback should be there before protocols are started. It makes sense
> to have a standard startup order.

This actually becomes easier after my patch:

Now that it's untangled from net_olddevs_init(), you can simply change 
the module_init(loopback_init) to a different initcall level.

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: [2.6 patch] drivers/net/loopback.c: convert to module_init()

[Thomas Graf]

* Adrian Bunk <bunk@stusta.de> 2006-12-14 00:12
> On Wed, Dec 13, 2006 at 03:01:43PM -0800, Stephen Hemminger wrote:
> > Loopback should be there before protocols are started. It makes sense
> > to have a standard startup order.
> 
> This actually becomes easier after my patch:
> 
> Now that it's untangled from net_olddevs_init(), you can simply change 
> the module_init(loopback_init) to a different initcall level.

Not really, the device management inits as subsys, the ip layer hooks
into fs_initcall() which comes right after. The loopback was actually
registered after the protocol so far. I think Adrian's patch is fine
if the module_init() is changed to device_initcall().

Re: [2.6 patch] drivers/net/loopback.c: convert to module_init()

[Adrian Bunk]

On Thu, Dec 14, 2006 at 12:18:48AM +0100, Thomas Graf wrote:
> * Adrian Bunk <bunk@stusta.de> 2006-12-14 00:12
> > On Wed, Dec 13, 2006 at 03:01:43PM -0800, Stephen Hemminger wrote:
> > > Loopback should be there before protocols are started. It makes sense
> > > to have a standard startup order.
> > 
> > This actually becomes easier after my patch:
> > 
> > Now that it's untangled from net_olddevs_init(), you can simply change 
> > the module_init(loopback_init) to a different initcall level.
> 
> Not really, the device management inits as subsys, the ip layer hooks
> into fs_initcall() which comes right after. The loopback was actually
> registered after the protocol so far. I think Adrian's patch is fine
> if the module_init() is changed to device_initcall().

It doesn't matter (and I don't care) since for the non-modular case 
(that always applies for loopback), there's:
  #define __initcall(fn) device_initcall(fn)
  #define module_init(x)  __initcall(x);

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: [2.6 patch] drivers/net/loopback.c: convert to module_init()

[Stephen Hemminger]

On Thu, 14 Dec 2006 00:31:28 +0100
Adrian Bunk <bunk@stusta.de> wrote:

> On Thu, Dec 14, 2006 at 12:18:48AM +0100, Thomas Graf wrote:
> > * Adrian Bunk <bunk@stusta.de> 2006-12-14 00:12
> > > On Wed, Dec 13, 2006 at 03:01:43PM -0800, Stephen Hemminger wrote:
> > > > Loopback should be there before protocols are started. It makes sense
> > > > to have a standard startup order.
> > > 
> > > This actually becomes easier after my patch:
> > > 
> > > Now that it's untangled from net_olddevs_init(), you can simply change 
> > > the module_init(loopback_init) to a different initcall level.
> > 
> > Not really, the device management inits as subsys, the ip layer hooks
> > into fs_initcall() which comes right after. The loopback was actually
> > registered after the protocol so far. I think Adrian's patch is fine
> > if the module_init() is changed to device_initcall().
> 
> It doesn't matter (and I don't care) since for the non-modular case 
> (that always applies for loopback), there's:
>   #define __initcall(fn) device_initcall(fn)
>   #define module_init(x)  __initcall(x);
> 
> cu
> Adrian
> 

But what if other network device is not a module. We want loopback
to be first. so it needs to be before other device_initcall's

Re: [2.6 patch] drivers/net/loopback.c: convert to module_init()

[David Miller]

From: Stephen Hemminger <shemminger@osdl.org>
Date: Wed, 13 Dec 2006 15:41:25 -0800

> But what if other network device is not a module. We want loopback
> to be first. so it needs to be before other device_initcall's

There is zero evidence that loopback must be first.

I was not able to provide any after my study of the code last night,
and I haven't seen any concrete proof shown today either.

You mentioned "the dst code" relies on ifindex==1, what exactly do you
mean by that?  Give specific code examples instead of ambiguous and
non-specific references.

And this claim doesn't make any sense, because anyone who looks at
drivers/net/Makefile can see that several drivers are linked in way
before Space.o, and thus when built statically would today end up in
the device list way before the loopback driver.  So it can't be
"broken" by Adrian's change, since it's already not putting loopback
first and we can prove this! :-)

Re: [2.6 patch] drivers/net/loopback.c: convert to module_init()

[David Miller]

From: Stephen Hemminger <shemminger@osdl.org>
Date: Wed, 13 Dec 2006 15:41:25 -0800

> But what if other network device is not a module. We want loopback
> to be first. so it needs to be before other device_initcall's

That's not happening today, so it's "broken" today, which suggests
that the order in all likelyhood no longer matters.

Re: [2.6 patch] drivers/net/loopback.c: convert to module_init()

[David Miller]

From: Thomas Graf <tgraf@suug.ch>
Date: Thu, 14 Dec 2006 00:18:48 +0100

> Not really, the device management inits as subsys, the ip layer hooks
> into fs_initcall() which comes right after. The loopback was actually
> registered after the protocol so far. I think Adrian's patch is fine
> if the module_init() is changed to device_initcall().

module_init == device_initcall when the object is being compiled
statically into the kernel, which is always the case for the loopback
driver

I mentioned this in my original reply to Adrian, just in case anyone
happened to actually read that :-)

Re: [2.6 patch] drivers/net/loopback.c: convert to module_init()

[David Miller]

From: Al Viro <viro@ftp.linux.org.uk>
Date: Wed, 13 Dec 2006 20:12:14 +0000

> There might be practical considerations along the lines of "we want
> lookups for loopback to be fast"...

We have that taken care of, it's called "&loopback_dev".

None of the performance-caring paths do dev_get_by_name("lo")