Re: [PATCH 2/2] printk: ringbuffer: avoid memcpy() on state_var

From: Petr Mladek
Date: Mon Sep 14 2020 - 08:48:40 EST


On Mon 2020-09-14 11:54:03, John Ogness wrote:
> @state_var is copied as part of the descriptor copying via
> memcpy(). This is not allowed because @state_var is an atomic type,
> which in some implementations may contain a spinlock.

Great catch!

> Avoid using memcpy() with @state_var by explicitly copying the other
> fields of the descriptor. @state_var is set using atomic set
> operator before returning.

Just thinking loudly.

Strictly speaking, memcpy() might be used when the atomic variable
is later corrected by an atomic operation. And it is done by the first
patch.

I think that it is a matter of taste what solution is more error
prone. Either this function must be updated when a new field
is added into struct prb_desc or the atomic_long_set() must not
get removed.

Hmm, missing memcpy() should be rather easy to debug. While missing
atomic_long_set() is really subtle problem problem. So, the new
code is better from my POV.


> Fixes: b6cf8b3f3312 ("printk: add lockless ringbuffer")
> Signed-off-by: John Ogness <john.ogness@xxxxxxxxxxxxx>

Reviewed-by: Petr Mladek <pmladek@xxxxxxxx>

Best Regards,
Petr