* 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 a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox