]> git.proxmox.com Git - mirror_ubuntu-focal-kernel.git/commit
btrfs: add barriers to btrfs_sync_log before log_commit_wait wakeups
authorDavid Sterba <dsterba@suse.com>
Tue, 24 Apr 2018 12:53:56 +0000 (14:53 +0200)
committerDavid Sterba <dsterba@suse.com>
Mon, 28 May 2018 16:23:06 +0000 (18:23 +0200)
commit3d3a2e610ea5e7c6d4f9481ecce5d8e2d8317843
tree1aeb76ba83d5a47cd0f580267d6b97966d531ce0
parent110a21feedd78d398598d91be57db60e19b76fe0
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 <nborisov@suse.com>
Link: https://lkml.kernel.org/r/6ee068d8-1a69-3728-00d1-d86293d43c9f@suse.com
Reviewed-by: Nikolay Borisov <nborisov@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
fs/btrfs/tree-log.c