-
Notifications
You must be signed in to change notification settings - Fork 71
READ_ONCE and WRITE_ONCE
There are several reasons to use at least READ_ONCE and WRITE_ONCE for all shared memory accesses:
- It makes code easier to understand
- It allows automatic data race detection
- It is required by relevant standards
There are no reasons to not use them (see Performance considerations).
Accesses to shared memory is an important thing and it should be clearly visible in source code. Whether the accessed memory location can be concurrently read/written significantly affects the way you reason about the code. For example, consider the following code:
tty = port->itty;
if (tty == NULL)
return;It looks like nothing special and does not suggest that this "simple" piece of code actually hides non-trivial synchronization protocol. If the code would look like:
tty = READ_ONCE(port->itty);
if (tty == NULL)
return;It would be clear that concurrent mutations of port->itty are possible, and thus greater attention is required (especially if you are tracking a bug). For example, what is the ownership story here? If port->itty can become NULL concurrently, can't it also be deleted?
Consider another piece of code which waits for a response from a device and then copies out reply:
int rc = -1;
...
wait_event_timeout(ps2dev->wait, !(ps2dev->flags & PS2_FLAG_CMD), timeout);
for (i = 0; i < receive; i++)
param[i] = ps2dev->cmdbuf[(receive - 1) - i];
if (ps2dev->cmdcnt)
goto out;
rc = 0;
out:
return rc;This contains a subtle bug: if the reply arrives right after timeout, then we can copyout uninitialized garbage as response expecting that we will return an error from the function; but at the point we check cmdcnt response has already arrived, so we return success and garbage as response.
If intentional accesses to shared memory are not marked in some way, then it is also impossible to automatically detect non-intentional accesses to shared memory (that is, forgotten locks, accesses from wrong threads, inconsistent reads of uint64, etc). There are tools that try to find these unpleasant classes of bugs (KTSAN, KernelStrider), but they cannot accomplish this task while kernel source code is sprinkled with intentional racy accesses that are indistinguishable from bugs.
Linux-Kernel Memory Model says:
Loads from and stores to normal variables should be protected with the ACCESS_ONCE() macro.
Similarly, C standard says:
5.1.2.4/25 The execution of a program contains a data race if it contains two conflicting actions in different threads, at least one of which is not atomic, and neither happens before the other. Any such data race results in undefined behavior.
As the consequence C compilers stopped guarantying that "word accesses are atomic".
TODO