Re: [PATCH v2 0/2] Add ADXL367 driver

[Lars-Peter Clausen <lars@metafoo.de>]

On 12/7/21 10:43 AM, Cosmin Tanislav wrote:
> I have one question that is not actually specific to this driver but would
> help me clear up some issues.
>
> I used mutex_lock and mutex_unlock when accessing anything in driver's
> state that could potentially be written by another process in parallel.
>
> I heard mixed opinions about this. Some people said that it is not
> necessary to lock everywhere because loads and stores for data with size
> smaller or equal than register size would be done in one single atomic
> instruction.
>
> On the other hand, I also heard that this is not true unless WRITE_ONCE
> and READ_ONCE is used.
>
> It felt weird using WRITE_ONCE and READ_ONCE in this driver, so I kept
> using mutexes.
>
> Could I get some opinions on this matter?

What you wrote sums it up very well. READ_ONCE/WRITE_ONCE are required 
for correctness when no lock is used. The compiler is allowed to do all 
sorts of optimizations that could break multi-threading, when 
READ_ONCE/WRITE_ONCE is not used. E.g.

if (x)
   foo->bar = 10;
else
   foo->bar = 20;

Could be implemented as

foo->bar = 20;
if (x)
   foo->bar = 10;

In the absence of multi-threading the result will be the same. But if 
another thread reads foo->bar just at the right time it will read the 
incorrect 20.

For simple things like `foo->bar = x;` it is unlikely that the compiler 
will do anything other than the single store. But it could and the code 
is not correct without the WRITE_ONCE.

Using a mutex is OK, since non of this is performance critical.