avoid-lockdep-warning-when-inheriting-encryption-context

   1 ext4: avoid lockdep warning when inheriting encryption context
   2
   3 From: Eric Biggers <ebiggers@google.com>
   4
   5 On a lockdep-enabled kernel, xfstests generic/027 fails due to a lockdep
   6 warning when run on ext4 mounted with -o test_dummy_encryption:
   7
   8     xfs_io/4594 is trying to acquire lock:
   9      (jbd2_handle
  10     ){++++.+}, at:
  11     [<ffffffff813096ef>] jbd2_log_wait_commit+0x5/0x11b
  12
  13     but task is already holding lock:
  14      (jbd2_handle
  15     ){++++.+}, at:
  16     [<ffffffff813000de>] start_this_handle+0x354/0x3d8
  17
  18 The abbreviated call stack is:
  19
  20  [<ffffffff813096ef>] ? jbd2_log_wait_commit+0x5/0x11b
  21  [<ffffffff8130972a>] jbd2_log_wait_commit+0x40/0x11b
  22  [<ffffffff813096ef>] ? jbd2_log_wait_commit+0x5/0x11b
  23  [<ffffffff8130987b>] ? __jbd2_journal_force_commit+0x76/0xa6
  24  [<ffffffff81309896>] __jbd2_journal_force_commit+0x91/0xa6
  25  [<ffffffff813098b9>] jbd2_journal_force_commit_nested+0xe/0x18
  26  [<ffffffff812a6049>] ext4_should_retry_alloc+0x72/0x79
  27  [<ffffffff812f0c1f>] ext4_xattr_set+0xef/0x11f
  28  [<ffffffff812cc35b>] ext4_set_context+0x3a/0x16b
  29  [<ffffffff81258123>] fscrypt_inherit_context+0xe3/0x103
  30  [<ffffffff812ab611>] __ext4_new_inode+0x12dc/0x153a
  31  [<ffffffff812bd371>] ext4_create+0xb7/0x161
  32
  33 When a file is created in an encrypted directory, ext4_set_context() is
  34 called to set an encryption context on the new file.  This calls
  35 ext4_xattr_set(), which contains a retry loop where the journal is
  36 forced to commit if an ENOSPC error is encountered.
  37
  38 If the task actually were to wait for the journal to commit in this
  39 case, then it would deadlock because a handle remains open from
  40 __ext4_new_inode(), so the running transaction can't be committed yet.
  41 Fortunately, __jbd2_journal_force_commit() avoids the deadlock by not
  42 allowing the running transaction to be committed while the current task
  43 has it open.  However, the above lockdep warning is still triggered.
  44
  45 This was a false positive which was introduced by: 1eaa566d368b: jbd2:
  46 track more dependencies on transaction commit
  47
  48 Fix the problem by passing the handle through the 'fs_data' argument to
  49 ext4_set_context(), then using ext4_xattr_set_handle() instead of
  50 ext4_xattr_set().  And in the case where no journal handle is specified
  51 and ext4_set_context() has to open one, add an ENOSPC retry loop since
  52 in that case it is the outermost transaction.
  53
  54 Signed-off-by: Eric Biggers <ebiggers@google.com>
  55 ---
  56  fs/ext4/ialloc.c |  3 +--
  57  fs/ext4/super.c  | 32 ++++++++++++++++++++++----------
  58  2 files changed, 23 insertions(+), 12 deletions(-)
  59
  60 diff --git a/fs/ext4/ialloc.c b/fs/ext4/ialloc.c
  61 index 170421e..f543281 100644
  62 --- a/fs/ext4/ialloc.c
  63 +++ b/fs/ext4/ialloc.c
  64 @@ -1115,8 +1115,7 @@ struct inode *__ext4_new_inode(handle_t *handle, struct inode *dir,
  65         }
  66
  67         if (encrypt) {
  68 -               /* give pointer to avoid set_context with journal ops. */
  69 -               err = fscrypt_inherit_context(dir, inode, &encrypt, true);
  70 +               err = fscrypt_inherit_context(dir, inode, handle, true);
  71                 if (err)
  72                         goto fail_free_drop;
  73         }
  74 diff --git a/fs/ext4/super.c b/fs/ext4/super.c
  75 index ff6f3ab..22d50cb 100644
  76 --- a/fs/ext4/super.c
  77 +++ b/fs/ext4/super.c
  78 @@ -1114,14 +1114,22 @@ static int ext4_prepare_context(struct inode *inode)
  79  static int ext4_set_context(struct inode *inode, const void *ctx, size_t len,
  80                                                         void *fs_data)
  81  {
  82 -       handle_t *handle;
  83 -       int res, res2;
  84 +       handle_t *handle = fs_data;
  85 +       int res, res2, retries = 0;
  86
  87 -       /* fs_data is null when internally used. */
  88 -       if (fs_data) {
  89 -               res  = ext4_xattr_set(inode, EXT4_XATTR_INDEX_ENCRYPTION,
  90 -                               EXT4_XATTR_NAME_ENCRYPTION_CONTEXT, ctx,
  91 -                               len, 0);
  92 +       /*
  93 +        * If a journal handle was specified, then the encryption context is
  94 +        * being set on a new inode via inheritance and is part of a larger
  95 +        * transaction to create the inode.  Otherwise the encryption context is
  96 +        * being set on an existing inode in its own transaction.  Only in the
  97 +        * latter case should the "retry on ENOSPC" logic be used.
  98 +        */
  99 +
 100 +       if (handle) {
 101 +               res = ext4_xattr_set_handle(handle, inode,
 102 +                                           EXT4_XATTR_INDEX_ENCRYPTION,
 103 +                                           EXT4_XATTR_NAME_ENCRYPTION_CONTEXT,
 104 +                                           ctx, len, 0);
 105                 if (!res) {
 106                         ext4_set_inode_flag(inode, EXT4_INODE_ENCRYPT);
 107                         ext4_clear_inode_state(inode,
 108 @@ -1130,14 +1138,15 @@ static int ext4_set_context(struct inode *inode, const void *ctx, size_t len,
 109                 return res;
 110         }
 111
 112 +retry:
 113         handle = ext4_journal_start(inode, EXT4_HT_MISC,
 114                         ext4_jbd2_credits_xattr(inode));
 115         if (IS_ERR(handle))
 116                 return PTR_ERR(handle);
 117
 118 -       res = ext4_xattr_set(inode, EXT4_XATTR_INDEX_ENCRYPTION,
 119 -                       EXT4_XATTR_NAME_ENCRYPTION_CONTEXT, ctx,
 120 -                       len, 0);
 121 +       res = ext4_xattr_set_handle(handle, inode, EXT4_XATTR_INDEX_ENCRYPTION,
 122 +                                   EXT4_XATTR_NAME_ENCRYPTION_CONTEXT,
 123 +                                   ctx, len, 0);
 124         if (!res) {
 125                 ext4_set_inode_flag(inode, EXT4_INODE_ENCRYPT);
 126                 res = ext4_mark_inode_dirty(handle, inode);
 127 @@ -1145,6 +1154,9 @@ static int ext4_set_context(struct inode *inode, const void *ctx, size_t len,
 128                         EXT4_ERROR_INODE(inode, "Failed to mark inode dirty");
 129         }
 130         res2 = ext4_journal_stop(handle);
 131 +
 132 +       if (res == -ENOSPC && ext4_should_retry_alloc(inode->i_sb, &retries))
 133 +               goto retry;
 134         if (!res)
 135                 res = res2;
 136         return res;
 137 --
 138 2.8.0.rc3.226.g39d4020
 139
 140