Messages in this thread | | | Date | Mon, 1 Nov 2021 11:06:23 +0100 | From | Petr Mladek <> | Subject | Re: [PATCH v2] printk: ringbuffer: Improve prb_next_seq() performance |
| |
On Wed 2021-10-27 17:54:42, John Ogness wrote: > On 2021-10-27, Petr Mladek <pmladek@suse.com> wrote: > > diff --git a/kernel/printk/printk_ringbuffer.c b/kernel/printk/printk_ringbuffer.c > > index 8a7b7362c0dd..24f47fbefbb5 100644 > > --- a/kernel/printk/printk_ringbuffer.c > > +++ b/kernel/printk/printk_ringbuffer.c > > @@ -474,8 +474,10 @@ static enum desc_state desc_read(struct prb_desc_ring *desc_ring, > > * state has been re-checked. A memcpy() for all of @desc > > * cannot be used because of the atomic_t @state_var field. > > */ > > - memcpy(&desc_out->text_blk_lpos, &desc->text_blk_lpos, > > - sizeof(desc_out->text_blk_lpos)); /* LMM(desc_read:C) */ > > + if (desc_out) { > > + memcpy(&desc_out->text_blk_lpos, &desc->text_blk_lpos, > > + sizeof(desc_out->text_blk_lpos)); /* LMM(desc_read:C) */ > > + } > > if (seq_out) > > *seq_out = info->seq; /* also part of desc_read:C */ > > if (caller_id_out) > > You also need this hunk: > > @@ -530,7 +530,8 @@ static enum desc_state desc_read(struct prb_desc_ring *desc_ring, > state_val = atomic_long_read(state_var); /* LMM(desc_read:E) */ > d_state = get_desc_state(id, state_val); > out: > - atomic_long_set(&desc_out->state_var, state_val); > + if (desc_out) > + atomic_long_set(&desc_out->state_var, state_val); > return d_state; > } > > Sorry, I missed it in my last review. This time I build+boot tested the patch.
Urgh. Great catch! It seems that my testing was not good enough. Or that I was just lucky.
> With this hunk added: > > Reviewed-by: John Ogness <john.ogness@linutronix.de>
I am going to queue it for 5.17 after the pull request for 5.16 is accepted. I do not feel comfortable to rush this into 5.16, especially after finding the bug. The merge window has just started...
Anyway, thanks a lot for review and catching the mistake.
Best Regards, Petr
| |