2007-10-11 10:53:08

by Rob Landley

[permalink] [raw]
Subject: Scsi on sparc build break in 2.6.23.

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.


2007-10-11 11:05:39

by Adrian Bunk

[permalink] [raw]
Subject: Re: Scsi on sparc build break in 2.6.23.

On Thu, Oct 11, 2007 at 05:52:48AM -0500, Rob Landley wrote:
> 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?

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
send your .config.

> Rob

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

2007-10-11 13:17:40

by Rob Landley

[permalink] [raw]
Subject: Re: Scsi on sparc build break in 2.6.23.

On Thursday 11 October 2007 6:05:55 am Adrian Bunk wrote:
> On Thu, Oct 11, 2007 at 05:52:48AM -0500, Rob Landley wrote:
> > 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?
>
> 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
> send your .config.

*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.


Attachments:
(No filename) (1.40 kB)
miniconfig-linux (1.97 kB)
Download all attachments

2007-10-11 13:31:29

by James Bottomley

[permalink] [raw]
Subject: Re: Scsi on sparc build break in 2.6.23.

On Thu, 2007-10-11 at 08:17 -0500, Rob Landley wrote:
> On Thursday 11 October 2007 6:05:55 am Adrian Bunk wrote:
> > On Thu, Oct 11, 2007 at 05:52:48AM -0500, Rob Landley wrote:
> > > 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?
> >
> > 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
> > send your .config.
>
> *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).

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


2007-10-11 15:21:34

by Adrian Bunk

[permalink] [raw]
Subject: Re: Scsi on sparc build break in 2.6.23.

On Thu, Oct 11, 2007 at 08:17:22AM -0500, Rob Landley wrote:
> On Thursday 11 October 2007 6:05:55 am Adrian Bunk wrote:
> > On Thu, Oct 11, 2007 at 05:52:48AM -0500, Rob Landley wrote:
> > > 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?
> >
> > 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
> > send your .config.
>
> *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
>...

I assume you have the full .config in your build directory, and could
have taken it from there?

> Rob

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

2007-10-11 15:35:08

by Adrian Bunk

[permalink] [raw]
Subject: [2.6 patch] esp_scsi.c: remove __dev{init,exit}

On Thu, Oct 11, 2007 at 08:30:38AM -0500, James Bottomley wrote:
> On Thu, 2007-10-11 at 08:17 -0500, Rob Landley wrote:
> > On Thursday 11 October 2007 6:05:55 am Adrian Bunk wrote:
> > > On Thu, Oct 11, 2007 at 05:52:48AM -0500, Rob Landley wrote:
> > > > 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?
> > >
> > > 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
> > > send your .config.
> >
> > *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).
>
> 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.

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.

Patch below.

> James

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 <[email protected]>

---

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);
}

2007-10-11 21:47:29

by Rob Landley

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

On Thursday 11 October 2007 10:35:20 am Adrian Bunk wrote:
> 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 <[email protected]>

Acked-by: Rob Landley <[email protected]>

Worked for me...

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

2007-10-11 21:49:51

by David Miller

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

From: Rob Landley <[email protected]>
Date: Thu, 11 Oct 2007 17:47:02 -0500

> On Thursday 11 October 2007 10:35:20 am Adrian Bunk wrote:
> > 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 <[email protected]>
>
> Acked-by: Rob Landley <[email protected]>

I'm fine with this fix too.

Acked-by: David S. Miller <[email protected]>

2007-10-11 23:56:41

by Adrian Bunk

[permalink] [raw]
Subject: Re: Scsi on sparc build break in 2.6.23.

On Thu, Oct 11, 2007 at 05:37:30PM -0500, Rob Landley wrote:
> On Thursday 11 October 2007 10:21:49 am Adrian Bunk wrote:
> > I assume you have the full .config in your build directory, and could
> > have taken it from there?
>
> Actually I don't. (The extracted source tree is in a temporary directory
> which the script deletes afterwards unless I tell it not to. I just followed
> my own instructions to create the .config and attach it, but I see that James
> Bottomley already diagnosed it and you came up with a patch. Off to try
> it...)

I was able to follow your instructions for generating it.

But I hope you got my point that it's much easier to debug such issues
when you generate the .config yourself and attach it when sending the
bug report.

> Rob

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

2007-10-12 00:12:46

by Randy Dunlap

[permalink] [raw]
Subject: Re: Scsi on sparc build break in 2.6.23.

On Fri, 12 Oct 2007 01:56:59 +0200 Adrian Bunk wrote:

> On Thu, Oct 11, 2007 at 05:37:30PM -0500, Rob Landley wrote:
> > On Thursday 11 October 2007 10:21:49 am Adrian Bunk wrote:
> > > I assume you have the full .config in your build directory, and could
> > > have taken it from there?
> >
> > Actually I don't. (The extracted source tree is in a temporary directory
> > which the script deletes afterwards unless I tell it not to. I just followed
> > my own instructions to create the .config and attach it, but I see that James
> > Bottomley already diagnosed it and you came up with a patch. Off to try
> > it...)
>
> I was able to follow your instructions for generating it.
>
> But I hope you got my point that it's much easier to debug such issues
> when you generate the .config yourself and attach it when sending the
> bug report.

I use the mini-config method quite a bit (probably not as much
as Rob does), but I agree with Adrian, it should have just been
included here in full (not mini).

---
~Randy

2007-10-13 19:10:29

by Rob Landley

[permalink] [raw]
Subject: Re: Scsi on sparc build break in 2.6.23.

On Thursday 11 October 2007 6:56:59 pm Adrian Bunk wrote:
> On Thu, Oct 11, 2007 at 05:37:30PM -0500, Rob Landley wrote:
> > On Thursday 11 October 2007 10:21:49 am Adrian Bunk wrote:
> > > I assume you have the full .config in your build directory, and could
> > > have taken it from there?
> >
> > Actually I don't. (The extracted source tree is in a temporary directory
> > which the script deletes afterwards unless I tell it not to. I just
> > followed my own instructions to create the .config and attach it, but I
> > see that James Bottomley already diagnosed it and you came up with a
> > patch. Off to try it...)
>
> I was able to follow your instructions for generating it.
>
> But I hope you got my point that it's much easier to debug such issues
> when you generate the .config yourself and attach it when sending the
> bug report.

*shrug* I find miniconfig much easier to read because I can see what the 50
or so intentional options are, not just the 1000 or so background options set
to various default values.

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

2007-10-13 19:26:21

by Adrian Bunk

[permalink] [raw]
Subject: Re: Scsi on sparc build break in 2.6.23.

On Sat, Oct 13, 2007 at 02:09:35PM -0500, Rob Landley wrote:
> On Thursday 11 October 2007 6:56:59 pm Adrian Bunk wrote:
> > On Thu, Oct 11, 2007 at 05:37:30PM -0500, Rob Landley wrote:
> > > On Thursday 11 October 2007 10:21:49 am Adrian Bunk wrote:
> > > > I assume you have the full .config in your build directory, and could
> > > > have taken it from there?
> > >
> > > Actually I don't. (The extracted source tree is in a temporary directory
> > > which the script deletes afterwards unless I tell it not to. I just
> > > followed my own instructions to create the .config and attach it, but I
> > > see that James Bottomley already diagnosed it and you came up with a
> > > patch. Off to try it...)
> >
> > I was able to follow your instructions for generating it.
> >
> > But I hope you got my point that it's much easier to debug such issues
> > when you generate the .config yourself and attach it when sending the
> > bug report.
>
> *shrug* I find miniconfig much easier to read because I can see what the 50
> or so intentional options are, not just the 1000 or so background options set
> to various default values.

The .config is the canonical format everyone is used to.

It also has advantages like a defined order of the options.

And it is actually an advantage that you see what the other options are
set to (e.g. in this case your miniconfig did not show the value of
CONFIG_HOTPLUG which was the most important option for understanding the
bug) - it doesn't matter whether you {dis,en}abled it intentionally,
all that matter is it's value.

There might be use cases where a miniconfig is better, but for debugging
compile errors a full .config is much better.

> Rob

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

2007-10-18 08:18:53

by Rob Landley

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

On Thursday 11 October 2007 4:49:28 pm David Miller wrote:
> From: Rob Landley <[email protected]>
> Date: Thu, 11 Oct 2007 17:47:02 -0500
>
> > On Thursday 11 October 2007 10:35:20 am Adrian Bunk wrote:
> > > 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 <[email protected]>
> >
> > Acked-by: Rob Landley <[email protected]>
>
> I'm fine with this fix too.
>
> Acked-by: David S. Miller <[email protected]>

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.