Skip to contentSkip to conversation

f2fs: do in batch synchronously readahead during GC

PATCH In review Chao Yu 3 participants 5 messages Aug 26, 2016 – Aug 29, 2016 F2FS file system

lore
Share & tools
lore

Chao Yu

author maintainer
#0 lore raw

PATCH 1/2 f2fs: do in batch synchronously readahead during GC

From: Chao Yu <[email protected]>

In order to enhance performance, we try to readahead node page during GC, but before loading node page we should get block address of node page which is stored in NAT table, so synchronously read of single NAT page block our readahead flow.

f2fs_submit_page_bio: dev = (251,0), ino = 2, page_index = 0xa1e, oldaddr = 0xa1e, newaddr = 0xa1e, rw = READ_SYNC(MP), type = META
f2fs_submit_page_bio: dev = (251,0), ino = 1, page_index = 0x35e9, oldaddr = 0x72d7a, newaddr = 0x72d7a, rw = READAHEAD ^H, type = NODE
f2fs_submit_page_bio: dev = (251,0), ino = 2, page_index = 0xc1f, oldaddr = 0xc1f, newaddr = 0xc1f, rw = READ_SYNC(MP), type = META
f2fs_submit_page_bio: dev = (251,0), ino = 1, page_index = 0x389d, oldaddr = 0x72d7d, newaddr = 0x72d7d, rw = READAHEAD ^H, type = NODE
f2fs_submit_page_bio: dev = (251,0), ino = 1, page_index = 0x3a82, oldaddr = 0x72d7f, newaddr = 0x72d7f, rw = READAHEAD ^H, type = NODE
f2fs_submit_page_bio: dev = (251,0), ino = 1, page_index = 0x3bfa, oldaddr = 0x72d86, newaddr = 0x72d86, rw = READAHEAD ^H, type = NODE

This patch adds one phase that do readahead NAT pages in batch before readahead node page for more effeciently.

f2fs_submit_page_bio: dev = (251,0), ino = 2, page_index = 0x1952, oldaddr = 0x1952, newaddr = 0x1952, rw = READ_SYNC(MP), type = META
f2fs_submit_page_mbio: dev = (251,0), ino = 2, page_index = 0xc34, oldaddr = 0xc34, newaddr = 0xc34, rw = READ_SYNC(MP), type = META
f2fs_submit_page_mbio: dev = (251,0), ino = 2, page_index = 0xa33, oldaddr = 0xa33, newaddr = 0xa33, rw = READ_SYNC(MP), type = META
f2fs_submit_page_mbio: dev = (251,0), ino = 2, page_index = 0xc30, oldaddr = 0xc30, newaddr = 0xc30, rw = READ_SYNC(MP), type = META
f2fs_submit_page_mbio: dev = (251,0), ino = 2, page_index = 0xc32, oldaddr = 0xc32, newaddr = 0xc32, rw = READ_SYNC(MP), type = META
f2fs_submit_page_mbio: dev = (251,0), ino = 2, page_index = 0xc26, oldaddr = 0xc26, newaddr = 0xc26, rw = READ_SYNC(MP), type = META
f2fs_submit_page_mbio: dev = (251,0), ino = 2, page_index = 0xa2b, oldaddr = 0xa2b, newaddr = 0xa2b, rw = READ_SYNC(MP), type = META
f2fs_submit_page_mbio: dev = (251,0), ino = 2, page_index = 0xc23, oldaddr = 0xc23, newaddr = 0xc23, rw = READ_SYNC(MP), type = META
f2fs_submit_page_mbio: dev = (251,0), ino = 2, page_index = 0xc24, oldaddr = 0xc24, newaddr = 0xc24, rw = READ_SYNC(MP), type = META
f2fs_submit_page_mbio: dev = (251,0), ino = 2, page_index = 0xa10, oldaddr = 0xa10, newaddr = 0xa10, rw = READ_SYNC(MP), type = META
f2fs_submit_page_mbio: dev = (251,0), ino = 2, page_index = 0xc2c, oldaddr = 0xc2c, newaddr = 0xc2c, rw = READ_SYNC(MP), type = META
f2fs_submit_page_bio: dev = (251,0), ino = 1, page_index = 0x5db7, oldaddr = 0x6be00, newaddr = 0x6be00, rw = READAHEAD ^H, type = NODE
f2fs_submit_page_bio: dev = (251,0), ino = 1, page_index = 0x5db9, oldaddr = 0x6be17, newaddr = 0x6be17, rw = READAHEAD ^H, type = NODE
f2fs_submit_page_bio: dev = (251,0), ino = 1, page_index = 0x5dbc, oldaddr = 0x6be1a, newaddr = 0x6be1a, rw = READAHEAD ^H, type = NODE
f2fs_submit_page_bio: dev = (251,0), ino = 1, page_index = 0x5dc3, oldaddr = 0x6be20, newaddr = 0x6be20, rw = READAHEAD ^H, type = NODE
f2fs_submit_page_bio: dev = (251,0), ino = 1, page_index = 0x5dc7, oldaddr = 0x6be24, newaddr = 0x6be24, rw = READAHEAD ^H, type = NODE
f2fs_submit_page_bio: dev = (251,0), ino = 1, page_index = 0x5dc9, oldaddr = 0x6be25, newaddr = 0x6be25, rw = READAHEAD ^H, type = NODE

---
 fs/f2fs/gc.c | 33 +++++++++++++++++++++++----------
 1 file changed, 23 insertions(+), 10 deletions(-)
fs/f2fs/gc.c +23 -10
index c1599b4..cdc44a6 100644
@@ -423,10 +423,10 @@ static int check_valid_map(struct f2fs_sb_info *sbi,
 static void gc_node_segment(struct f2fs_sb_info *sbi,
 		struct f2fs_summary *sum, unsigned int segno, int gc_type)
 {
-	bool initial = true;
 	struct f2fs_summary *entry;
 	block_t start_addr;
 	int off;
+	int phase = 0;
 
 	start_addr = START_BLOCK(sbi, segno);
 
@@ -445,10 +445,18 @@ next_step:
 		if (check_valid_map(sbi, segno, off) == 0)
 			continue;
 
-		if (initial) {
+		if (phase == 0) {
+			ra_meta_pages(sbi, NAT_BLOCK_OFFSET(nid), 1,
+							META_NAT, true);
+			continue;
+		}
+
+		if (phase == 1) {
 			ra_node_page(sbi, nid);
 			continue;
 		}
+
+		/* phase == 2 */
 		node_page = get_node_page(sbi, nid);
 		if (IS_ERR(node_page))
 			continue;
@@ -469,10 +477,8 @@ next_step:
 		stat_inc_node_blk_count(sbi, 1, gc_type);
 	}
 
-	if (initial) {
-		initial = false;
+	if (++phase < 3)
 		goto next_step;
-	}
 }
 
 /*
@@ -706,6 +712,7 @@ next_step:
 		struct node_info dni; /* dnode info for the data */
 		unsigned int ofs_in_node, nofs;
 		block_t start_bidx;
+		nid_t nid = le32_to_cpu(entry->nid);
 
 		/* stop BG_GC if there is not enough free sections. */
 		if (gc_type == BG_GC && has_not_enough_free_secs(sbi, 0))
@@ -715,7 +722,13 @@ next_step:
 			continue;
 
 		if (phase == 0) {
-			ra_node_page(sbi, le32_to_cpu(entry->nid));
+			ra_meta_pages(sbi, NAT_BLOCK_OFFSET(nid), 1,
+							META_NAT, true);
+			continue;
+		}
+
+		if (phase == 1) {
+			ra_node_page(sbi, nid);
 			continue;
 		}
 
@@ -723,14 +736,14 @@ next_step:
 		if (!is_alive(sbi, entry, &dni, start_addr + off, &nofs))
 			continue;
 
-		if (phase == 1) {
+		if (phase == 2) {
 			ra_node_page(sbi, dni.ino);
 			continue;
 		}
 
 		ofs_in_node = le16_to_cpu(entry->ofs_in_node);
 
-		if (phase == 2) {
+		if (phase == 3) {
 			inode = f2fs_iget(sb, dni.ino);
 			if (IS_ERR(inode) || is_bad_inode(inode))
 				continue;
@@ -756,7 +769,7 @@ next_step:
 			continue;
 		}
 
-		/* phase 3 */
+		/* phase 4 */
 		inode = find_gc_inode(gc_list, dni.ino);
 		if (inode) {
 			struct f2fs_inode_info *fi = F2FS_I(inode);
@@ -789,7 +802,7 @@ next_step:
 		}
 	}
 
-	if (++phase < 4)
+	if (++phase < 5)
 		goto next_step;
 }
 
Signature · 4 lines
2.7.2


------------------------------------------------------------------------------
author maintainer
#1 lore raw

PATCH 2/2 f2fs: fix to update node page under cp_rwsem

From: Chao Yu <[email protected]>

Update node page under cp_rwsem in order to keep data consistency during writting checkpoint.

---
 fs/f2fs/inode.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
fs/f2fs/inode.c +4 -1
index 9ac5efc..1057c73 100644
@@ -377,8 +377,11 @@ retry:
 		goto retry;
 	}
 
-	if (err)
+	if (err) {
+		f2fs_lock_op(sbi);
 		update_inode_page(inode);
+		f2fs_unlock_op(sbi);
+	}
 	sb_end_intwrite(inode->i_sb);
 no_delete:
 	stat_dec_inline_xattr(inode);
Signature · 4 lines
2.7.2


------------------------------------------------------------------------------
maintainer
#2 lore raw

Re: [PATCH 2/2] f2fs: fix to update node page under cp_rwsem

Quoted 15 lines · Chao Yu

From: Chao Yu <[email protected]>

Update node page under cp_rwsem in order to keep data consistency during writting checkpoint.

---
 fs/f2fs/inode.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
fs/f2fs/inode.c +0 -0
index 9ac5efc..1057c73 100644

fs/f2fs/inode.c retry: · line 384

Quoted 9 lines
fs/f2fs/inode.c +4 -1
@@ -377,8 +377,11 @@ retry:
 		goto retry;
 	}
 
-	if (err)
+	if (err) {
+		f2fs_lock_op(sbi);
 		update_inode_page(inode);
+		f2fs_unlock_op(sbi);
+	}

Any corner case?
Now, it allows updating node page when checkpoint is doing.

Thanks,

Quoted 5 lines
fs/f2fs/inode.c +0 -0
 	sb_end_intwrite(inode->i_sb);
 no_delete:
 	stat_dec_inline_xattr(inode);
Signature · 1 line
2.7.2
#3 lore raw

Re: [PATCH 2/2] f2fs: fix to update node page under cp_rwsem

Quoted 19 lines · Jaegeuk Kim
Quoted 18 lines · Chao Yu

From: Chao Yu <[email protected]>

Update node page under cp_rwsem in order to keep data consistency during writting checkpoint.

---
 fs/f2fs/inode.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
fs/f2fs/inode.c +0 -0
index 9ac5efc..1057c73 100644
@@ -377,8 +377,11 @@ retry:
 		goto retry;
 	}
 
Quoted 8 lines

fs/f2fs/inode.c retry: · line 384

Quoted 6 lines
fs/f2fs/inode.c +4 -1
-	if (err)
+	if (err) {
+		f2fs_lock_op(sbi);
 		update_inode_page(inode);
+		f2fs_unlock_op(sbi);
+	}

Any corner case?

No, just follow the old rule.

Quoted 1 line

Now, it allows updating node page when checkpoint is doing.

Oh, so for current rule, during checkpoint it:
1. allows updating meta info of inode which is not related to metadata of filesystem (e.g. i_size, i_block, i_nlink..)
2. does not allow updating meta info of inode which is related to metadata of filesystem (i_addr, i_nid..)

Is my understanding right?

Thanks,

Quoted 11 lines

Thanks,

Quoted 5 lines
fs/f2fs/inode.c +0 -0
 	sb_end_intwrite(inode->i_sb);
 no_delete:
 	stat_dec_inline_xattr(inode);
Signature · 1 line
2.7.2

.

------------------------------------------------------------------------------

maintainer
#4 lore raw

Re: [PATCH 2/2] f2fs: fix to update node page under cp_rwsem

Quoted 31 lines · Chao Yu
Quoted 19 lines · Jaegeuk Kim
Quoted 18 lines · Chao Yu

From: Chao Yu <[email protected]>

Update node page under cp_rwsem in order to keep data consistency during writting checkpoint.

---
 fs/f2fs/inode.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
fs/f2fs/inode.c +0 -0
index 9ac5efc..1057c73 100644
@@ -377,8 +377,11 @@ retry:
 		goto retry;
 	}
 
Quoted 8 lines

fs/f2fs/inode.c retry: · line 384

Quoted 6 lines
fs/f2fs/inode.c +4 -1
-	if (err)
+	if (err) {
+		f2fs_lock_op(sbi);
 		update_inode_page(inode);
+		f2fs_unlock_op(sbi);
+	}

Any corner case?

No, just follow the old rule.

Quoted 7 lines
Quoted 1 line

Now, it allows updating node page when checkpoint is doing.

Oh, so for current rule, during checkpoint it:
1. allows updating meta info of inode which is not related to metadata of filesystem (e.g. i_size, i_block, i_nlink..)
2. does not allow updating meta info of inode which is related to metadata of filesystem (i_addr, i_nid..)

Seems like that.
The checkpoint flushes any dirty inodes by block_operations. And, this is to allow any meta updates in its inode page which doesn't hurt current processing checkpoint.

Thanks,

Quoted 16 lines

Is my understanding right?

Thanks,

Quoted 11 lines

Thanks,

Quoted 5 lines
fs/f2fs/inode.c +0 -0
 	sb_end_intwrite(inode->i_sb);
 no_delete:
 	stat_dec_inline_xattr(inode);
Signature · 1 line
2.7.2

.