* REGRESSION: serial_cs broken by 8250 changes
@ 2007-08-02 23:06 Daniel Ritz
2007-08-02 23:15 ` Andrew Morton
2007-08-02 23:24 ` Yinghai Lu
0 siblings, 2 replies; 7+ messages in thread
From: Daniel Ritz @ 2007-08-02 23:06 UTC (permalink / raw)
To: Andrew Morton, Linus Torvalds; +Cc: Yinghai Lu, Bjorn Helgaas, linux-kernel
commit 18a8bd949d6adb311ea816125ff65050df1f3f6e breaks serial_cs badly
with an oops, completely killing PCMCIA.
register_console() now calls console->early_setup(). which in case of
8250.c (the only user anyway) is serial8250_console_early_setup()
which is __init, calling 8250_early.c:serial8250_find_port_for_earlycon()
which is __init as well. boom.
the changelog mentions SERIAL_PORT_DFNS removal which happens to be
commit 7e92b4fc345f5b6f57585fbe5ffdb0f24d7c9b26. but this got reverted
by commit 57d4810ea0d9ca58a7bcc1336607f0cede0a2abf. so i'd suggest to
just revert the 8250 changes as well.
rgds
-daniel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: REGRESSION: serial_cs broken by 8250 changes
2007-08-02 23:06 REGRESSION: serial_cs broken by 8250 changes Daniel Ritz
@ 2007-08-02 23:15 ` Andrew Morton
2007-08-02 23:24 ` Yinghai Lu
1 sibling, 0 replies; 7+ messages in thread
From: Andrew Morton @ 2007-08-02 23:15 UTC (permalink / raw)
To: Daniel Ritz; +Cc: Linus Torvalds, Yinghai Lu, Bjorn Helgaas, linux-kernel
On Fri, 3 Aug 2007 01:06:32 +0200
Daniel Ritz <daniel.ritz-ml@swissonline.ch> wrote:
> commit 18a8bd949d6adb311ea816125ff65050df1f3f6e breaks serial_cs badly
> with an oops, completely killing PCMCIA.
>
> register_console() now calls console->early_setup(). which in case of
> 8250.c (the only user anyway) is serial8250_console_early_setup()
> which is __init, calling 8250_early.c:serial8250_find_port_for_earlycon()
> which is __init as well. boom.
>
> the changelog mentions SERIAL_PORT_DFNS removal which happens to be
> commit 7e92b4fc345f5b6f57585fbe5ffdb0f24d7c9b26. but this got reverted
> by commit 57d4810ea0d9ca58a7bcc1336607f0cede0a2abf. so i'd suggest to
> just revert the 8250 changes as well.
>
<head spins>
Sounds like you'd be the ideal person to propose a patch ;)
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: REGRESSION: serial_cs broken by 8250 changes
2007-08-02 23:06 REGRESSION: serial_cs broken by 8250 changes Daniel Ritz
2007-08-02 23:15 ` Andrew Morton
@ 2007-08-02 23:24 ` Yinghai Lu
2007-08-02 23:35 ` Andrew Morton
1 sibling, 1 reply; 7+ messages in thread
From: Yinghai Lu @ 2007-08-02 23:24 UTC (permalink / raw)
To: Daniel Ritz, Andrew Morton, Linus Torvalds, Bjorn Helgaas; +Cc: linux-kernel
Daniel Ritz wrote:
> commit 18a8bd949d6adb311ea816125ff65050df1f3f6e breaks serial_cs badly
> with an oops, completely killing PCMCIA.
>
> register_console() now calls console->early_setup(). which in case of
> 8250.c (the only user anyway) is serial8250_console_early_setup()
> which is __init, calling 8250_early.c:serial8250_find_port_for_earlycon()
> which is __init as well. boom.
>
> the changelog mentions SERIAL_PORT_DFNS removal which happens to be
> commit 7e92b4fc345f5b6f57585fbe5ffdb0f24d7c9b26. but this got reverted
> by commit 57d4810ea0d9ca58a7bcc1336607f0cede0a2abf. so i'd suggest to
> just revert the 8250 changes as well.
>
> rgds
> -daniel
Is there any flag or sign that init code has been released?
We could use that to prevent init code to be called after code is freed.
Thanks
Yinghai Lu
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: REGRESSION: serial_cs broken by 8250 changes
2007-08-02 23:24 ` Yinghai Lu
@ 2007-08-02 23:35 ` Andrew Morton
2007-08-03 0:18 ` Yinghai Lu
2007-08-03 16:54 ` Maciej W. Rozycki
0 siblings, 2 replies; 7+ messages in thread
From: Andrew Morton @ 2007-08-02 23:35 UTC (permalink / raw)
To: Yinghai.Lu; +Cc: Daniel Ritz, Linus Torvalds, Bjorn Helgaas, linux-kernel
On Thu, 02 Aug 2007 16:24:42 -0700
Yinghai Lu <Yinghai.Lu@Sun.COM> wrote:
> Daniel Ritz wrote:
> > commit 18a8bd949d6adb311ea816125ff65050df1f3f6e breaks serial_cs badly
> > with an oops, completely killing PCMCIA.
> >
> > register_console() now calls console->early_setup(). which in case of
> > 8250.c (the only user anyway) is serial8250_console_early_setup()
> > which is __init, calling 8250_early.c:serial8250_find_port_for_earlycon()
> > which is __init as well. boom.
> >
> > the changelog mentions SERIAL_PORT_DFNS removal which happens to be
> > commit 7e92b4fc345f5b6f57585fbe5ffdb0f24d7c9b26. but this got reverted
> > by commit 57d4810ea0d9ca58a7bcc1336607f0cede0a2abf. so i'd suggest to
> > just revert the 8250 changes as well.
> >
> > rgds
> > -daniel
>
> Is there any flag or sign that init code has been released?
Nope.
> We could use that to prevent init code to be called after code is freed.
If we can omit a function call without breaking anything then we shouldn't
have been calling that function at all ;)
It sounds like making serial8250_console_early_setup() and
serial8250_find_port_for_earlycon() non-__init will fix this.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: REGRESSION: serial_cs broken by 8250 changes
2007-08-02 23:35 ` Andrew Morton
@ 2007-08-03 0:18 ` Yinghai Lu
2007-08-03 14:07 ` Daniel Ritz
2007-08-03 16:54 ` Maciej W. Rozycki
1 sibling, 1 reply; 7+ messages in thread
From: Yinghai Lu @ 2007-08-03 0:18 UTC (permalink / raw)
To: Andrew Morton; +Cc: Daniel Ritz, Linus Torvalds, Bjorn Helgaas, linux-kernel
Andrew Morton wrote:
> On Thu, 02 Aug 2007 16:24:42 -0700
> Yinghai Lu <Yinghai.Lu@Sun.COM> wrote:
>
>> Daniel Ritz wrote:
>>> commit 18a8bd949d6adb311ea816125ff65050df1f3f6e breaks serial_cs badly
>>> with an oops, completely killing PCMCIA.
>>>
>>> register_console() now calls console->early_setup(). which in case of
>>> 8250.c (the only user anyway) is serial8250_console_early_setup()
>>> which is __init, calling 8250_early.c:serial8250_find_port_for_earlycon()
>>> which is __init as well. boom.
>>>
>>> the changelog mentions SERIAL_PORT_DFNS removal which happens to be
>>> commit 7e92b4fc345f5b6f57585fbe5ffdb0f24d7c9b26. but this got reverted
>>> by commit 57d4810ea0d9ca58a7bcc1336607f0cede0a2abf. so i'd suggest to
>>> just revert the 8250 changes as well.
>>>
>>> rgds
>>> -daniel
>> Is there any flag or sign that init code has been released?
>
> Nope.
>
>> We could use that to prevent init code to be called after code is freed.
>
> If we can omit a function call without breaking anything then we shouldn't
> have been calling that function at all ;)
>
> It sounds like making serial8250_console_early_setup() and
> serial8250_find_port_for_earlycon() non-__init will fix this.
>
yes, together update_console_cmdline in kernel/printk.c
Daniel,
can you test that in your setup?
Thanks
Yinghai Lu
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: REGRESSION: serial_cs broken by 8250 changes
2007-08-03 0:18 ` Yinghai Lu
@ 2007-08-03 14:07 ` Daniel Ritz
0 siblings, 0 replies; 7+ messages in thread
From: Daniel Ritz @ 2007-08-03 14:07 UTC (permalink / raw)
To: Yinghai.Lu; +Cc: Andrew Morton, Linus Torvalds, Bjorn Helgaas, linux-kernel
On Friday 03 August 2007 02:18:10 Yinghai Lu wrote:
> Andrew Morton wrote:
> > On Thu, 02 Aug 2007 16:24:42 -0700
> > Yinghai Lu <Yinghai.Lu@Sun.COM> wrote:
> >
> >> Daniel Ritz wrote:
> >>> commit 18a8bd949d6adb311ea816125ff65050df1f3f6e breaks serial_cs badly
> >>> with an oops, completely killing PCMCIA.
> >>>
> >>> register_console() now calls console->early_setup(). which in case of
> >>> 8250.c (the only user anyway) is serial8250_console_early_setup()
> >>> which is __init, calling 8250_early.c:serial8250_find_port_for_earlycon()
> >>> which is __init as well. boom.
> >>>
> >>> the changelog mentions SERIAL_PORT_DFNS removal which happens to be
> >>> commit 7e92b4fc345f5b6f57585fbe5ffdb0f24d7c9b26. but this got reverted
> >>> by commit 57d4810ea0d9ca58a7bcc1336607f0cede0a2abf. so i'd suggest to
> >>> just revert the 8250 changes as well.
> >>>
> >>> rgds
> >>> -daniel
> >> Is there any flag or sign that init code has been released?
> >
> > Nope.
> >
> >> We could use that to prevent init code to be called after code is freed.
> >
> > If we can omit a function call without breaking anything then we shouldn't
> > have been calling that function at all ;)
> >
> > It sounds like making serial8250_console_early_setup() and
> > serial8250_find_port_for_earlycon() non-__init will fix this.
> >
>
> yes, together update_console_cmdline in kernel/printk.c
>
> Daniel,
> can you test that in your setup?
>
yep, seems to work. patch attached.
rgds
-daniel
----------
[PATCH] serial: fix 8250 early console setup
the early setup function serial8250_console_early_setup() can be called
from non __init code (eg. hotpluggable serial ports like serial_cs) so
remove the __init from the call chain to avoid crashes.
Signed-off-by: Daniel Ritz <daniel.ritz@gmx.ch>
Cc: Yinghai Lu <yinghai.lu@sun.com>
diff --git a/drivers/serial/8250.c b/drivers/serial/8250.c
index 2f5a5ac..3013130 100644
--- a/drivers/serial/8250.c
+++ b/drivers/serial/8250.c
@@ -2514,7 +2514,7 @@ static int __init serial8250_console_setup(struct console *co, char *options)
return uart_set_options(port, co, baud, parity, bits, flow);
}
-static int __init serial8250_console_early_setup(void)
+static int serial8250_console_early_setup(void)
{
return serial8250_find_port_for_earlycon();
}
diff --git a/drivers/serial/8250_early.c b/drivers/serial/8250_early.c
index 150cad5..4d4c9f0 100644
--- a/drivers/serial/8250_early.c
+++ b/drivers/serial/8250_early.c
@@ -227,7 +227,7 @@ int __init setup_early_serial8250_console(char *cmdline)
return 0;
}
-int __init serial8250_find_port_for_earlycon(void)
+int serial8250_find_port_for_earlycon(void)
{
struct early_serial8250_device *device = &early_device;
struct uart_port *port = &device->port;
diff --git a/kernel/printk.c b/kernel/printk.c
index 051d27e..bd2cd06 100644
--- a/kernel/printk.c
+++ b/kernel/printk.c
@@ -732,7 +732,7 @@ int __init add_preferred_console(char *name, int idx, char *options)
return 0;
}
-int __init update_console_cmdline(char *name, int idx, char *name_new, int idx_new, char *options)
+int update_console_cmdline(char *name, int idx, char *name_new, int idx_new, char *options)
{
struct console_cmdline *c;
int i;
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: REGRESSION: serial_cs broken by 8250 changes
2007-08-02 23:35 ` Andrew Morton
2007-08-03 0:18 ` Yinghai Lu
@ 2007-08-03 16:54 ` Maciej W. Rozycki
1 sibling, 0 replies; 7+ messages in thread
From: Maciej W. Rozycki @ 2007-08-03 16:54 UTC (permalink / raw)
To: Andrew Morton
Cc: Yinghai.Lu, Daniel Ritz, Linus Torvalds, Bjorn Helgaas, linux-kernel
On Thu, 2 Aug 2007, Andrew Morton wrote:
> > We could use that to prevent init code to be called after code is freed.
>
> If we can omit a function call without breaking anything then we shouldn't
> have been calling that function at all ;)
Well, we can always put a couple of otherwise unneeded calls here and
there to make the compiler happy to enjoy its freedom to produce code and
exercise the calling convention rules set by the ABI before the actual
use. ;-)
Maciej
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2007-08-03 16:55 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-08-02 23:06 REGRESSION: serial_cs broken by 8250 changes Daniel Ritz
2007-08-02 23:15 ` Andrew Morton
2007-08-02 23:24 ` Yinghai Lu
2007-08-02 23:35 ` Andrew Morton
2007-08-03 0:18 ` Yinghai Lu
2007-08-03 14:07 ` Daniel Ritz
2007-08-03 16:54 ` Maciej W. Rozycki
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.