summaryrefslogtreecommitdiffstats
path: root/fs
diff options
context:
space:
mode:
authorDavid Sterba <dsterba@suse.com>2018-04-24 14:53:56 +0200
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>2018-08-03 07:50:28 +0200
commit31371d2dad493ce0bc7892cb6c1c3f5f397e12b9 (patch)
tree72d30f4ef3a638d31d1982199a3340100f63ca4b /fs
parent3bf165384e82f70571ba2f439391dab86eddb23d (diff)
downloadlinux-stable-31371d2dad493ce0bc7892cb6c1c3f5f397e12b9.tar.gz
linux-stable-31371d2dad493ce0bc7892cb6c1c3f5f397e12b9.tar.bz2
linux-stable-31371d2dad493ce0bc7892cb6c1c3f5f397e12b9.zip
btrfs: add barriers to btrfs_sync_log before log_commit_wait wakeups
[ Upstream commit 3d3a2e610ea5e7c6d4f9481ecce5d8e2d8317843 ] 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... Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Diffstat (limited to 'fs')
-rw-r--r--fs/btrfs/tree-log.c10
1 files changed, 8 insertions, 2 deletions
diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c
index bf4e22df7c97..e1b4a59485df 100644
--- a/fs/btrfs/tree-log.c
+++ b/fs/btrfs/tree-log.c
@@ -3041,8 +3041,11 @@ out_wake_log_root:
mutex_unlock(&log_root_tree->log_mutex);
/*
- * The barrier before waitqueue_active is implied by mutex_unlock
+ * The barrier before waitqueue_active is needed so all the updates
+ * above are seen by the woken threads. It might not be necessary, but
+ * proving that seems to be hard.
*/
+ smp_mb();
if (waitqueue_active(&log_root_tree->log_commit_wait[index2]))
wake_up(&log_root_tree->log_commit_wait[index2]);
out:
@@ -3053,8 +3056,11 @@ out:
mutex_unlock(&root->log_mutex);
/*
- * The barrier before waitqueue_active is implied by mutex_unlock
+ * The barrier before waitqueue_active is needed so all the updates
+ * above are seen by the woken threads. It might not be necessary, but
+ * proving that seems to be hard.
*/
+ smp_mb();
if (waitqueue_active(&root->log_commit_wait[index1]))
wake_up(&root->log_commit_wait[index1]);
return ret;