History

ext4: get rid of EXT4_GET_BLOCKS_NO_LOCK flag

When dioread_nolock mode is enabled, we grab i_data_sem in ext4_ext_direct_IO() and therefore we need to instruct _ext4_get_block() not to grab i_data_sem again using EXT4_GET_BLOCKS_NO_LOCK. However holding i_data_sem over overwrite direct IO isn't needed these days. We have exclusion against truncate / hole punching because we increase i_dio_count under i_mutex in ext4_ext_direct_IO() so once ext4_file_write_iter() verifies blocks are allocated & written, they are guaranteed to stay so during the whole direct IO even after we drop i_mutex. So we can just remove this locking abuse and the no longer necessary EXT4_GET_BLOCKS_NO_LOCK flag.

Jan Kara committed 10 years ago

Commit 2dcba471 parent e74031f3 files · +22 −28Browse files

Unified · Split

3 files
  1. fs/ext4/ext4.h +1 −3
  2. fs/ext4/inode.c +20 −23
  3. include/trace/events/ext4.h +1 −2
fs/ext4/ext4.h+1−3
@@ -555,10 +555,8 @@ enum { · 554 unmodified lines
555#define EXT4_GET_BLOCKS_NO_NORMALIZE 0x0040555#define EXT4_GET_BLOCKS_NO_NORMALIZE 0x0040
556 /* Request will not result in inode size update (user for fallocate) */556 /* Request will not result in inode size update (user for fallocate) */
557#define EXT4_GET_BLOCKS_KEEP_SIZE 0x0080557#define EXT4_GET_BLOCKS_KEEP_SIZE 0x0080
558 /* Do not take i_data_sem locking in ext4_map_blocks */
559#define EXT4_GET_BLOCKS_NO_LOCK 0x0100
560 /* Convert written extents to unwritten */558 /* Convert written extents to unwritten */
561#define EXT4_GET_BLOCKS_CONVERT_UNWRITTEN 0x0200559#define EXT4_GET_BLOCKS_CONVERT_UNWRITTEN 0x0100
562 560
563/*561/*
564 * The bit position of these flags must not overlap with any of the562 * The bit position of these flags must not overlap with any of the
fs/ext4/inode.c+20−23
@@ -403,8 +403,7 @@ static void ext4_map_blocks_es_recheck(handle_t *handle, · 402 unmodified lines
403 * out taking i_data_sem. So at the time the unwritten extent403 * out taking i_data_sem. So at the time the unwritten extent
404 * could be converted.404 * could be converted.
405 */405 */
406 if (!(flags & EXT4_GET_BLOCKS_NO_LOCK))406 down_read(&EXT4_I(inode)->i_data_sem);
407 down_read(&EXT4_I(inode)->i_data_sem);
408 if (ext4_test_inode_flag(inode, EXT4_INODE_EXTENTS)) {407 if (ext4_test_inode_flag(inode, EXT4_INODE_EXTENTS)) {
409 retval = ext4_ext_map_blocks(handle, inode, map, flags &408 retval = ext4_ext_map_blocks(handle, inode, map, flags &
410 EXT4_GET_BLOCKS_KEEP_SIZE);409 EXT4_GET_BLOCKS_KEEP_SIZE);
@@ -412,8 +411,7 @@ static void ext4_map_blocks_es_recheck(handle_t *handle, · 1 unmodified line
412 retval = ext4_ind_map_blocks(handle, inode, map, flags &411 retval = ext4_ind_map_blocks(handle, inode, map, flags &
413 EXT4_GET_BLOCKS_KEEP_SIZE);412 EXT4_GET_BLOCKS_KEEP_SIZE);
414 }413 }
415 if (!(flags & EXT4_GET_BLOCKS_NO_LOCK))414 up_read((&EXT4_I(inode)->i_data_sem));
416 up_read((&EXT4_I(inode)->i_data_sem));
417 415
418 /*416 /*
419 * We don't check m_len because extent will be collpased in status417 * We don't check m_len because extent will be collpased in status
@@ -509,8 +507,7 @@ int ext4_map_blocks(handle_t *handle, struct inode *inode, · 89 unmodified lines
509 * Try to see if we can get the block without requesting a new507 * Try to see if we can get the block without requesting a new
510 * file system block.508 * file system block.
511 */509 */
512 if (!(flags & EXT4_GET_BLOCKS_NO_LOCK))510 down_read(&EXT4_I(inode)->i_data_sem);
513 down_read(&EXT4_I(inode)->i_data_sem);
514 if (ext4_test_inode_flag(inode, EXT4_INODE_EXTENTS)) {511 if (ext4_test_inode_flag(inode, EXT4_INODE_EXTENTS)) {
515 retval = ext4_ext_map_blocks(handle, inode, map, flags &512 retval = ext4_ext_map_blocks(handle, inode, map, flags &
516 EXT4_GET_BLOCKS_KEEP_SIZE);513 EXT4_GET_BLOCKS_KEEP_SIZE);
@@ -541,8 +538,7 @@ int ext4_map_blocks(handle_t *handle, struct inode *inode, · 24 unmodified lines
541 if (ret < 0)538 if (ret < 0)
542 retval = ret;539 retval = ret;
543 }540 }
544 if (!(flags & EXT4_GET_BLOCKS_NO_LOCK))541 up_read((&EXT4_I(inode)->i_data_sem));
545 up_read((&EXT4_I(inode)->i_data_sem));
546 542
547found:543found:
548 if (retval > 0 && map->m_flags & EXT4_MAP_MAPPED) {544 if (retval > 0 && map->m_flags & EXT4_MAP_MAPPED) {
@@ -674,7 +670,7 @@ static int _ext4_get_block(struct inode *inode, sector_t iblock, · 125 unmodified lines
674 map.m_lblk = iblock;670 map.m_lblk = iblock;
675 map.m_len = bh->b_size >> inode->i_blkbits;671 map.m_len = bh->b_size >> inode->i_blkbits;
676 672
677 if (flags && !(flags & EXT4_GET_BLOCKS_NO_LOCK) && !handle) {673 if (flags && !handle) {
678 /* Direct IO write... */674 /* Direct IO write... */
679 if (map.m_len > DIO_MAX_BLOCKS)675 if (map.m_len > DIO_MAX_BLOCKS)
680 map.m_len = DIO_MAX_BLOCKS;676 map.m_len = DIO_MAX_BLOCKS;
@@ -879,9 +875,6 @@ int do_journal_get_write_access(handle_t *handle, · 198 unmodified lines
879 return ret;875 return ret;
880}876}
881 877
882static int ext4_get_block_write_nolock(struct inode *inode, sector_t iblock,
883 struct buffer_head *bh_result, int create);
884
885#ifdef CONFIG_EXT4_FS_ENCRYPTION878#ifdef CONFIG_EXT4_FS_ENCRYPTION
886static int ext4_block_write_begin(struct page *page, loff_t pos, unsigned len,879static int ext4_block_write_begin(struct page *page, loff_t pos, unsigned len,
887 get_block_t *get_block)880 get_block_t *get_block)
@@ -3054,13 +3047,21 @@ int ext4_get_block_write(struct inode *inode, sector_t iblock, · 2166 unmodified lines
3054 EXT4_GET_BLOCKS_IO_CREATE_EXT);3047 EXT4_GET_BLOCKS_IO_CREATE_EXT);
3055}3048}
3056 3049
3057static int ext4_get_block_write_nolock(struct inode *inode, sector_t iblock,3050static int ext4_get_block_overwrite(struct inode *inode, sector_t iblock,
3058 struct buffer_head *bh_result, int create)3051 struct buffer_head *bh_result, int create)
3059{3052{
3060 ext4_debug("ext4_get_block_write_nolock: inode %lu, create flag %d\n",3053 int ret;
3054
3055 ext4_debug("ext4_get_block_overwrite: inode %lu, create flag %d\n",
3061 inode->i_ino, create);3056 inode->i_ino, create);
3062 return _ext4_get_block(inode, iblock, bh_result,3057 ret = _ext4_get_block(inode, iblock, bh_result, 0);
3063 EXT4_GET_BLOCKS_NO_LOCK);3058 /*
3059 * Blocks should have been preallocated! ext4_file_write_iter() checks
3060 * that.
3061 */
3062 WARN_ON_ONCE(!buffer_mapped(bh_result));
3063
3064 return ret;
3064}3065}
3065 3066
3066int ext4_get_block_dax(struct inode *inode, sector_t iblock,3067int ext4_get_block_dax(struct inode *inode, sector_t iblock,
@@ -3143,10 +3144,8 @@ static ssize_t ext4_ext_direct_IO(struct kiocb *iocb, struct iov_iter *iter, · 76 unmodified lines
3143 /* If we do a overwrite dio, i_mutex locking can be released */3144 /* If we do a overwrite dio, i_mutex locking can be released */
3144 overwrite = *((int *)iocb->private);3145 overwrite = *((int *)iocb->private);
3145 3146
3146 if (overwrite) {3147 if (overwrite)
3147 down_read(&EXT4_I(inode)->i_data_sem);
3148 mutex_unlock(&inode->i_mutex);3148 mutex_unlock(&inode->i_mutex);
3149 }
3150 3149
3151 /*3150 /*
3152 * We could direct write to holes and fallocate.3151 * We could direct write to holes and fallocate.
@@ -3189,7 +3188,7 @@ static ssize_t ext4_ext_direct_IO(struct kiocb *iocb, struct iov_iter *iter, · 36 unmodified lines
3189 }3188 }
3190 3189
3191 if (overwrite) {3190 if (overwrite) {
3192 get_block_func = ext4_get_block_write_nolock;3191 get_block_func = ext4_get_block_overwrite;
3193 } else {3192 } else {
3194 get_block_func = ext4_get_block_write;3193 get_block_func = ext4_get_block_write;
3195 dio_flags = DIO_LOCKING;3194 dio_flags = DIO_LOCKING;
@@ -3245,10 +3244,8 @@ retake_lock: · 49 unmodified lines
3245 if (iov_iter_rw(iter) == WRITE)3244 if (iov_iter_rw(iter) == WRITE)
3246 inode_dio_end(inode);3245 inode_dio_end(inode);
3247 /* take i_mutex locking again if we do a ovewrite dio */3246 /* take i_mutex locking again if we do a ovewrite dio */
3248 if (overwrite) {3247 if (overwrite)
3249 up_read(&EXT4_I(inode)->i_data_sem);
3250 mutex_lock(&inode->i_mutex);3248 mutex_lock(&inode->i_mutex);
3251 }
3252 3249
3253 return ret;3250 return ret;
3254}3251}
include/trace/events/ext4.h+1−2
@@ -42,8 +42,7 @@ struct extent_status; · 41 unmodified lines
42 { EXT4_GET_BLOCKS_CONVERT, "CONVERT" }, \42 { EXT4_GET_BLOCKS_CONVERT, "CONVERT" }, \
43 { EXT4_GET_BLOCKS_METADATA_NOFAIL, "METADATA_NOFAIL" }, \43 { EXT4_GET_BLOCKS_METADATA_NOFAIL, "METADATA_NOFAIL" }, \
44 { EXT4_GET_BLOCKS_NO_NORMALIZE, "NO_NORMALIZE" }, \44 { EXT4_GET_BLOCKS_NO_NORMALIZE, "NO_NORMALIZE" }, \
45 { EXT4_GET_BLOCKS_KEEP_SIZE, "KEEP_SIZE" }, \45 { EXT4_GET_BLOCKS_KEEP_SIZE, "KEEP_SIZE" })
46 { EXT4_GET_BLOCKS_NO_LOCK, "NO_LOCK" })
47 46
48#define show_mflags(flags) __print_flags(flags, "", \47#define show_mflags(flags) __print_flags(flags, "", \
49 { EXT4_MAP_NEW, "N" }, \48 { EXT4_MAP_NEW, "N" }, \