Re: [RFC][PATCHv4 0/7] printk: introduce printing kernel threads

[Sergey Senozhatsky]

Hello,

On (06/02/17 18:03), Sergey Senozhatsky wrote:
[..]
> 	I've managed to reproduce some of the issues with a single printk
> kthread solution that Jan Kara talked about. Sometimes scheduler decides
> sometimes scheduler decides that printk kthread should run on the same CPU
> as the process that is doing printing, so printk kthread never takes over
> and systems eventually lockups. With SMP threads we can wake up printk
> kthread on a remote CPU (and we know that it will be woken up on a remote
> CPU), so per my tests SMP thread-ed version of printing offloading works
> much better. But more tests are needed.
> 
> 	The patch set is in RFC stage. I think I'll move the whole
> offloading thing under CONFIG_PRINTK_OFFLOAD (e.g.) at some point.
> 
> 	As a side note, seems that with the SMP threaded implementation
> we can do (there are some constraints (!!), of course) some sort of less
> deadlock prone printk. Instead of calling into the scheduler, console_sem,
> console_unlock(), we can wake_up printk_kthread on a foreign CPU. So we will
> not take scheduler locks or console locks from this CPU. (very-very
> schematically):
> 
> int vprintk_emit(....)
> {
> 	logbuf_lock_irqsave(flags);
> 
> 	[..]
> 	printed_len += log_output(facility, level, lflags, dict, dictlen, text, text_len);
> 
> 	set_bit(PRINTK_PENDING_OUTPUT, &printk_pending);
> 
> 	for_each_cpu_and(cpu, cpu_online_mask, &printk_cpumask) {
> 		if (cpu != smp_processor_id())
> 			wake_up_process(per_cpu(printk_kthread, cpu));
> 	}
> 
> 	logbuf_unlock_irqrestore(flags);
> 	return printed_len;
> }

to be clear, this patch set is a RFC to "just another idea to try out".
it's not meant to be "the final proposal". it has it's pros and cons.


so yet one more alternative solution... here it goes


for offloading don't just wake_up_process(printk_kthread) and hope for
the best, but instead queue IRQ work on foreign CPUs that would try to
bring up the printk_kthread.. CPUs that can process IRQ also can do
rescheduling and probably will have better chances to wake_up printk_kthread
than current printing CPU.



in other words, in console_offload_printing() do something like this
(composed in email client)


	for_each_cpu_and(cpu, cpu_online_mask, &printk_cpumask) {
		set_bit(PRINTK_PENDING_PRINTK_OFFLOAD, &printk_pending);
		irq_work_queue_on(per_cpu_ptr(&wake_up_klogd_work, cpu), cpu);
	}


and in wake_up_klogd_work_func() do

	if (test_and_clear_bit(PRINTK_PENDING_PRINTK_OFFLOAD, &printk_pending))
		wake_up_process(printk_kthread);


and _probably_ we can do two things in wake_up_klogd_work_func() for
PRINTK_PENDING_PRINTK_OFFLOAD. both wake_up() printk_kthread  AND
"if (console_trylock()) console_unlock()". like this:


	if (test_and_clear_bit(PRINTK_PENDING_PRINTK_OFFLOAD, &printk_pending)) {
		wake_up_process(printk_kthread);

		if (console_trylock())
			console_unlock();
	}


the rational is that one of the CPUs has requested offloading and we need to
help that CPU out. that console_unlock() from wake_up_klogd_work_func() will
be under the same `atomic_print_limit' constrains, so it will eventually request
offloading, if there will be too many messages to handle. hopefully not too
late.

we still don't have guarantees that printk_kthread will be scheduled on
a CPU that can run it immediately or anytime in the future, but not too
late. printk_kthread can even be scheduled on the CPU that has requested
offloading in the first place (am I wrong on this assumption?). which means
that printk_kthread may not be able to take over (think of a printk() dump
from IRQ context). that's the reason why I additionally want to
console_trylock() for PRINTK_PENDING_PRINTK_OFFLOAD.


but console_trylock() is not really reliable. not at all. it's a very fast
one shot action that most likely will see console_sem still being locked
but the CPU that has requested offloading. I want something to wait on
console_sem, that's reliably. and we need woken up (running) printk_kthread
for that.


may be... we can set CPU affinity on printk_kthread before we wake it up?
place into rq of the CPU that is processing PRINTK_PENDING_PRINTK_OFFLOAD?

or allow it on any CPU but the currently printing CPU.

or have per-CPU printk kthreads and wake_up processes that are already
bound to specific rq-s. (I know it's ugly, to put it politely. I'm just
trying different solutions/approaches).

	-ss