Skip to contentSkip to conversation

ext4: dio overwrite nolock

PATCH Applied Zheng Liu 2 participants 7 messages Jul 1, 2012 – Jul 9, 2012 ext4 file system

lore
Share & tools
lore

Zheng Liu

author
#0 lore raw

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(-)
author
#1 lore raw

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.

CC Tao Ma <[email protected]>
CC Eric Sandeen <[email protected]>
CC Robin Dong <[email protected]>
---
 fs/ext4/file.c |   60 +++++++++++++++++++++++++++++++++++--------------------
 1 files changed, 38 insertions(+), 22 deletions(-)
fs/ext4/file.c +38 -22
index 8c7642a..a10dc77 100644
@@ -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
maintainer Applied
#2 lore raw

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_write is 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]>

Applied, thanks.

-Ted

author
#3 lore raw

PATCH v3 2/3 ext4: add a new flag for ext4_map_blocks

In reply to Zheng Liu

From: 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.

CC Tao Ma <[email protected]>
CC Eric Sandeen <[email protected]>
CC Robin Dong <[email protected]>
---
 fs/ext4/ext4.h  |    2 +
 fs/ext4/inode.c |   59 +++++++++++++++++++++++++++++++++++++++++++++---------
 2 files changed, 51 insertions(+), 10 deletions(-)
fs/ext4/ext4.h +2 -0
index cfc4e01..d1a2b1e 100644
@@ -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
maintainer Applied
#4 lore raw

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_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.

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 acquire i_data_sem lock 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]>

Thanks, applied.

- Ted

author
#5 lore raw

PATCH v3 3/3 ext4: add dio overwrite nolock

In reply to 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 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.

CC Tao Ma <[email protected]>
CC Eric Sandeen <[email protected]>
CC Robin Dong <[email protected]>
---
 fs/ext4/file.c  |   51 +++++++++++++++++++++++++++++++++++++++++++++++++--
 fs/ext4/inode.c |   17 +++++++++++++++++
 2 files changed, 66 insertions(+), 2 deletions(-)
fs/ext4/file.c +49 -2
index a10dc77..1c81509 100644
@@ -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
index 76cb3b1..8c84070 100644
@@ -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
maintainer Applied
#6 lore raw

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 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.

Quoted 4 lines
CC Tao Ma <[email protected]>
CC Eric Sandeen <[email protected]>
CC Robin Dong <[email protected]>

Thanks, applied.

- Ted