Re: [2.6 patch] esp_scsi.c: remove __dev{init,exit}

Previous thread: Regression from 2.6.21 to 2.6.22 and above by Stefan Scheler on Thursday, October 11, 2007 - 3:41 am. (3 messages)

Next thread: Re: [PATCH] FAT: Fix printk format strings. by Bodo Eggert on Thursday, October 11, 2007 - 3:58 am. (2 messages)
From: Rob Landley
Date: Thursday, October 11, 2007 - 3:52 am

CONFIG_SCSI_SUNESP=y breaks the build in 2.6.23:

  LD      vmlinux
`scsi_esp_unregister' referenced in section `__ksymtab' of drivers/built-in.o: 
defined in discarded section `.exit.text' of drivers/built-in.o
make: *** [vmlinux] Error 1

Do you need my full .config to reproduce this?

Rob
-- 
"One of my most productive days was throwing away 1000 lines of code."
  - Ken Thompson.
-

From: Adrian Bunk
Date: Thursday, October 11, 2007 - 4:05 am

Please always attach the .config when reporting errors.
The few bytes don't matter and it often saves some time.

I have an idea regarding what might be going wrong in this case,
but it would cost me additional time to look at it because you didn't 

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

-

From: Rob Landley
Date: Thursday, October 11, 2007 - 6:17 am

*shrug*  That's why I asked.

The reason I hesitated is I use miniconfig files rather than big .config 
files, and some people get confused by that.  Drop the attached 
miniconfig-linux in the kernel source directory and go:
  make ARCH=sparc allnoconfig KCONFIG_ALLCONFIG=miniconfig-linux

That expands it to a big .config file, and from there "make ARCH=sparc 
CROSS_COMPILE=sparc-" to reproduce the problem.  Assuming you have a sparc 
cross-compiler lying around.

Disable CONFIG_SCSI_SUNESP and it builds to the end, (and the result boots but 
won't mount the root filesystem, which is sort of expected).

Rob
-- 
"One of my most productive days was throwing away 1000 lines of code."
  - Ken Thompson.
From: James Bottomley
Date: Thursday, October 11, 2007 - 6:30 am

This is a very subtle error.  You're building without hotplug, which
causes __devexit to become __exit, so scsi_esp_unregister is placed in
the discard section of vmlinux.  Unfortunately, the EXPORT_SYMBOL causes
it to be referenced from the symbol export table.

The fix is probably just to remove the __devexit tag from the function
rather than trying to work out how to make the export symbol conditional
on the symbol not being discarded.

James


-

From: Adrian Bunk
Date: Thursday, October 11, 2007 - 8:35 am

You can't make it conditional on that since a built-in esp_scsi could 
have modular users.

scsi_esp_register() is also buggy since it's impossible that the 
EXPORT_SYMBOL(scsi_esp_register) works in the CONFIG_HOTPLUG=n case when 
it's __devinit - it will always Oops.

Having anything exported __{,dev}{init,exit} is always very likely to be 
buggy.


cu
Adrian


<--  snip  -->


Since scsi_esp_{,un}register() are EXPORT_SYMBOL'ed, these functions
(and the functions they use) can't be __dev{init,exit}.

Based on a bug report by Rob Landley.

Signed-off-by: Adrian Bunk <bunk@kernel.org>

---

 drivers/scsi/esp_scsi.c |   10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

--- linux-2.6.23/drivers/scsi/esp_scsi.c.old	2007-10-11 17:29:50.000000000 +0200
+++ linux-2.6.23/drivers/scsi/esp_scsi.c	2007-10-11 17:31:25.000000000 +0200
@@ -2138,7 +2138,7 @@
 }
 EXPORT_SYMBOL(scsi_esp_intr);
 
-static void __devinit esp_get_revision(struct esp *esp)
+static void esp_get_revision(struct esp *esp)
 {
 	u8 val;
 
@@ -2187,7 +2187,7 @@
 	}
 }
 
-static void __devinit esp_init_swstate(struct esp *esp)
+static void esp_init_swstate(struct esp *esp)
 {
 	int i;
 
@@ -2233,7 +2233,7 @@
 	esp_read8(ESP_INTRPT);
 }
 
-static void __devinit esp_set_clock_params(struct esp *esp)
+static void esp_set_clock_params(struct esp *esp)
 {
 	int fmhz;
 	u8 ccf;
@@ -2306,7 +2306,7 @@
 
 static struct scsi_transport_template *esp_transport_template;
 
-int __devinit scsi_esp_register(struct esp *esp, struct device *dev)
+int scsi_esp_register(struct esp *esp, struct device *dev)
 {
 	static int instance;
 	int err;
@@ -2346,7 +2346,7 @@
 }
 EXPORT_SYMBOL(scsi_esp_register);
 
-void __devexit scsi_esp_unregister(struct esp *esp)
+void scsi_esp_unregister(struct esp *esp)
 {
 	scsi_remove_host(esp->host);
 }

-

From: Rob Landley
Date: Thursday, October 11, 2007 - 3:47 pm

Acked-by: Rob Landley <rob@landley.net>

Worked for me...

Rob
-- 
"One of my most productive days was throwing away 1000 lines of code."
  - Ken Thompson.
-

From: David Miller
Date: Thursday, October 11, 2007 - 2:49 pm

From: Rob Landley <rob@landley.net>

I'm fine with this fix too.

Acked-by: David S. Miller <davem@davemloft.net>
-

From: Rob Landley
Date: Thursday, October 18, 2007 - 1:18 am

Did this already go to the -stable guys for 2.6.23.2?

Rob
-- 
"One of my most productive days was throwing away 1000 lines of code."
  - Ken Thompson.
-

From: Adrian Bunk
Date: Thursday, October 11, 2007 - 8:21 am

I assume you have the full .config in your build directory, and could 

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

-

Previous thread: Regression from 2.6.21 to 2.6.22 and above by Stefan Scheler on Thursday, October 11, 2007 - 3:41 am. (3 messages)

Next thread: Re: [PATCH] FAT: Fix printk format strings. by Bodo Eggert on Thursday, October 11, 2007 - 3:58 am. (2 messages)