diff options
| author | David Sterba <[email protected]> | 2018-04-24 12:53:56 +0000 |
|---|---|---|
| committer | David Sterba <[email protected]> | 2018-05-28 16:23:06 +0000 |
| commit | 3d3a2e610ea5e7c6d4f9481ecce5d8e2d8317843 (patch) | |
| tree | 1aeb76ba83d5a47cd0f580267d6b97966d531ce0 /fs/btrfs/dev-replace.c | |
| parent | btrfs: introduce conditional wakeup helpers (diff) | |
| download | kernel-3d3a2e610ea5e7c6d4f9481ecce5d8e2d8317843.tar.gz kernel-3d3a2e610ea5e7c6d4f9481ecce5d8e2d8317843.zip | |
btrfs: add barriers to btrfs_sync_log before log_commit_wait wakeups
Currently the code assumes that there's an implied barrier by the
sequence of code preceding the wakeup, namely the mutex unlock.
As Nikolay pointed out:
I think this is wrong (not your code) but the original assumption that
the RELEASE semantics provided by mutex_unlock is sufficient.
According to memory-barriers.txt:
Section 'LOCK ACQUISITION FUNCTIONS' states:
(2) RELEASE operation implication:
Memory operations issued before the RELEASE will be completed before the
RELEASE operation has completed.
Memory operations issued after the RELEASE *may* be completed before the
RELEASE operation has completed.
(I've bolded the may portion)
The example given there:
As an example, consider the following:
*A = a;
*B = b;
ACQUIRE
*C = c;
*D = d;
RELEASE
*E = e;
*F = f;
The following sequence of events is acceptable:
ACQUIRE, {*F,*A}, *E, {*C,*D}, *B, RELEASE
So if we assume that *C is modifying the flag which the waitqueue is checking,
and *E is the actual wakeup, then those accesses can be re-ordered...
IMHO this code should be considered broken...
---
To be on the safe side, add the barriers. The synchronization logic
around log using the mutexes and several other threads does not make it
easy to reason for/against the barrier.
CC: Nikolay Borisov <[email protected]>
Link: https://lkml.kernel.org/r/[email protected]
Reviewed-by: Nikolay Borisov <[email protected]>
Signed-off-by: David Sterba <[email protected]>
Diffstat (limited to 'fs/btrfs/dev-replace.c')
0 files changed, 0 insertions, 0 deletions
