get-rid-of-EXT4_GET_BLOCKS_NO_LOCK-flag

   1 ext4: get rid of EXT4_GET_BLOCKS_NO_LOCK flag
   2
   3 From: Jan Kara <jack@suse.com>
   4
   5 When dioread_nolock mode is enabled, we grab i_data_sem in
   6 ext4_ext_direct_IO() and therefore we need to instruct _ext4_get_block()
   7 not to grab i_data_sem again using EXT4_GET_BLOCKS_NO_LOCK. However
   8 holding i_data_sem over overwrite direct IO isn't needed these days. We
   9 have exclusion against truncate / hole punching because we increase
  10 i_dio_count under i_mutex in ext4_ext_direct_IO() so once
  11 ext4_file_write_iter() verifies blocks are allocated & written, they are
  12 guaranteed to stay so during the whole direct IO even after we drop
  13 i_mutex.
  14
  15 So we can just remove this locking abuse and the no longer necessary
  16 EXT4_GET_BLOCKS_NO_LOCK flag.
  17
  18 Signed-off-by: Jan Kara <jack@suse.com>
  19 Signed-off-by: Theodore Ts'o <tytso@mit.edu>
  20 ---
  21  fs/ext4/ext4.h              |  4 +---
  22  fs/ext4/inode.c             | 43 ++++++++++++++++++++-----------------------
  23  include/trace/events/ext4.h |  3 +--
  24  3 files changed, 22 insertions(+), 28 deletions(-)
  25
  26 diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
  27 index c86546efec30..142e935d1f3d 100644
  28 --- a/fs/ext4/ext4.h
  29 +++ b/fs/ext4/ext4.h
  30 @@ -551,10 +551,8 @@ enum {
  31  #define EXT4_GET_BLOCKS_NO_NORMALIZE           0x0040
  32         /* Request will not result in inode size update (user for fallocate) */
  33  #define EXT4_GET_BLOCKS_KEEP_SIZE              0x0080
  34 -       /* Do not take i_data_sem locking in ext4_map_blocks */
  35 -#define EXT4_GET_BLOCKS_NO_LOCK                        0x0100
  36         /* Convert written extents to unwritten */
  37 -#define EXT4_GET_BLOCKS_CONVERT_UNWRITTEN      0x0200
  38 +#define EXT4_GET_BLOCKS_CONVERT_UNWRITTEN      0x0100
  39
  40  /*
  41   * The bit position of these flags must not overlap with any of the
  42 diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
  43 index f72212ef1fee..39da009cbc89 100644
  44 --- a/fs/ext4/inode.c
  45 +++ b/fs/ext4/inode.c
  46 @@ -403,8 +403,7 @@ static void ext4_map_blocks_es_recheck(handle_t *handle,
  47          * out taking i_data_sem.  So at the time the unwritten extent
  48          * could be converted.
  49          */
  50 -       if (!(flags & EXT4_GET_BLOCKS_NO_LOCK))
  51 -               down_read(&EXT4_I(inode)->i_data_sem);
  52 +       down_read(&EXT4_I(inode)->i_data_sem);
  53         if (ext4_test_inode_flag(inode, EXT4_INODE_EXTENTS)) {
  54                 retval = ext4_ext_map_blocks(handle, inode, map, flags &
  55                                              EXT4_GET_BLOCKS_KEEP_SIZE);
  56 @@ -412,8 +411,7 @@ static void ext4_map_blocks_es_recheck(handle_t *handle,
  57                 retval = ext4_ind_map_blocks(handle, inode, map, flags &
  58                                              EXT4_GET_BLOCKS_KEEP_SIZE);
  59         }
  60 -       if (!(flags & EXT4_GET_BLOCKS_NO_LOCK))
  61 -               up_read((&EXT4_I(inode)->i_data_sem));
  62 +       up_read((&EXT4_I(inode)->i_data_sem));
  63
  64         /*
  65          * We don't check m_len because extent will be collpased in status
  66 @@ -509,8 +507,7 @@ int ext4_map_blocks(handle_t *handle, struct inode *inode,
  67          * Try to see if we can get the block without requesting a new
  68          * file system block.
  69          */
  70 -       if (!(flags & EXT4_GET_BLOCKS_NO_LOCK))
  71 -               down_read(&EXT4_I(inode)->i_data_sem);
  72 +       down_read(&EXT4_I(inode)->i_data_sem);
  73         if (ext4_test_inode_flag(inode, EXT4_INODE_EXTENTS)) {
  74                 retval = ext4_ext_map_blocks(handle, inode, map, flags &
  75                                              EXT4_GET_BLOCKS_KEEP_SIZE);
  76 @@ -541,8 +538,7 @@ int ext4_map_blocks(handle_t *handle, struct inode *inode,
  77                 if (ret < 0)
  78                         retval = ret;
  79         }
  80 -       if (!(flags & EXT4_GET_BLOCKS_NO_LOCK))
  81 -               up_read((&EXT4_I(inode)->i_data_sem));
  82 +       up_read((&EXT4_I(inode)->i_data_sem));
  83
  84  found:
  85         if (retval > 0 && map->m_flags & EXT4_MAP_MAPPED) {
  86 @@ -674,7 +670,7 @@ static int _ext4_get_block(struct inode *inode, sector_t iblock,
  87         map.m_lblk = iblock;
  88         map.m_len = bh->b_size >> inode->i_blkbits;
  89
  90 -       if (flags && !(flags & EXT4_GET_BLOCKS_NO_LOCK) && !handle) {
  91 +       if (flags && !handle) {
  92                 /* Direct IO write... */
  93                 if (map.m_len > DIO_MAX_BLOCKS)
  94                         map.m_len = DIO_MAX_BLOCKS;
  95 @@ -879,9 +875,6 @@ int do_journal_get_write_access(handle_t *handle,
  96         return ret;
  97  }
  98
  99 -static int ext4_get_block_write_nolock(struct inode *inode, sector_t iblock,
 100 -                  struct buffer_head *bh_result, int create);
 101 -
 102  #ifdef CONFIG_EXT4_FS_ENCRYPTION
 103  static int ext4_block_write_begin(struct page *page, loff_t pos, unsigned len,
 104                                   get_block_t *get_block)
 105 @@ -3012,13 +3005,21 @@ int ext4_get_block_write(struct inode *inode, sector_t iblock,
 106                                EXT4_GET_BLOCKS_IO_CREATE_EXT);
 107  }
 108
 109 -static int ext4_get_block_write_nolock(struct inode *inode, sector_t iblock,
 110 +static int ext4_get_block_overwrite(struct inode *inode, sector_t iblock,
 111                    struct buffer_head *bh_result, int create)
 112  {
 113 -       ext4_debug("ext4_get_block_write_nolock: inode %lu, create flag %d\n",
 114 +       int ret;
 115 +
 116 +       ext4_debug("ext4_get_block_overwrite: inode %lu, create flag %d\n",
 117                    inode->i_ino, create);
 118 -       return _ext4_get_block(inode, iblock, bh_result,
 119 -                              EXT4_GET_BLOCKS_NO_LOCK);
 120 +       ret = _ext4_get_block(inode, iblock, bh_result, 0);
 121 +       /*
 122 +        * Blocks should have been preallocated! ext4_file_write_iter() checks
 123 +        * that.
 124 +        */
 125 +       WARN_ON_ONCE(ret == 0);
 126 +
 127 +       return ret;
 128  }
 129
 130  int ext4_get_block_dax(struct inode *inode, sector_t iblock,
 131 @@ -3101,10 +3102,8 @@ static ssize_t ext4_ext_direct_IO(struct kiocb *iocb, struct iov_iter *iter,
 132         /* If we do a overwrite dio, i_mutex locking can be released */
 133         overwrite = *((int *)iocb->private);
 134
 135 -       if (overwrite) {
 136 -               down_read(&EXT4_I(inode)->i_data_sem);
 137 +       if (overwrite)
 138                 mutex_unlock(&inode->i_mutex);
 139 -       }
 140
 141         /*
 142          * We could direct write to holes and fallocate.
 143 @@ -3147,7 +3146,7 @@ static ssize_t ext4_ext_direct_IO(struct kiocb *iocb, struct iov_iter *iter,
 144         }
 145
 146         if (overwrite) {
 147 -               get_block_func = ext4_get_block_write_nolock;
 148 +               get_block_func = ext4_get_block_overwrite;
 149         } else {
 150                 get_block_func = ext4_get_block_write;
 151                 dio_flags = DIO_LOCKING;
 152 @@ -3203,10 +3202,8 @@ retake_lock:
 153         if (iov_iter_rw(iter) == WRITE)
 154                 inode_dio_end(inode);
 155         /* take i_mutex locking again if we do a ovewrite dio */
 156 -       if (overwrite) {
 157 -               up_read(&EXT4_I(inode)->i_data_sem);
 158 +       if (overwrite)
 159                 mutex_lock(&inode->i_mutex);
 160 -       }
 161
 162         return ret;
 163  }
 164 diff --git a/include/trace/events/ext4.h b/include/trace/events/ext4.h
 165 index 594b4b29a224..5f2ace56efc5 100644
 166 --- a/include/trace/events/ext4.h
 167 +++ b/include/trace/events/ext4.h
 168 @@ -42,8 +42,7 @@ struct extent_status;
 169         { EXT4_GET_BLOCKS_CONVERT,              "CONVERT" },            \
 170         { EXT4_GET_BLOCKS_METADATA_NOFAIL,      "METADATA_NOFAIL" },    \
 171         { EXT4_GET_BLOCKS_NO_NORMALIZE,         "NO_NORMALIZE" },       \
 172 -       { EXT4_GET_BLOCKS_KEEP_SIZE,            "KEEP_SIZE" },          \
 173 -       { EXT4_GET_BLOCKS_NO_LOCK,              "NO_LOCK" })
 174 +       { EXT4_GET_BLOCKS_KEEP_SIZE,            "KEEP_SIZE" })
 175
 176  #define show_mflags(flags) __print_flags(flags, "",    \
 177         { EXT4_MAP_NEW,         "N" },                  \
 178 --
 179 2.1.4
 180
 181