On Fri 2023-06-23 10:12:50, Sebastian Andrzej Siewior wrote:
> On 2023-06-21 17:38:03 [+0200], Petr Mladek wrote:
> > But wait. AFAIK, the printk kthread is implemented only for the serial
> > console. So, it won't be safe for other consoles, especially
> > the problematic tty_insert_flip_string_and_push_buffer() call.
>
> > OK, what about using migrate_disable() in printk_deferred_enter()?
> > Something like:
> >
> > /*
> > * The printk_deferred_enter/exit macros are available only as a hack.
> > * They define a per-CPU context where all printk console printing
> > * is deferred because it might cause a deadlock otherwise.
> > *
> > * It is highly recommended to use them only in a context with interrupts
> > * disabled. Otherwise, other unrelated printk() calls might be deferred
> > * when they interrupt/preempt the deferred code section.
> > *
> > * They should not be used for to deffer printing of many messages. It might
> > * create softlockup when they are flushed later.
> > *
> > * IMPORTANT: Any new use of these MUST be consulted with printk maintainers.
> > * It might have unexpected side effects on the printk infrastructure.
> > */
> > #ifdef CONFIG_PREEMPT_RT
> >
> > #define printk_deferred_enter() \
> > do { \
> > migrate_disable(); \
> > __printk_safe_enter(); \
> > } while (0)
> >
> > #define printk_deferred_exit() \
> > do { \
> > __printk_safe_exit(); \
> > migrate_enable(); \
> > } while (0)
> >
> > #else /* CONFIG_PREEMPT_RT */
> >
> > #define printk_deferred_enter() \
> > do { \
> > preempt_disable(); \
> > __printk_safe_enter(); \
> > } while (0)
> >
> > #define printk_deferred_exit() \
> > do { \
> > __printk_safe_exit(); \
> > preempt_enable(); \
> > } while (0)
> >
> > #endif /* CONFIG_PREEMPT_RT */
> >
> > Note that I have used preempt_disable() on non-RT because it is much
> > cheaper. And IRQs will be disabled anyway on non-RT system
> > in this code path.
>
> It would do _but_ is it really needed? All users but one (the one in
> __build_all_zonelists()) have interrupts already disabled. This isn't a
> hot path and is not used very common. Does it justify the ifdef?
>
> An unconditional migrate_disable() would do the job if you want it to be
> safe
Makes sense. The ifdef does not look worth it.
> but I would rather suggest lockdep annotation instead of disabling
> either preemption or migration. If I read the comment on top right, this
> API isn't open for the public.
It might work when mm people agree to explicitly call
migrate_disable() in __build_all_zonelists(). I mean
to call there:
migrate_disable();
printk_deferred_enter();
write_seqlock_irqsave(&zonelist_update_seq, flags);
Plus it would require some comments.
For me, it is acceptable to hide at least the migrate_disable() part
into printk_deferred_enter().
> The global variable would be probably be simpler but I guess you want
> to have direct printing on other CPUs.
Yes, the scope should be as limited as possible.
> To be honst, I would suggest to work towards always printing in a thread
> with an emergency console than this deferred/ safe duckt tape.
I have used this argument many years. But it is not much convincing
after 10 years. Note that John has sent 1st RFC 4 years ago and we
still do not have the kthreads :-/
Best Regards,
Petr
