releases/4.14.60/btrfs-add-barriers-to-btrfs_sync_log-before-log_commit_wait-wakeups.patch

   1 From foo@baz Sat Jul 28 10:25:26 CEST 2018
   2 From: David Sterba <dsterba@suse.com>
   3 Date: Tue, 24 Apr 2018 14:53:56 +0200
   4 Subject: btrfs: add barriers to btrfs_sync_log before log_commit_wait wakeups
   5
   6 From: David Sterba <dsterba@suse.com>
   7
   8 [ Upstream commit 3d3a2e610ea5e7c6d4f9481ecce5d8e2d8317843 ]
   9
  10 Currently the code assumes that there's an implied barrier by the
  11 sequence of code preceding the wakeup, namely the mutex unlock.
  12
  13 As Nikolay pointed out:
  14
  15 I think this is wrong (not your code) but the original assumption that
  16 the RELEASE semantics provided by mutex_unlock is sufficient.
  17 According to memory-barriers.txt:
  18
  19 Section 'LOCK ACQUISITION FUNCTIONS' states:
  20
  21  (2) RELEASE operation implication:
  22
  23      Memory operations issued before the RELEASE will be completed before the
  24      RELEASE operation has completed.
  25
  26      Memory operations issued after the RELEASE *may* be completed before the
  27      RELEASE operation has completed.
  28
  29 (I've bolded the may portion)
  30
  31 The example given there:
  32
  33 As an example, consider the following:
  34
  35     *A = a;
  36     *B = b;
  37     ACQUIRE
  38     *C = c;
  39     *D = d;
  40     RELEASE
  41     *E = e;
  42     *F = f;
  43
  44 The following sequence of events is acceptable:
  45
  46     ACQUIRE, {*F,*A}, *E, {*C,*D}, *B, RELEASE
  47
  48 So if we assume that *C is modifying the flag which the waitqueue is checking,
  49 and *E is the actual wakeup, then those accesses can be re-ordered...
  50
  51 IMHO this code should be considered broken...
  52 Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
  53 ---
  54
  55 To be on the safe side, add the barriers. The synchronization logic
  56 around log using the mutexes and several other threads does not make it
  57 easy to reason for/against the barrier.
  58
  59 CC: Nikolay Borisov <nborisov@suse.com>
  60 Link: https://lkml.kernel.org/r/6ee068d8-1a69-3728-00d1-d86293d43c9f@suse.com
  61 Reviewed-by: Nikolay Borisov <nborisov@suse.com>
  62 Signed-off-by: David Sterba <dsterba@suse.com>
  63
  64 Signed-off-by: Sasha Levin <alexander.levin@microsoft.com>
  65 Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
  66 ---
  67  fs/btrfs/tree-log.c |   10 ++++++++--
  68  1 file changed, 8 insertions(+), 2 deletions(-)
  69
  70 --- a/fs/btrfs/tree-log.c
  71 +++ b/fs/btrfs/tree-log.c
  72 @@ -3041,8 +3041,11 @@ out_wake_log_root:
  73         mutex_unlock(&log_root_tree->log_mutex);
  74
  75         /*
  76 -        * The barrier before waitqueue_active is implied by mutex_unlock
  77 +        * The barrier before waitqueue_active is needed so all the updates
  78 +        * above are seen by the woken threads. It might not be necessary, but
  79 +        * proving that seems to be hard.
  80          */
  81 +       smp_mb();
  82         if (waitqueue_active(&log_root_tree->log_commit_wait[index2]))
  83                 wake_up(&log_root_tree->log_commit_wait[index2]);
  84  out:
  85 @@ -3053,8 +3056,11 @@ out:
  86         mutex_unlock(&root->log_mutex);
  87
  88         /*
  89 -        * The barrier before waitqueue_active is implied by mutex_unlock
  90 +        * The barrier before waitqueue_active is needed so all the updates
  91 +        * above are seen by the woken threads. It might not be necessary, but
  92 +        * proving that seems to be hard.
  93          */
  94 +       smp_mb();
  95         if (waitqueue_active(&root->log_commit_wait[index1]))
  96                 wake_up(&root->log_commit_wait[index1]);
  97         return ret;