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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux