Re: [PATCH v5 0/2] printk: Console owner and waiter logic cleanup

Sergey Senozhatsky
2018-01-23T15:21:36Z

On (01/23/18 09:56), Steven Rostedt wrote:
[..]
> > Why do we even use irq_work for printk_safe?
>
> Why not?
>
> Really, I think you are trying to solve a symptom and not the problem.
> If we are having issues with irq_work, we are going to have issues with
> a work queue. It's just spreading out the problem instead of fixing it.

I don't want to have heuristics in print_safe, I don't want to have a magic
number controlled by a user-space visible knob, I don't want to have the
first 3 lines of a lockdep splat.

The problem is - we flush printk_safe too soon and printing CPU ends up
in a lockup - it log_store()-s new messages while it's printing the pending
ones. It's fine to do so when CPU is in preemptible context. Really, we
should not care in printk_safe as long as we don't lockup the kernel. The
misbehaving console must be fixed. If CPU is not in preemptible context then
we do lockup the kernel. Because we flush printk_safe regardless of the
current CPU context. If we will flush printk_safe via WQ then we automatically
add this "OK! The CPU is preemptible, we can log_store(), it's totally OK, we
will not lockup it up." thing. Yes, we fill up the logbuf with probably needed
and appreciated or unneeded messages. But we should not care in printk_safe.
We don't lockup the kernel... And the misbehaving console must be fixed.

I disagree with "If we are having issues with irq_work, we are going to have
issues with a work queue". There is a tremendous difference between irq_work
on that CPU and queue_work_on(smp_proessor_id()). One does not care about CPU
context, the other one does.

-ss