Hi,
slowly looking through the patches.
On (12/22/15 14:47), Jan Kara wrote:
[..]
> @@ -1803,10 +1869,24 @@ asmlinkage int vprintk_emit(int facility, int level,
> logbuf_cpu = UINT_MAX;
> raw_spin_unlock(&logbuf_lock);
> lockdep_on();
> + /*
> + * By default we print message to console asynchronously so that kernel
> + * doesn't get stalled due to slow serial console. That can lead to
> + * softlockups, lost interrupts, or userspace timing out under heavy
> + * printing load.
> + *
> + * However we resort to synchronous printing of messages during early
> + * boot, when oops is in progress, or when synchronous printing was
> + * explicitely requested by kernel parameter.
> + */
> + if (keventd_up() && !oops_in_progress && !sync_print) {
> + __this_cpu_or(printk_pending, PRINTK_PENDING_OUTPUT);
> + irq_work_queue(this_cpu_ptr(&wake_up_klogd_work));
> + } else
> + sync_print = true;
> local_irq_restore(flags);
can we replace this oops_in_progress check with something more reliable?
CPU0 CPU1 - CPUN
panic()
local_irq_disable() executing foo() with irqs disabled,
console_verbose() or processing an extremely long irq handler.
bust_spinlocks()
oops_in_progress++
smp_send_stop()
bust_spinlocks()
oops_in_progress-- ok, IPI arrives
dump_stack()/printk()/etc from IPI_CPU_STOP
"while (1) cpu_relax()" with irq/fiq
disabled/halt/etc.
smp_send_stop() wrapped in `oops_in_progress++/oops_in_progress--' is arch
specific,
and some platforms don't do any IPI-delivered (e.g. via num_online_cpus())
checks at
all. Some do. For example, arm/arm64:
void smp_send_stop(void)
...
/* Wait up to one second for other CPUs to stop */
timeout = USEC_PER_SEC;
while (num_online_cpus() > 1 && timeout--)
udelay(1);
if (num_online_cpus() > 1)
pr_warn("SMP: failed to stop secondary CPUs\n");
...
so there are non-zero chances that IPI will arrive to CPU after
'oops_in_progress--',
and thus dump_stack()/etc. happening on that/those cpu/cpus will be lost.
bust_spinlocks(0) does
...
if (--oops_in_progress == 0)
wake_up_klogd();
...
but local cpu has irqs disabled and `panic_timeout' can be zero.
How about setting 'sync_print' to 'true' in...
bust_spinlocks() /* only set to true */
or
console_verbose() /* um... may be... */
or
having a separate one-liner for that
void console_panic_mode(void)
{
sync_print = true;
}
and call it early in panic(), before we send out IPI_STOP.
-ss
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
