Re: [PATCH v5 0/2] printk: Console owner and waiter logic cleanup
From: Sergey Senozhatsky
Date: Wed Jan 17 2018 - 01:29:31 EST
On (01/16/18 11:13), Petr Mladek wrote:
[..]
> IMHO, it would make sense if flushing the printk buffer behaves
> the same when called either from printk() or from any other path.
> I mean that it should be aggressive and allow an effective
> hand off.
>
> It should be safe as long as foo_specific_work() does not take
> too much time.
>
> From other side. The cond_resched() in console_unlock() should
> be obsoleted by the hand-shake code.
hm, let's not have too optimistic expectations. hand off works in very
specific conditions. console is not exclusively owned by printk, and
console_sem is not printk's own lock. even things like
systemd -> n_tty_write -> do_output_char -> con_write
involves console_lock() and console_unlock(). IOW user space
logging/debugging can cause printk stalls, and vice versa.
by the way, do_con_write() explicitly calls console_conditional_schedule()
under console_sem, before it goes to console_unlock(). so the scope of
"situation normal, console_sem locked, the owner scheduled out" is much
bigger than just vprintk_emit() -> console_unlock(). IMHO.
and there are even more things there. personally, I don't think
that hand off is enough to obsolete anything in that area.
[...]
> They both were obvious solutions that helped to reduce the risk
> of soft-lockups. The first one handled evidently safe scenarios.
> The second one was even more aggressive. I would say that
> they both were more or less add-hoc solutions that did not
> take into account the other side effects (delaying output,
> even loosing messages).
agreed.
> I would not say that one is a diametric difference between them.
> Therefore if we remove one for a reason, we should think about
> reverting the other as well. But again. I am fine if we remove
> only one now.
>
> Does this make any sense?
I see cond_resched() as a mirroring of console_lock()->console_unlock()
behaviour on PREEMPT systems, and as such it looks valid to me, so we
probably better keep it there. IMHO.
-ss