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.
