ext4: dio overwrite nolock
v3 cover ext4: dio overwrite nolock
Hello list,
Here is the third version of dio overwrite nolock. In this version, I rework some codes according to Robin's reply.
This patch set can improve the performance of ext4 when the user does a dio overwrite because, when a dio overwrite occurs, we don't need to take i_mutex lock in some conditons. The condition includes the size of file doesn't be changed, and align aio. So dio write can be parallelized in these conditions.
In patch 1, ext4_file_dio_write is defined to split buffered I/O and direct I/O in ext4_file_write so that some codes can be added to check whether we can do a dio overwrite without i_mutex lock later.
In patch 2, a new flag called EXT4_GET_BLOCKS_NOLOCK and a new get_block function that is named ext4_get_block_write_nolock are defined to do a lookup to let me know whether the extent of the file at this offset has been initialized because we need to know whether a dio overwrite needs to modify the metadata of the file or not.
In patch 3, we implement dio overwrite nolock. In ext4_file_dio_write, we check whether we can do a dio overwrite without lock. Then we use 'iocb->private' to store this flag to tell ext4_ext_direct_IO to handle it because file_update_time will start a new journal and it will cause a deadlock. So we need to finish to update file time with i_mutex lock, and release lock in ext4_ext_direct_IO.
v3 <- v2:
* change the condition check of dio overwrite nolock in ext4_file_dio_write
* rename 'create' variable to 'flags' in ext4_get_block_write_nolock
* remove a duplicated if statement in ext4_ext_direct_IO
v2 <- v1:
* rebase to 3.5
* rework ext4_file_dio_write to avoid to copy vfs's code back into ext4
* add some comments to explain how to determine whether we can do a nolocking overwrite dio
Any comments are welcomed. Thanks.
Regards,
Zheng
Zheng Liu (3):
ext4: split ext4_file_write into buffered IO and direct IO
ext4: add a new flag for ext4_map_blocks
ext4: add dio overwrite nolock
fs/ext4/ext4.h | 2 + fs/ext4/file.c | 107 +++++++++++++++++++++++++++++++++++++++++++----------- fs/ext4/inode.c | 76 ++++++++++++++++++++++++++++++++++----- 3 files changed, 153 insertions(+), 32 deletions(-)
PATCH v3 1/3 ext4: split ext4_file_write into buffered IO and direct IO
From: Zheng Liu <[email protected]>
ext4_file_dio_write is defined in order to split buffered IO and direct IO in ext4. This patch just refactor some stuff in write path.
--- fs/ext4/file.c | 60 +++++++++++++++++++++++++++++++++++-------------------- 1 files changed, 38 insertions(+), 22 deletions(-)
fs/ext4/file.c +38 -22
@@ -90,34 +90,16 @@ ext4_unaligned_aio(struct inode *inode, const struct iovec *iov, }static ssize_t -ext4_file_write (struct kiocb *iocb,const struct iovec *iov, -unsigned long nr_segs,loff_t pos) +ext4_file_dio_write (struct kiocb *iocb,const struct iovec *iov, +unsigned long nr_segs,loff_t pos) {struct inode *inode = iocb->ki_filp->f_path.dentry->d_inode;int unaligned_aio =0 ;ssize_t ret; -/* -* If we have encountered a bitmap-format file, the size limit -* is smaller than s_maxbytes, which is for extent-mapped files. -*/ - -if (!(ext4_test_inode_flag (inode,EXT4_INODE_EXTENTS ))) { -struct ext4_sb_info *sbi =EXT4_SB (inode->i_sb); -size_t length =iov_length (iov, nr_segs); - -if ((pos > sbi->s_bitmap_maxbytes || - (pos == sbi->s_bitmap_maxbytes && length >0 ))) -return -EFBIG ; - -if (pos + length > sbi->s_bitmap_maxbytes) { - nr_segs =iov_shorten ((struct iovec *)iov, nr_segs, - sbi->s_bitmap_maxbytes - pos); - } - }else if (unlikely ((iocb->ki_filp->f_flags &O_DIRECT ) && - !is_sync_kiocb (iocb))) { +if (ext4_test_inode_flag (inode,EXT4_INODE_EXTENTS ) && + !is_sync_kiocb (iocb)) unaligned_aio =ext4_unaligned_aio (inode, iov, nr_segs, pos); - }/* Unaligned direct AIO must be serialized; see comment above */ if (unaligned_aio) {
@@ -141,6 +123,40 @@ ext4_file_write(struct kiocb *iocb, const struct iovec *iov,return ret; } +static ssize_t +ext4_file_write (struct kiocb *iocb,const struct iovec *iov, +unsigned long nr_segs,loff_t pos) +{ +struct inode *inode = iocb->ki_filp->f_path.dentry->d_inode; +ssize_t ret; + +/* +* If we have encountered a bitmap-format file, the size limit +* is smaller than s_maxbytes, which is for extent-mapped files. +*/ + +if (!(ext4_test_inode_flag (inode,EXT4_INODE_EXTENTS ))) { +struct ext4_sb_info *sbi =EXT4_SB (inode->i_sb); +size_t length =iov_length (iov, nr_segs); + +if ((pos > sbi->s_bitmap_maxbytes || + (pos == sbi->s_bitmap_maxbytes && length >0 ))) +return -EFBIG ; + +if (pos + length > sbi->s_bitmap_maxbytes) { + nr_segs =iov_shorten ((struct iovec *)iov, nr_segs, + sbi->s_bitmap_maxbytes - pos); + } + } + +if (unlikely (iocb->ki_filp->f_flags &O_DIRECT )) + ret =ext4_file_dio_write (iocb, iov, nr_segs, pos); +else + ret =generic_file_aio_write (iocb, iov, nr_segs, pos); + +return ret; +} +static const struct vm_operations_struct ext4_file_vm_ops = { .fault = filemap_fault, .page_mkwrite = ext4_page_mkwrite,
Signature · 1 line
1.7.4.1
Re: [PATCH 1/3 v3] ext4: split ext4_file_write into buffered IO and direct IO
Quoted 9 lines · Zheng Liu
From: Zheng Liu <[email protected]>
ext4_file_dio_writeis defined in order to split buffered IO and direct IO in ext4. This patch just refactor some stuff in write path.CC Tao Ma <[email protected]>CC Eric Sandeen <[email protected]>CC Robin Dong <[email protected]>Signed-off-by Zheng Liu <[email protected]>
Applied, thanks.
-Ted
PATCH v3 2/3 ext4: add a new flag for ext4_map_blocks
In reply to Zheng LiuFrom: Zheng Liu <[email protected]>
EXT4_GET_BLOCKS_NO_LOCK flag is added to indicate that we don't need to acquire i_data_sem lock in ext4_map_blocks. Meanwhile, it lets _ext4_get_block do not start a new journal because when we do a overwrite dio, there is no any metadata that needs to be modified.
We define a new function called ext4_get_block_write_nolock, which is used in dio overwrite nolock. In this function, it doesn't try to acquire i_data_sem lock and doesn't start a new journal as it does a lookup.
--- fs/ext4/ext4.h | 2 + fs/ext4/inode.c | 59 +++++++++++++++++++++++++++++++++++++++++++++--------- 2 files changed, 51 insertions(+), 10 deletions(-)
fs/ext4/ext4.h +2 -0
@@ -571,6 +571,8 @@ enum {#define EXT4_GET_BLOCKS_NO_NORMALIZE 0x0040/* Request will not result in inode size update (user for fallocate) */ #define EXT4_GET_BLOCKS_KEEP_SIZE 0x0080 +/* Do not take i_data_sem locking in ext4_map_blocks */ +#define EXT4_GET_BLOCKS_NO_LOCK 0x0100/* * Flags used by ext4_free_blocks
fs/ext4/inode.c +49 -10
95 lines not on this page · raw message
Signature · 1 line
1.7.4.1
Re: [PATCH 2/3 v3] ext4: add a new flag for ext4_map_blocks
Quoted 7 lines · Zheng Liu
From: Zheng Liu <[email protected]>
EXT4_GET_BLOCKS_NO_LOCKflag is added to indicate that we don't need to acquirei_data_semlock inext4_map_blocks. Meanwhile, it lets_ext4_get_blockdo not start a new journal because when we do a overwrite dio, there is no any metadata that needs to be modified.
Quoted 8 lines
We define a new function called
ext4_get_block_write_nolock, which is used in dio overwrite nolock. In this function, it doesn't try to acquirei_data_semlock and doesn't start a new journal as it does a lookup.CC Tao Ma <[email protected]>CC Eric Sandeen <[email protected]>CC Robin Dong <[email protected]>Signed-off-by Zheng Liu <[email protected]>
Thanks, applied.
- Ted
PATCH v3 3/3 ext4: add dio overwrite nolock
In reply to Zheng LiuFrom: Zheng Liu <[email protected]>
Aligned and overwrite direct I/O can be parallelized. In ext4_file_dio_write, we first check whether these conditions are satisfied or not. If so, we take i_data_sem and release i_mutex lock directly. Meanwhile iocb->private is set to indicate that this is a dio overwrite, and it will be handled in ext4_ext_direct_IO.
--- fs/ext4/file.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++-- fs/ext4/inode.c | 17 +++++++++++++++++ 2 files changed, 66 insertions(+), 2 deletions(-)
fs/ext4/file.c +49 -2
@@ -93,9 +93,13 @@ static ssize_t ext4_file_dio_write(struct kiocb *iocb, const struct iovec *iov, unsigned long nr_segs, loff_t pos) { - struct inode *inode = iocb->ki_filp->f_path.dentry->d_inode; + struct file *file = iocb->ki_filp; +struct inode *inode = file->f_mapping->host; +struct blk_plug plug;int unaligned_aio =0 ;ssize_t ret; +int overwrite =0 ; +size_t length =iov_length (iov, nr_segs);if (ext4_test_inode_flag (inode,EXT4_INODE_EXTENTS ) && !is_sync_kiocb (iocb))
@@ -115,7 +119,50 @@ ext4_file_dio_write(struct kiocb *iocb, const struct iovec *iov,ext4_aiodio_wait (inode); } - ret =generic_file_aio_write (iocb, iov, nr_segs, pos); +BUG_ON (iocb->ki_pos != pos); + +mutex_lock (&inode->i_mutex); +blk_start_plug (&plug); + + iocb->private = &overwrite; + +/* check whether we do a DIO overwrite or not */ +if (ext4_should_dioread_nolock (inode) && !unaligned_aio && + !file->f_mapping->nrpages && pos + length <=i_size_read (inode)) { +struct ext4_map_blocks map; +unsigned int blkbits = inode->i_blkbits; +int err, len; + + map.m_lblk = pos >> blkbits; + map.m_len = (EXT4_BLOCK_ALIGN (pos + length, blkbits) >> blkbits) + - map.m_lblk; + len = map.m_len; + + err =ext4_map_blocks (NULL , inode, &map,0 ); +/* +* 'err==len' means that all of blocks has been preallocated no +* matter they are initialized or not. For excluding +* uninitialized extents, we need to check m_flags. There are +* two conditions that indicate for initialized extents. +* 1) If we hit extent cache, EXT4_MAP_MAPPED flag is returned; +* 2) If we do a real lookup, non-flags are returned. +* So we should check these two conditions. +*/ +if (err == len && (map.m_flags &EXT4_MAP_MAPPED )) + overwrite =1 ; + } + + ret =__generic_file_aio_write (iocb, iov, nr_segs, &iocb->ki_pos); +mutex_unlock (&inode->i_mutex); + +if (ret >0 || ret == -EIOCBQUEUED ) { +ssize_t err; + + err =generic_write_sync (file, pos, ret); +if (err <0 && ret >0 ) + ret = err; + } +blk_finish_plug (&plug);if (unaligned_aio)mutex_unlock (ext4_aio_mutex (inode));
fs/ext4/inode.c +17 -0
@@ -2996,6 +2996,16 @@ static ssize_t ext4_ext_direct_IO(int rw, struct kiocb *iocb,if (rw ==WRITE && final_size <= inode->i_size) {int overwrite =0 ; +BUG_ON (iocb->private ==NULL ); + +/* If we do a overwrite dio, i_mutex locking can be released */ + overwrite = *((int *)iocb->private); + +if (overwrite) { +down_read (&EXT4_I (inode)->i_data_sem); +mutex_unlock (&inode->i_mutex); + } +/* * We could direct write to holes and fallocate. *
@@ -3083,6 +3093,13 @@ static ssize_t ext4_ext_direct_IO(int rw, struct kiocb *iocb, ret = err;ext4_clear_inode_state (inode,EXT4_STATE_DIO_UNWRITTEN ); } + +/* take i_mutex locking again if we do a ovewrite dio */ +if (overwrite) { +up_read (&EXT4_I (inode)->i_data_sem); +mutex_lock (&inode->i_mutex); + } +return ret; }
Signature · 1 line
1.7.4.1
Re: [PATCH 3/3 v3] ext4: add dio overwrite nolock
Quoted 8 lines · Zheng Liu
From: Zheng Liu <[email protected]>
Aligned and overwrite direct I/O can be parallelized. In
ext4_file_dio_write, we first check whether these conditions are satisfied or not. If so, we takei_data_semand releasei_mutexlock directly. Meanwhileiocb->privateis set to indicate that this is a dio overwrite, and it will be handled inext4_ext_direct_IO.
Quoted 4 lines
CC Tao Ma <[email protected]>CC Eric Sandeen <[email protected]>CC Robin Dong <[email protected]>Signed-off-by Zheng Liu <[email protected]>
Thanks, applied.
- Ted