linux-ext4 - [PATCH] ext4: remove unnecessary ext4_inode_datasync

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Date:   Tue,  2 Nov 2021 10:42:58 +0800
From:   Zhongwei Cai <sunrise_l@...u.edu.cn>
To:     tytso@....edu, adilger.kernel@...ger.ca
Cc:     linux-ext4@...r.kernel.org, mingkaidong@...il.com,
        Zhongwei Cai <sunrise_l@...u.edu.cn>
Subject: [PATCH] ext4: remove unnecessary ext4_inode_datasync_dirty in read path

ext4_inode_datasync_dirty will call read_lock(&journal->j_state_lock) in
journal mode, which is unnecessary in read path (As far as I know, the
IOMAP_F_DIRTY flag set in the if branch is only used in write path,
making it unnecessary in read path. Please correct me if I'm wrong).
and will cause cache contention overhead under high concurrency
especially in DAX mode. The unnecessary ext4_inode_datasync_dirty can be
eliminated by passing flags into ext4_set_iomap and checking it.

Performance tests are shown below. Workloads include simply reading files,
fileserver in filebench and readrandom/readsequence in RocksDB under
nosync mode.

Sixteen thread performance under ext4-DAX:
 Throughput (Kop/s) | original |  +patch  | improvement
 -------------------+----------+----------+--------------
 Read 4KB block     |   11456  |   27651  |  +141.37%
 fileserver         |     339  |     343  |  +1.18%
 readrandom         |    1807  |    1837  |  +1.66%
 readsequence       |   29724  |   30102  |  +1.27%

Signed-off-by: Zhongwei Cai <sunrise_l@...u.edu.cn>
---
 fs/ext4/inode.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 0f06305167d5..72ec2074ef54 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -3274,7 +3274,7 @@ static bool ext4_inode_datasync_dirty(struct inode *inode)
 
 static void ext4_set_iomap(struct inode *inode, struct iomap *iomap,
 			   struct ext4_map_blocks *map, loff_t offset,
-			   loff_t length)
+			   loff_t length, int flags)
 {
 	u8 blkbits = inode->i_blkbits;
 
@@ -3284,8 +3284,8 @@ static void ext4_set_iomap(struct inode *inode, struct iomap *iomap,
 	 * there is no other metadata changes being made or are pending.
 	 */
 	iomap->flags = 0;
-	if (ext4_inode_datasync_dirty(inode) ||
-	    offset + length > i_size_read(inode))
+	if ((flags & IOMAP_WRITE) && (ext4_inode_datasync_dirty(inode) ||
+	    offset + length > i_size_read(inode)))
 		iomap->flags |= IOMAP_F_DIRTY;
 
 	if (map->m_flags & EXT4_MAP_NEW)
@@ -3423,7 +3423,7 @@ static int ext4_iomap_begin(struct inode *inode, loff_t offset, loff_t length,
 	if (ret < 0)
 		return ret;
 out:
-	ext4_set_iomap(inode, iomap, &map, offset, length);
+	ext4_set_iomap(inode, iomap, &map, offset, length, flags);
 
 	return 0;
 }
@@ -3543,7 +3543,7 @@ static int ext4_iomap_begin_report(struct inode *inode, loff_t offset,
 		delalloc = ext4_iomap_is_delalloc(inode, &map);
 
 set_iomap:
-	ext4_set_iomap(inode, iomap, &map, offset, length);
+	ext4_set_iomap(inode, iomap, &map, offset, length, flags);
 	if (delalloc && iomap->type == IOMAP_HOLE)
 		iomap->type = IOMAP_DELALLOC;
 
-- 
2.26.0