Messages in this thread | | | Subject | Re: [PATCH v2 0/2] Add ADXL367 driver | From | Lars-Peter Clausen <> | Date | Mon, 13 Dec 2021 12:34:19 +0100 |
| |
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.
| |