Messages in this thread | | | Date | Tue, 3 Mar 2020 10:18:47 +0100 | From | Petr Mladek <> | Subject | Re: [PATCH] printk: queue wake_up_klogd irq_work only if per-CPU areas are ready |
| |
On Tue 2020-03-03 13:40:59, Sergey Senozhatsky wrote: > Lech Perczak [0] reports that after commit 1b710b1b10ef > ("char/random: silence a lockdep splat with printk()") > user-space syslog/kmsg readers are not able to read new > kernel messages. The reason is printk_deferred() being > called too early (as was pointed out by Petr and John). > > Fix printk_deferred() and do not queue per-CPU irq_work > before per-CPU areas are initialized. > > diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c > index ad4606234545..d951d35a0786 100644 > --- a/kernel/printk/printk.c > +++ b/kernel/printk/printk.c > @@ -1147,12 +1159,25 @@ static void __init log_buf_add_cpu(void) > static inline void log_buf_add_cpu(void) {} > #endif /* CONFIG_SMP */ > > +static void __init set_percpu_data_ready(void) > +{ > + __printk_percpu_data_ready = true; > +} > + > void __init setup_log_buf(int early) > { > unsigned long flags; > char *new_log_buf; > unsigned int free; > > + /* > + * Some archs call setup_log_buf() multiple times - first is very > + * early, e.g. from setup_arch(), and second - when percpu_areas > + * are initialised. > + */ > + if (!early) > + set_percpu_data_ready(); > + > if (log_buf != __log_buf) > return; > > diff --git a/kernel/printk/printk_safe.c b/kernel/printk/printk_safe.c > index b4045e782743..d9a659a686f3 100644 > --- a/kernel/printk/printk_safe.c > +++ b/kernel/printk/printk_safe.c > @@ -27,7 +27,6 @@ a> * There are situations when we want to make sure that all buffers > * were handled or when IRQs are blocked. > */ > -static int printk_safe_irq_ready __read_mostly; > > #define SAFE_LOG_BUF_LEN ((1 << CONFIG_PRINTK_SAFE_LOG_BUF_SHIFT) - \ > sizeof(atomic_t) - \ > @@ -51,7 +50,7 @@ static DEFINE_PER_CPU(struct printk_safe_seq_buf, nmi_print_seq); > /* Get flushed in a more safe context. */ > static void queue_flush_work(struct printk_safe_seq_buf *s) > { > - if (printk_safe_irq_ready) > + if (printk_percpu_data_ready()) > irq_work_queue(&s->work);
This is not safe. printk_percpu_data_ready() returns true even before s->work gets initialized by printk_safe_init().
Solution would be to call printk_safe_init() from setup_log_buf() before calling set_percpu_data_ready().
Or I would keep printk_safe code as it. I hope that it will get removed rather soon anyway.
Otherwise, it looks good to me.
Best Regards, Petr
| |