Linux EXT4 FS development
 help / color / mirror / Atom feed
From: Liu Song <liusong@linux.alibaba.com>
To: tytso@mit.edu, adilger.kernel@dilger.ca
Cc: linux-ext4@vger.kernel.org, linux-kernel@vger.kernel.org,
	liusong@linux.alibaba.com
Subject: [PATCH] ext4: do not mark inode dirty which is already dirtied under append-write scenario
Date: Tue, 11 Jul 2023 11:42:56 +0800	[thread overview]
Message-ID: <20230711034256.72651-1-liusong@linux.alibaba.com> (raw)

In the append-write scenario, after ensuring that the dirty inode can be
seen by the writeback process, there is no need to execute
"mark_inode_dirty" for every write. Instead, we can rely on
"ext4_mark_inode_dirty" executed when updating i_disksize in
"mpage_map_and_submit_extent" to ensure data consistency, which can
significantly improve performance in high-frequency append-write
scenarios.

In test scenarios of Kafka version 2.6.2, using packet size of 2K
resulted in a 10% performance improvement.

Signed-off-by: Liu Song <liusong@linux.alibaba.com>
---
 fs/ext4/inode.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 56 insertions(+), 2 deletions(-)

diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 9d9f414f99fe..d1aa775c9936 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -3128,6 +3128,57 @@ static int ext4_da_should_update_i_disksize(struct page *page,
 	return 1;
 }
 
+/*
+ * Copy from generic_write_end, add conditions to execute mark_inode_dirty
+ * to avoid additional overhead caused by frequent dirty inode operations
+ */
+static int ext4_da_generic_write_end(struct file *file, struct address_space *mapping,
+			loff_t pos, unsigned len, unsigned copied, bool need_dirty,
+			struct page *page, void *fsdata)
+{
+	struct inode *inode = mapping->host;
+	loff_t old_size = inode->i_size;
+	bool i_size_changed = false;
+	int was_dirty;
+
+	copied = block_write_end(file, mapping, pos, len, copied, page, fsdata);
+
+	/*
+	 * No need to use i_size_read() here, the i_size cannot change under us
+	 * because we hold i_rwsem.
+	 *
+	 * But it's important to update i_size while still holding page lock:
+	 * page writeout could otherwise come in and zero beyond i_size.
+	 */
+	if (pos + copied > inode->i_size) {
+		i_size_write(inode, pos + copied);
+		i_size_changed = true;
+	}
+
+	unlock_page(page);
+	put_page(page);
+
+	if (old_size < pos)
+		pagecache_isize_extended(inode, old_size, pos);
+
+	/*
+	 * In the append-write scenario, if the inode is marked as dirty,
+	 * it is ensured that the inode will be seen by the writeback process.
+	 * In the ext4_writepages process, when updating i_disksize,
+	 * corresponding metadata updates are also performed.
+	 * Therefore, it is unnecessary to repeatedly execute mark_inode_dirty
+	 * to improve performance.
+	 */
+	if (i_size_changed) {
+		spin_lock(&inode->i_lock);
+		was_dirty = inode->i_state & I_DIRTY;
+		spin_unlock(&inode->i_lock);
+		if (!was_dirty || need_dirty)
+			mark_inode_dirty(inode);
+	}
+	return copied;
+}
+
 static int ext4_da_write_end(struct file *file,
 			     struct address_space *mapping,
 			     loff_t pos, unsigned len, unsigned copied,
@@ -3137,6 +3188,7 @@ static int ext4_da_write_end(struct file *file,
 	loff_t new_i_size;
 	unsigned long start, end;
 	int write_mode = (int)(unsigned long)fsdata;
+	bool need_dirty = false;
 
 	if (write_mode == FALL_BACK_TO_NONDELALLOC)
 		return ext4_write_end(file, mapping, pos,
@@ -3169,10 +3221,12 @@ static int ext4_da_write_end(struct file *file,
 	 */
 	new_i_size = pos + copied;
 	if (copied && new_i_size > inode->i_size &&
-	    ext4_da_should_update_i_disksize(page, end))
+	    ext4_da_should_update_i_disksize(page, end)) {
 		ext4_update_i_disksize(inode, new_i_size);
+		need_dirty = true;
+	}
 
-	return generic_write_end(file, mapping, pos, len, copied, page, fsdata);
+	return ext4_da_generic_write_end(file, mapping, pos, len, copied, need_dirty, page, fsdata);
 }
 
 /*
-- 
2.19.1.6.gb485710b


             reply	other threads:[~2023-07-11  3:43 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-07-11  3:42 Liu Song [this message]
2023-08-06  3:12 ` [PATCH] ext4: do not mark inode dirty which is already dirtied under append-write scenario Liu Song
2023-08-09 12:25 ` Jan Kara
2023-08-10  6:59   ` Liu Song

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230711034256.72651-1-liusong@linux.alibaba.com \
    --to=liusong@linux.alibaba.com \
    --cc=adilger.kernel@dilger.ca \
    --cc=linux-ext4@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=tytso@mit.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox