Messages in this thread | | | Date | Wed, 9 Dec 2020 05:35:39 +0900 | From | Sergey Senozhatsky <> | Subject | Re: [PATCH next v3 3/3] printk: remove logbuf_lock protection for ringbuffer |
| |
On (20/12/07 23:26), John Ogness wrote: > diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c > index e1f068677a74..f3c0fcc3163f 100644 > --- a/kernel/printk/printk.c > +++ b/kernel/printk/printk.c > @@ -1068,7 +1068,6 @@ void __init setup_log_buf(int early) > struct printk_record r; > size_t new_descs_size; > size_t new_infos_size; > - unsigned long flags; > char *new_log_buf; > unsigned int free; > u64 seq; > @@ -1126,8 +1125,6 @@ void __init setup_log_buf(int early) > new_descs, ilog2(new_descs_count), > new_infos); > > - logbuf_lock_irqsave(flags); > - > log_buf_len = new_log_buf_len; > log_buf = new_log_buf; > new_log_buf_len = 0; > @@ -1143,8 +1140,6 @@ void __init setup_log_buf(int early) > */ > prb = &printk_rb_dynamic; > > - logbuf_unlock_irqrestore(flags);
logbuf_lock_irqsave() does two things - it locks the logbuf lock and enables the printk_safe gating. While we can drop the former, the latter one must stay until we have a complete replacement.
Looking more:
> --- > logbuf_lock_irqsave(flags); > > log_buf_len = new_log_buf_len; > log_buf = new_log_buf; > new_log_buf_len = 0; > > free = __LOG_BUF_LEN; > prb_for_each_record(0, &printk_rb_static, seq, &r) > free -= add_to_rb(&printk_rb_dynamic, &r); > > /* > * This is early enough that everything is still running on the > * boot CPU and interrupts are disabled. So no new messages will > * appear during the transition to the dynamic buffer. > */ > prb = &printk_rb_dynamic; > > logbuf_unlock_irqrestore(flags); ---
I'd say that I'd prefer to keep logbuf initialisation under printk_safe(), per-CPU buffer can be already initialised at this point. Strictly speaking we can have new message during transition to the dynamic buffer - there are functions there that can pr_err/warn while we prb_for_each_record/add_to_rb.
> -ss
| |