| Message ID | 20180430145558.4308-5-npiggin@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | e00da0f2db91b90e990cc05088f03adbc58af895 |
| Headers | show |
| Series | hvc and powerpc opal console latency reduction | expand |
Nicholas Piggin <npiggin@gmail.com> writes: > Use the more refined and tested event polling loop from opal_put_chars > as the fallback console flush in the opal-kmsg path. This loop is used > by the console driver today, whereas the opal-kmsg fallback is not > likely to have been used for years. > > Use WARN_ONCE rather than a printk when the fallback is invoked to > prepare for moving the console flush into a common function. Do we want to add a WARN in that path? If we're panicking things might get worse if we WARN (which takes a trap). cheers
On Fri, 04 May 2018 15:16:37 +1000 Michael Ellerman <mpe@ellerman.id.au> wrote: > Nicholas Piggin <npiggin@gmail.com> writes: > > > Use the more refined and tested event polling loop from opal_put_chars > > as the fallback console flush in the opal-kmsg path. This loop is used > > by the console driver today, whereas the opal-kmsg fallback is not > > likely to have been used for years. > > > > Use WARN_ONCE rather than a printk when the fallback is invoked to > > prepare for moving the console flush into a common function. > > Do we want to add a WARN in that path? If we're panicking things might > get worse if we WARN (which takes a trap). True, probably a good idea not to... oh there's a printk_once so that'll work nicely. Thanks, Nick
Nicholas Piggin <npiggin@gmail.com> writes: > On Fri, 04 May 2018 15:16:37 +1000 > Michael Ellerman <mpe@ellerman.id.au> wrote: >> Nicholas Piggin <npiggin@gmail.com> writes: >> > Use the more refined and tested event polling loop from opal_put_chars >> > as the fallback console flush in the opal-kmsg path. This loop is used >> > by the console driver today, whereas the opal-kmsg fallback is not >> > likely to have been used for years. >> > >> > Use WARN_ONCE rather than a printk when the fallback is invoked to >> > prepare for moving the console flush into a common function. >> >> Do we want to add a WARN in that path? If we're panicking things might >> get worse if we WARN (which takes a trap). > > True, probably a good idea not to... oh there's a printk_once so > that'll work nicely. Cool. I have this series in a tree so you can send me an incremental diff if it's reasonably small. cheers
On Mon, 07 May 2018 20:36:39 +1000 Michael Ellerman <mpe@ellerman.id.au> wrote: > Nicholas Piggin <npiggin@gmail.com> writes: > > On Fri, 04 May 2018 15:16:37 +1000 > > Michael Ellerman <mpe@ellerman.id.au> wrote: > >> Nicholas Piggin <npiggin@gmail.com> writes: > >> > Use the more refined and tested event polling loop from opal_put_chars > >> > as the fallback console flush in the opal-kmsg path. This loop is used > >> > by the console driver today, whereas the opal-kmsg fallback is not > >> > likely to have been used for years. > >> > > >> > Use WARN_ONCE rather than a printk when the fallback is invoked to > >> > prepare for moving the console flush into a common function. > >> > >> Do we want to add a WARN in that path? If we're panicking things might > >> get worse if we WARN (which takes a trap). > > > > True, probably a good idea not to... oh there's a printk_once so > > that'll work nicely. > > Cool. > > I have this series in a tree so you can send me an incremental diff if > it's reasonably small. It's a one liner (also moved location of message back to where it was originally). The next patch will clash because it moves this over into opal.c, so you'd have to fix that by hand. Thanks, Nick --- arch/powerpc/platforms/powernv/opal-kmsg.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/powerpc/platforms/powernv/opal-kmsg.c b/arch/powerpc/platforms/powernv/opal-kmsg.c index fd2bbf4fd6dc..c610ef3541aa 100644 --- a/arch/powerpc/platforms/powernv/opal-kmsg.c +++ b/arch/powerpc/platforms/powernv/opal-kmsg.c @@ -53,12 +53,12 @@ static void force_opal_console_flush(struct kmsg_dumper *dumper, } else { __be64 evt; - WARN_ONCE(1, "opal: OPAL_CONSOLE_FLUSH missing.\n"); /* * If OPAL_CONSOLE_FLUSH is not implemented in the firmware, * the console can still be flushed by calling the polling * function while it has OPAL_EVENT_CONSOLE_OUTPUT events. */ + printk_once(KERN_NOTICE "opal: OPAL_CONSOLE_FLUSH missing.\n"); do { opal_poll_events(&evt); } while (be64_to_cpu(evt) & OPAL_EVENT_CONSOLE_OUTPUT);
diff --git a/arch/powerpc/platforms/powernv/opal-kmsg.c b/arch/powerpc/platforms/powernv/opal-kmsg.c index f8f41ccce75f..fd2bbf4fd6dc 100644 --- a/arch/powerpc/platforms/powernv/opal-kmsg.c +++ b/arch/powerpc/platforms/powernv/opal-kmsg.c @@ -51,20 +51,17 @@ static void force_opal_console_flush(struct kmsg_dumper *dumper, } while (rc == OPAL_PARTIAL); /* More to flush */ } else { - int i; + __be64 evt; + WARN_ONCE(1, "opal: OPAL_CONSOLE_FLUSH missing.\n"); /* * If OPAL_CONSOLE_FLUSH is not implemented in the firmware, * the console can still be flushed by calling the polling - * function enough times to flush the buffer. We don't know - * how much output still needs to be flushed, but we can be - * generous since the kernel is in panic and doesn't need - * to do much else. + * function while it has OPAL_EVENT_CONSOLE_OUTPUT events. */ - printk(KERN_NOTICE "opal: OPAL_CONSOLE_FLUSH missing.\n"); - for (i = 0; i < 1024; i++) { - opal_poll_events(NULL); - } + do { + opal_poll_events(&evt); + } while (be64_to_cpu(evt) & OPAL_EVENT_CONSOLE_OUTPUT); } }