releases/3.6.2/ext4-move_extent-code-cleanup.patch

   1 From 03bd8b9b896c8e940f282f540e6b4de90d666b7c Mon Sep 17 00:00:00 2001
   2 From: Dmitry Monakhov <dmonakhov@openvz.org>
   3 Date: Wed, 26 Sep 2012 12:32:19 -0400
   4 Subject: ext4: move_extent code cleanup
   5
   6 From: Dmitry Monakhov <dmonakhov@openvz.org>
   7
   8 commit 03bd8b9b896c8e940f282f540e6b4de90d666b7c upstream.
   9
  10 - Remove usless checks, because it is too late to check that inode != NULL
  11   at the moment it was referenced several times.
  12 - Double lock routines looks very ugly and locking ordering relays on
  13   order of i_ino, but other kernel code rely on order of pointers.
  14   Let's make them simple and clean.
  15 - check that inodes belongs to the same SB as soon as possible.
  16
  17 Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
  18 Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
  19 Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
  20
  21 ---
  22  fs/ext4/move_extent.c |  167 ++++++++++++++------------------------------------
  23  1 file changed, 47 insertions(+), 120 deletions(-)
  24
  25 --- a/fs/ext4/move_extent.c
  26 +++ b/fs/ext4/move_extent.c
  27 @@ -141,55 +141,21 @@ mext_next_extent(struct inode *inode, st
  28  }
  29
  30  /**
  31 - * mext_check_null_inode - NULL check for two inodes
  32 - *
  33 - * If inode1 or inode2 is NULL, return -EIO. Otherwise, return 0.
  34 - */
  35 -static int
  36 -mext_check_null_inode(struct inode *inode1, struct inode *inode2,
  37 -                     const char *function, unsigned int line)
  38 -{
  39 -       int ret = 0;
  40 -
  41 -       if (inode1 == NULL) {
  42 -               __ext4_error(inode2->i_sb, function, line,
  43 -                       "Both inodes should not be NULL: "
  44 -                       "inode1 NULL inode2 %lu", inode2->i_ino);
  45 -               ret = -EIO;
  46 -       } else if (inode2 == NULL) {
  47 -               __ext4_error(inode1->i_sb, function, line,
  48 -                       "Both inodes should not be NULL: "
  49 -                       "inode1 %lu inode2 NULL", inode1->i_ino);
  50 -               ret = -EIO;
  51 -       }
  52 -       return ret;
  53 -}
  54 -
  55 -/**
  56   * double_down_write_data_sem - Acquire two inodes' write lock of i_data_sem
  57   *
  58 - * @orig_inode:                original inode structure
  59 - * @donor_inode:       donor inode structure
  60 - * Acquire write lock of i_data_sem of the two inodes (orig and donor) by
  61 - * i_ino order.
  62 + * Acquire write lock of i_data_sem of the two inodes
  63   */
  64  static void
  65 -double_down_write_data_sem(struct inode *orig_inode, struct inode *donor_inode)
  66 +double_down_write_data_sem(struct inode *first, struct inode *second)
  67  {
  68 -       struct inode *first = orig_inode, *second = donor_inode;
  69 +       if (first < second) {
  70 +               down_write(&EXT4_I(first)->i_data_sem);
  71 +               down_write_nested(&EXT4_I(second)->i_data_sem, SINGLE_DEPTH_NESTING);
  72 +       } else {
  73 +               down_write(&EXT4_I(second)->i_data_sem);
  74 +               down_write_nested(&EXT4_I(first)->i_data_sem, SINGLE_DEPTH_NESTING);
  75
  76 -       /*
  77 -        * Use the inode number to provide the stable locking order instead
  78 -        * of its address, because the C language doesn't guarantee you can
  79 -        * compare pointers that don't come from the same array.
  80 -        */
  81 -       if (donor_inode->i_ino < orig_inode->i_ino) {
  82 -               first = donor_inode;
  83 -               second = orig_inode;
  84         }
  85 -
  86 -       down_write(&EXT4_I(first)->i_data_sem);
  87 -       down_write_nested(&EXT4_I(second)->i_data_sem, SINGLE_DEPTH_NESTING);
  88  }
  89
  90  /**
  91 @@ -969,14 +935,6 @@ mext_check_arguments(struct inode *orig_
  92                 return -EINVAL;
  93         }
  94
  95 -       /* Files should be in the same ext4 FS */
  96 -       if (orig_inode->i_sb != donor_inode->i_sb) {
  97 -               ext4_debug("ext4 move extent: The argument files "
  98 -                       "should be in same FS [ino:orig %lu, donor %lu]\n",
  99 -                       orig_inode->i_ino, donor_inode->i_ino);
 100 -               return -EINVAL;
 101 -       }
 102 -
 103         /* Ext4 move extent supports only extent based file */
 104         if (!(ext4_test_inode_flag(orig_inode, EXT4_INODE_EXTENTS))) {
 105                 ext4_debug("ext4 move extent: orig file is not extents "
 106 @@ -1072,35 +1030,19 @@ mext_check_arguments(struct inode *orig_
 107   * @inode1:    the inode structure
 108   * @inode2:    the inode structure
 109   *
 110 - * Lock two inodes' i_mutex by i_ino order.
 111 - * If inode1 or inode2 is NULL, return -EIO. Otherwise, return 0.
 112 + * Lock two inodes' i_mutex
 113   */
 114 -static int
 115 +static void
 116  mext_inode_double_lock(struct inode *inode1, struct inode *inode2)
 117  {
 118 -       int ret = 0;
 119 -
 120 -       BUG_ON(inode1 == NULL && inode2 == NULL);
 121 -
 122 -       ret = mext_check_null_inode(inode1, inode2, __func__, __LINE__);
 123 -       if (ret < 0)
 124 -               goto out;
 125 -
 126 -       if (inode1 == inode2) {
 127 -               mutex_lock(&inode1->i_mutex);
 128 -               goto out;
 129 -       }
 130 -
 131 -       if (inode1->i_ino < inode2->i_ino) {
 132 +       BUG_ON(inode1 == inode2);
 133 +       if (inode1 < inode2) {
 134                 mutex_lock_nested(&inode1->i_mutex, I_MUTEX_PARENT);
 135                 mutex_lock_nested(&inode2->i_mutex, I_MUTEX_CHILD);
 136         } else {
 137                 mutex_lock_nested(&inode2->i_mutex, I_MUTEX_PARENT);
 138                 mutex_lock_nested(&inode1->i_mutex, I_MUTEX_CHILD);
 139         }
 140 -
 141 -out:
 142 -       return ret;
 143  }
 144
 145  /**
 146 @@ -1109,28 +1051,13 @@ out:
 147   * @inode1:     the inode that is released first
 148   * @inode2:     the inode that is released second
 149   *
 150 - * If inode1 or inode2 is NULL, return -EIO. Otherwise, return 0.
 151   */
 152
 153 -static int
 154 +static void
 155  mext_inode_double_unlock(struct inode *inode1, struct inode *inode2)
 156  {
 157 -       int ret = 0;
 158 -
 159 -       BUG_ON(inode1 == NULL && inode2 == NULL);
 160 -
 161 -       ret = mext_check_null_inode(inode1, inode2, __func__, __LINE__);
 162 -       if (ret < 0)
 163 -               goto out;
 164 -
 165 -       if (inode1)
 166 -               mutex_unlock(&inode1->i_mutex);
 167 -
 168 -       if (inode2 && inode2 != inode1)
 169 -               mutex_unlock(&inode2->i_mutex);
 170 -
 171 -out:
 172 -       return ret;
 173 +       mutex_unlock(&inode1->i_mutex);
 174 +       mutex_unlock(&inode2->i_mutex);
 175  }
 176
 177  /**
 178 @@ -1187,16 +1114,23 @@ ext4_move_extents(struct file *o_filp, s
 179         ext4_lblk_t block_end, seq_start, add_blocks, file_end, seq_blocks = 0;
 180         ext4_lblk_t rest_blocks;
 181         pgoff_t orig_page_offset = 0, seq_end_page;
 182 -       int ret1, ret2, depth, last_extent = 0;
 183 +       int ret, depth, last_extent = 0;
 184         int blocks_per_page = PAGE_CACHE_SIZE >> orig_inode->i_blkbits;
 185         int data_offset_in_page;
 186         int block_len_in_page;
 187         int uninit;
 188
 189 -       /* orig and donor should be different file */
 190 -       if (orig_inode->i_ino == donor_inode->i_ino) {
 191 +       if (orig_inode->i_sb != donor_inode->i_sb) {
 192 +               ext4_debug("ext4 move extent: The argument files "
 193 +                       "should be in same FS [ino:orig %lu, donor %lu]\n",
 194 +                       orig_inode->i_ino, donor_inode->i_ino);
 195 +               return -EINVAL;
 196 +       }
 197 +
 198 +       /* orig and donor should be different inodes */
 199 +       if (orig_inode == donor_inode) {
 200                 ext4_debug("ext4 move extent: The argument files should not "
 201 -                       "be same file [ino:orig %lu, donor %lu]\n",
 202 +                       "be same inode [ino:orig %lu, donor %lu]\n",
 203                         orig_inode->i_ino, donor_inode->i_ino);
 204                 return -EINVAL;
 205         }
 206 @@ -1210,16 +1144,14 @@ ext4_move_extents(struct file *o_filp, s
 207         }
 208
 209         /* Protect orig and donor inodes against a truncate */
 210 -       ret1 = mext_inode_double_lock(orig_inode, donor_inode);
 211 -       if (ret1 < 0)
 212 -               return ret1;
 213 +       mext_inode_double_lock(orig_inode, donor_inode);
 214
 215         /* Protect extent tree against block allocations via delalloc */
 216         double_down_write_data_sem(orig_inode, donor_inode);
 217         /* Check the filesystem environment whether move_extent can be done */
 218 -       ret1 = mext_check_arguments(orig_inode, donor_inode, orig_start,
 219 +       ret = mext_check_arguments(orig_inode, donor_inode, orig_start,
 220                                     donor_start, &len);
 221 -       if (ret1)
 222 +       if (ret)
 223                 goto out;
 224
 225         file_end = (i_size_read(orig_inode) - 1) >> orig_inode->i_blkbits;
 226 @@ -1227,13 +1159,13 @@ ext4_move_extents(struct file *o_filp, s
 227         if (file_end < block_end)
 228                 len -= block_end - file_end;
 229
 230 -       ret1 = get_ext_path(orig_inode, block_start, &orig_path);
 231 -       if (ret1)
 232 +       ret = get_ext_path(orig_inode, block_start, &orig_path);
 233 +       if (ret)
 234                 goto out;
 235
 236         /* Get path structure to check the hole */
 237 -       ret1 = get_ext_path(orig_inode, block_start, &holecheck_path);
 238 -       if (ret1)
 239 +       ret = get_ext_path(orig_inode, block_start, &holecheck_path);
 240 +       if (ret)
 241                 goto out;
 242
 243         depth = ext_depth(orig_inode);
 244 @@ -1252,13 +1184,13 @@ ext4_move_extents(struct file *o_filp, s
 245                 last_extent = mext_next_extent(orig_inode,
 246                                         holecheck_path, &ext_cur);
 247                 if (last_extent < 0) {
 248 -                       ret1 = last_extent;
 249 +                       ret = last_extent;
 250                         goto out;
 251                 }
 252                 last_extent = mext_next_extent(orig_inode, orig_path,
 253                                                         &ext_dummy);
 254                 if (last_extent < 0) {
 255 -                       ret1 = last_extent;
 256 +                       ret = last_extent;
 257                         goto out;
 258                 }
 259                 seq_start = le32_to_cpu(ext_cur->ee_block);
 260 @@ -1272,7 +1204,7 @@ ext4_move_extents(struct file *o_filp, s
 261         if (le32_to_cpu(ext_cur->ee_block) > block_end) {
 262                 ext4_debug("ext4 move extent: The specified range of file "
 263                                                         "may be the hole\n");
 264 -               ret1 = -EINVAL;
 265 +               ret = -EINVAL;
 266                 goto out;
 267         }
 268
 269 @@ -1292,7 +1224,7 @@ ext4_move_extents(struct file *o_filp, s
 270                 last_extent = mext_next_extent(orig_inode, holecheck_path,
 271                                                 &ext_cur);
 272                 if (last_extent < 0) {
 273 -                       ret1 = last_extent;
 274 +                       ret = last_extent;
 275                         break;
 276                 }
 277                 add_blocks = ext4_ext_get_actual_len(ext_cur);
 278 @@ -1349,18 +1281,18 @@ ext4_move_extents(struct file *o_filp, s
 279                                                 orig_page_offset,
 280                                                 data_offset_in_page,
 281                                                 block_len_in_page, uninit,
 282 -                                               &ret1);
 283 +                                               &ret);
 284
 285                         /* Count how many blocks we have exchanged */
 286                         *moved_len += block_len_in_page;
 287 -                       if (ret1 < 0)
 288 +                       if (ret < 0)
 289                                 break;
 290                         if (*moved_len > len) {
 291                                 EXT4_ERROR_INODE(orig_inode,
 292                                         "We replaced blocks too much! "
 293                                         "sum of replaced: %llu requested: %llu",
 294                                         *moved_len, len);
 295 -                               ret1 = -EIO;
 296 +                               ret = -EIO;
 297                                 break;
 298                         }
 299
 300 @@ -1374,22 +1306,22 @@ ext4_move_extents(struct file *o_filp, s
 301                 }
 302
 303                 double_down_write_data_sem(orig_inode, donor_inode);
 304 -               if (ret1 < 0)
 305 +               if (ret < 0)
 306                         break;
 307
 308                 /* Decrease buffer counter */
 309                 if (holecheck_path)
 310                         ext4_ext_drop_refs(holecheck_path);
 311 -               ret1 = get_ext_path(orig_inode, seq_start, &holecheck_path);
 312 -               if (ret1)
 313 +               ret = get_ext_path(orig_inode, seq_start, &holecheck_path);
 314 +               if (ret)
 315                         break;
 316                 depth = holecheck_path->p_depth;
 317
 318                 /* Decrease buffer counter */
 319                 if (orig_path)
 320                         ext4_ext_drop_refs(orig_path);
 321 -               ret1 = get_ext_path(orig_inode, seq_start, &orig_path);
 322 -               if (ret1)
 323 +               ret = get_ext_path(orig_inode, seq_start, &orig_path);
 324 +               if (ret)
 325                         break;
 326
 327                 ext_cur = holecheck_path[depth].p_ext;
 328 @@ -1412,12 +1344,7 @@ out:
 329                 kfree(holecheck_path);
 330         }
 331         double_up_write_data_sem(orig_inode, donor_inode);
 332 -       ret2 = mext_inode_double_unlock(orig_inode, donor_inode);
 333 +       mext_inode_double_unlock(orig_inode, donor_inode);
 334
 335 -       if (ret1)
 336 -               return ret1;
 337 -       else if (ret2)
 338 -               return ret2;
 339 -
 340 -       return 0;
 341 +       return ret;
 342  }