* [PATCH] Revert "f2fs: handle dirty segments inside refresh_sit_entry"
@ 2017-10-28 11:57 Chao Yu
2017-10-28 15:43 ` [PATCH v2] This reverts commit 5e443818fa0b2a2845561ee25bec181424fb2889 Yunlong Song
2017-10-30 1:33 ` [PATCH v2 RESEND] Revert "f2fs: handle dirty segments inside refresh_sit_entry" Yunlong Song
0 siblings, 2 replies; 5+ messages in thread
From: Chao Yu @ 2017-10-28 11:57 UTC (permalink / raw)
To: jaegeuk; +Cc: linux-kernel, linux-f2fs-devel
From: Yunlong Song <yunlong.song@huawei.com>
This reverts commit 5e443818fa0b2a2845561ee25bec181424fb2889
The commit should be reverted because call sequence of below two parts
of code must be kept:
a. update sit information, it needs to be updated before segment
allocation since latter allocation may trigger SSR, and SSR allocation
needs latest valid block information of all segments.
b. update segment status, it needs to be updated after segment allocation
since we can skip updating current opened segment status.
Fixes: 5e443818fa0b ("f2fs: handle dirty segments inside refresh_sit_entry")
Suggested-by: Chao Yu <yuchao0@huawei.com>
Signed-off-by: Yunlong Song <yunlong.song@huawei.com>
---
fs/f2fs/segment.c | 18 ++++++++++++------
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index 0de1761928d3..644706000e31 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -1899,9 +1899,6 @@ void refresh_sit_entry(struct f2fs_sb_info *sbi, block_t old, block_t new)
update_sit_entry(sbi, new, 1);
if (GET_SEGNO(sbi, old) != NULL_SEGNO)
update_sit_entry(sbi, old, -1);
-
- locate_dirty_segment(sbi, GET_SEGNO(sbi, old));
- locate_dirty_segment(sbi, GET_SEGNO(sbi, new));
}
void invalidate_blocks(struct f2fs_sb_info *sbi, block_t addr)
@@ -2540,13 +2537,22 @@ void allocate_data_block(struct f2fs_sb_info *sbi, struct page *page,
stat_inc_block_count(sbi, curseg);
+ /*
+ * SIT information should be updated before segment allocation,
+ * since SSR needs latest valid block information.
+ */
+ refresh_sit_entry(sbi, old_blkaddr, *new_blkaddr);
+
if (!__has_curseg_space(sbi, type))
sit_i->s_ops->allocate_segment(sbi, type, false);
+
/*
- * SIT information should be updated after segment allocation,
- * since we need to keep dirty segments precisely under SSR.
+ * segment dirty status should be updated after segment allocation,
+ * so we just need to update status only one time after previous
+ * segment being closed.
*/
- refresh_sit_entry(sbi, old_blkaddr, *new_blkaddr);
+ locate_dirty_segment(sbi, GET_SEGNO(sbi, old_blkaddr));
+ locate_dirty_segment(sbi, GET_SEGNO(sbi, *new_blkaddr));
mutex_unlock(&sit_i->sentry_lock);
--
2.14.1.145.gb3622a4ee
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH v2] This reverts commit 5e443818fa0b2a2845561ee25bec181424fb2889 2017-10-28 11:57 [PATCH] Revert "f2fs: handle dirty segments inside refresh_sit_entry" Chao Yu @ 2017-10-28 15:43 ` Yunlong Song 2017-10-30 1:33 ` [PATCH v2 RESEND] Revert "f2fs: handle dirty segments inside refresh_sit_entry" Yunlong Song 1 sibling, 0 replies; 5+ messages in thread From: Yunlong Song @ 2017-10-28 15:43 UTC (permalink / raw) To: jaegeuk, chao, yuchao0, yunlong.song, yunlong.song Cc: linux-fsdevel, miaoxie, linux-kernel, linux-f2fs-devel The commit should be reverted because call sequence of below two parts of code must be kept: a. update sit information, it needs to be updated before segment allocation since latter allocation may trigger SSR, and SSR allocation needs latest valid block information of all segments. b. update segment status, it needs to be updated after segment allocation since we can skip updating current opened segment status. Fixes: 5e443818fa0b ("f2fs: handle dirty segments inside refresh_sit_entry") Suggested-by: Chao Yu <yuchao0@huawei.com> Signed-off-by: Yunlong Song <yunlong.song@huawei.com> --- fs/f2fs/f2fs.h | 1 - fs/f2fs/segment.c | 20 +++++++++++++------- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h index 13a96b8..f166112 100644 --- a/fs/f2fs/f2fs.h +++ b/fs/f2fs/f2fs.h @@ -2567,7 +2567,6 @@ int restore_node_summary(struct f2fs_sb_info *sbi, bool is_checkpointed_data(struct f2fs_sb_info *sbi, block_t blkaddr); void init_discard_policy(struct discard_policy *dpolicy, int discard_type, unsigned int granularity); -void refresh_sit_entry(struct f2fs_sb_info *sbi, block_t old, block_t new); void stop_discard_thread(struct f2fs_sb_info *sbi); bool f2fs_wait_discard_bios(struct f2fs_sb_info *sbi); void clear_prefree_segments(struct f2fs_sb_info *sbi, struct cp_control *cpc); diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c index 46dfbca..a3509e9 100644 --- a/fs/f2fs/segment.c +++ b/fs/f2fs/segment.c @@ -1884,14 +1884,11 @@ static void update_sit_entry(struct f2fs_sb_info *sbi, block_t blkaddr, int del) get_sec_entry(sbi, segno)->valid_blocks += del; } -void refresh_sit_entry(struct f2fs_sb_info *sbi, block_t old, block_t new) +static void refresh_sit_entry(struct f2fs_sb_info *sbi, block_t old, block_t new) { update_sit_entry(sbi, new, 1); if (GET_SEGNO(sbi, old) != NULL_SEGNO) update_sit_entry(sbi, old, -1); - - locate_dirty_segment(sbi, GET_SEGNO(sbi, old)); - locate_dirty_segment(sbi, GET_SEGNO(sbi, new)); } void invalidate_blocks(struct f2fs_sb_info *sbi, block_t addr) @@ -2529,13 +2526,22 @@ void allocate_data_block(struct f2fs_sb_info *sbi, struct page *page, stat_inc_block_count(sbi, curseg); + /* + * SIT information should be updated before segment allocation, + * since SSR needs latest valid block information. + */ + refresh_sit_entry(sbi, old_blkaddr, *new_blkaddr); + if (!__has_curseg_space(sbi, type)) sit_i->s_ops->allocate_segment(sbi, type, false); + /* - * SIT information should be updated after segment allocation, - * since we need to keep dirty segments precisely under SSR. + * segment dirty status should be updated after segment allocation, + * so we just need to update status only one time after previous + * segment being closed. */ - refresh_sit_entry(sbi, old_blkaddr, *new_blkaddr); + locate_dirty_segment(sbi, GET_SEGNO(sbi, old_blkaddr)); + locate_dirty_segment(sbi, GET_SEGNO(sbi, *new_blkaddr)); mutex_unlock(&sit_i->sentry_lock); -- 1.8.5.2 ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v2 RESEND] Revert "f2fs: handle dirty segments inside refresh_sit_entry" 2017-10-28 11:57 [PATCH] Revert "f2fs: handle dirty segments inside refresh_sit_entry" Chao Yu 2017-10-28 15:43 ` [PATCH v2] This reverts commit 5e443818fa0b2a2845561ee25bec181424fb2889 Yunlong Song @ 2017-10-30 1:33 ` Yunlong Song 2017-10-30 3:36 ` Chao Yu 1 sibling, 1 reply; 5+ messages in thread From: Yunlong Song @ 2017-10-30 1:33 UTC (permalink / raw) To: jaegeuk, chao, yuchao0, yunlong.song, yunlong.song Cc: linux-fsdevel, miaoxie, linux-kernel, linux-f2fs-devel This reverts commit 5e443818fa0b2a2845561ee25bec181424fb2889 The commit should be reverted because call sequence of below two parts of code must be kept: a. update sit information, it needs to be updated before segment allocation since latter allocation may trigger SSR, and SSR allocation needs latest valid block information of all segments. b. update segment status, it needs to be updated after segment allocation since we can skip updating current opened segment status. Fixes: 5e443818fa0b ("f2fs: handle dirty segments inside refresh_sit_entry") Suggested-by: Chao Yu <yuchao0@huawei.com> Signed-off-by: Yunlong Song <yunlong.song@huawei.com> --- fs/f2fs/f2fs.h | 1 - fs/f2fs/segment.c | 20 +++++++++++++------- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h index 13a96b8..f166112 100644 --- a/fs/f2fs/f2fs.h +++ b/fs/f2fs/f2fs.h @@ -2567,7 +2567,6 @@ int restore_node_summary(struct f2fs_sb_info *sbi, bool is_checkpointed_data(struct f2fs_sb_info *sbi, block_t blkaddr); void init_discard_policy(struct discard_policy *dpolicy, int discard_type, unsigned int granularity); -void refresh_sit_entry(struct f2fs_sb_info *sbi, block_t old, block_t new); void stop_discard_thread(struct f2fs_sb_info *sbi); bool f2fs_wait_discard_bios(struct f2fs_sb_info *sbi); void clear_prefree_segments(struct f2fs_sb_info *sbi, struct cp_control *cpc); diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c index 46dfbca..a3509e9 100644 --- a/fs/f2fs/segment.c +++ b/fs/f2fs/segment.c @@ -1884,14 +1884,11 @@ static void update_sit_entry(struct f2fs_sb_info *sbi, block_t blkaddr, int del) get_sec_entry(sbi, segno)->valid_blocks += del; } -void refresh_sit_entry(struct f2fs_sb_info *sbi, block_t old, block_t new) +static void refresh_sit_entry(struct f2fs_sb_info *sbi, block_t old, block_t new) { update_sit_entry(sbi, new, 1); if (GET_SEGNO(sbi, old) != NULL_SEGNO) update_sit_entry(sbi, old, -1); - - locate_dirty_segment(sbi, GET_SEGNO(sbi, old)); - locate_dirty_segment(sbi, GET_SEGNO(sbi, new)); } void invalidate_blocks(struct f2fs_sb_info *sbi, block_t addr) @@ -2529,13 +2526,22 @@ void allocate_data_block(struct f2fs_sb_info *sbi, struct page *page, stat_inc_block_count(sbi, curseg); + /* + * SIT information should be updated before segment allocation, + * since SSR needs latest valid block information. + */ + refresh_sit_entry(sbi, old_blkaddr, *new_blkaddr); + if (!__has_curseg_space(sbi, type)) sit_i->s_ops->allocate_segment(sbi, type, false); + /* - * SIT information should be updated after segment allocation, - * since we need to keep dirty segments precisely under SSR. + * segment dirty status should be updated after segment allocation, + * so we just need to update status only one time after previous + * segment being closed. */ - refresh_sit_entry(sbi, old_blkaddr, *new_blkaddr); + locate_dirty_segment(sbi, GET_SEGNO(sbi, old_blkaddr)); + locate_dirty_segment(sbi, GET_SEGNO(sbi, *new_blkaddr)); mutex_unlock(&sit_i->sentry_lock); -- 1.8.5.2 ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2 RESEND] Revert "f2fs: handle dirty segments inside refresh_sit_entry" 2017-10-30 1:33 ` [PATCH v2 RESEND] Revert "f2fs: handle dirty segments inside refresh_sit_entry" Yunlong Song @ 2017-10-30 3:36 ` Chao Yu 2017-10-30 9:39 ` Jaegeuk Kim 0 siblings, 1 reply; 5+ messages in thread From: Chao Yu @ 2017-10-30 3:36 UTC (permalink / raw) To: Yunlong Song, jaegeuk, yunlong.song Cc: linux-kernel, linux-f2fs-devel, linux-fsdevel, miaoxie On 2017/10/30 9:33, Yunlong Song wrote: > This reverts commit 5e443818fa0b2a2845561ee25bec181424fb2889 > > The commit should be reverted because call sequence of below two parts > of code must be kept: > a. update sit information, it needs to be updated before segment > allocation since latter allocation may trigger SSR, and SSR allocation > needs latest valid block information of all segments. > b. update segment status, it needs to be updated after segment allocation > since we can skip updating current opened segment status. > > Fixes: 5e443818fa0b ("f2fs: handle dirty segments inside refresh_sit_entry") > Suggested-by: Chao Yu <yuchao0@huawei.com> > Signed-off-by: Yunlong Song <yunlong.song@huawei.com> Reviewed-by: Chao Yu <yuchao0@huawei.com> Thanks, > --- > fs/f2fs/f2fs.h | 1 - > fs/f2fs/segment.c | 20 +++++++++++++------- > 2 files changed, 13 insertions(+), 8 deletions(-) > > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h > index 13a96b8..f166112 100644 > --- a/fs/f2fs/f2fs.h > +++ b/fs/f2fs/f2fs.h > @@ -2567,7 +2567,6 @@ int restore_node_summary(struct f2fs_sb_info *sbi, > bool is_checkpointed_data(struct f2fs_sb_info *sbi, block_t blkaddr); > void init_discard_policy(struct discard_policy *dpolicy, int discard_type, > unsigned int granularity); > -void refresh_sit_entry(struct f2fs_sb_info *sbi, block_t old, block_t new); > void stop_discard_thread(struct f2fs_sb_info *sbi); > bool f2fs_wait_discard_bios(struct f2fs_sb_info *sbi); > void clear_prefree_segments(struct f2fs_sb_info *sbi, struct cp_control *cpc); > diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c > index 46dfbca..a3509e9 100644 > --- a/fs/f2fs/segment.c > +++ b/fs/f2fs/segment.c > @@ -1884,14 +1884,11 @@ static void update_sit_entry(struct f2fs_sb_info *sbi, block_t blkaddr, int del) > get_sec_entry(sbi, segno)->valid_blocks += del; > } > > -void refresh_sit_entry(struct f2fs_sb_info *sbi, block_t old, block_t new) > +static void refresh_sit_entry(struct f2fs_sb_info *sbi, block_t old, block_t new) > { > update_sit_entry(sbi, new, 1); > if (GET_SEGNO(sbi, old) != NULL_SEGNO) > update_sit_entry(sbi, old, -1); > - > - locate_dirty_segment(sbi, GET_SEGNO(sbi, old)); > - locate_dirty_segment(sbi, GET_SEGNO(sbi, new)); > } > > void invalidate_blocks(struct f2fs_sb_info *sbi, block_t addr) > @@ -2529,13 +2526,22 @@ void allocate_data_block(struct f2fs_sb_info *sbi, struct page *page, > > stat_inc_block_count(sbi, curseg); > > + /* > + * SIT information should be updated before segment allocation, > + * since SSR needs latest valid block information. > + */ > + refresh_sit_entry(sbi, old_blkaddr, *new_blkaddr); > + > if (!__has_curseg_space(sbi, type)) > sit_i->s_ops->allocate_segment(sbi, type, false); > + > /* > - * SIT information should be updated after segment allocation, > - * since we need to keep dirty segments precisely under SSR. > + * segment dirty status should be updated after segment allocation, > + * so we just need to update status only one time after previous > + * segment being closed. > */ > - refresh_sit_entry(sbi, old_blkaddr, *new_blkaddr); > + locate_dirty_segment(sbi, GET_SEGNO(sbi, old_blkaddr)); > + locate_dirty_segment(sbi, GET_SEGNO(sbi, *new_blkaddr)); > > mutex_unlock(&sit_i->sentry_lock); > > ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 RESEND] Revert "f2fs: handle dirty segments inside refresh_sit_entry" 2017-10-30 3:36 ` Chao Yu @ 2017-10-30 9:39 ` Jaegeuk Kim 0 siblings, 0 replies; 5+ messages in thread From: Jaegeuk Kim @ 2017-10-30 9:39 UTC (permalink / raw) To: Chao Yu Cc: Yunlong Song, yunlong.song, chao, miaoxie, bintian.wang, linux-fsdevel, linux-f2fs-devel, linux-kernel On 10/30, Chao Yu wrote: > On 2017/10/30 9:33, Yunlong Song wrote: > > This reverts commit 5e443818fa0b2a2845561ee25bec181424fb2889 > > > > The commit should be reverted because call sequence of below two parts > > of code must be kept: > > a. update sit information, it needs to be updated before segment > > allocation since latter allocation may trigger SSR, and SSR allocation > > needs latest valid block information of all segments. > > b. update segment status, it needs to be updated after segment allocation > > since we can skip updating current opened segment status. > > > > Fixes: 5e443818fa0b ("f2fs: handle dirty segments inside refresh_sit_entry") > > Suggested-by: Chao Yu <yuchao0@huawei.com> > > Signed-off-by: Yunlong Song <yunlong.song@huawei.com> > > Reviewed-by: Chao Yu <yuchao0@huawei.com> > > Thanks, > > > --- > > fs/f2fs/f2fs.h | 1 - > > fs/f2fs/segment.c | 20 +++++++++++++------- > > 2 files changed, 13 insertions(+), 8 deletions(-) > > > > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h > > index 13a96b8..f166112 100644 > > --- a/fs/f2fs/f2fs.h > > +++ b/fs/f2fs/f2fs.h > > @@ -2567,7 +2567,6 @@ int restore_node_summary(struct f2fs_sb_info *sbi, > > bool is_checkpointed_data(struct f2fs_sb_info *sbi, block_t blkaddr); > > void init_discard_policy(struct discard_policy *dpolicy, int discard_type, > > unsigned int granularity); > > -void refresh_sit_entry(struct f2fs_sb_info *sbi, block_t old, block_t new); > > void stop_discard_thread(struct f2fs_sb_info *sbi); > > bool f2fs_wait_discard_bios(struct f2fs_sb_info *sbi); > > void clear_prefree_segments(struct f2fs_sb_info *sbi, struct cp_control *cpc); > > diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c > > index 46dfbca..a3509e9 100644 > > --- a/fs/f2fs/segment.c > > +++ b/fs/f2fs/segment.c > > @@ -1884,14 +1884,11 @@ static void update_sit_entry(struct f2fs_sb_info *sbi, block_t blkaddr, int del) > > get_sec_entry(sbi, segno)->valid_blocks += del; > > } > > > > -void refresh_sit_entry(struct f2fs_sb_info *sbi, block_t old, block_t new) > > +static void refresh_sit_entry(struct f2fs_sb_info *sbi, block_t old, block_t new) IMO, we don't even need this function, and insteda, do update_sit_entry work directly below. Merged with it, so let me know, if you have any concern. Thanks, > > { > > update_sit_entry(sbi, new, 1); > > if (GET_SEGNO(sbi, old) != NULL_SEGNO) > > update_sit_entry(sbi, old, -1); > > - > > - locate_dirty_segment(sbi, GET_SEGNO(sbi, old)); > > - locate_dirty_segment(sbi, GET_SEGNO(sbi, new)); > > } > > > > void invalidate_blocks(struct f2fs_sb_info *sbi, block_t addr) > > @@ -2529,13 +2526,22 @@ void allocate_data_block(struct f2fs_sb_info *sbi, struct page *page, > > > > stat_inc_block_count(sbi, curseg); > > > > + /* > > + * SIT information should be updated before segment allocation, > > + * since SSR needs latest valid block information. > > + */ > > + refresh_sit_entry(sbi, old_blkaddr, *new_blkaddr); > > + > > if (!__has_curseg_space(sbi, type)) > > sit_i->s_ops->allocate_segment(sbi, type, false); > > + > > /* > > - * SIT information should be updated after segment allocation, > > - * since we need to keep dirty segments precisely under SSR. > > + * segment dirty status should be updated after segment allocation, > > + * so we just need to update status only one time after previous > > + * segment being closed. > > */ > > - refresh_sit_entry(sbi, old_blkaddr, *new_blkaddr); > > + locate_dirty_segment(sbi, GET_SEGNO(sbi, old_blkaddr)); > > + locate_dirty_segment(sbi, GET_SEGNO(sbi, *new_blkaddr)); > > > > mutex_unlock(&sit_i->sentry_lock); > > > > ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-10-30 9:39 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-10-28 11:57 [PATCH] Revert "f2fs: handle dirty segments inside refresh_sit_entry" Chao Yu 2017-10-28 15:43 ` [PATCH v2] This reverts commit 5e443818fa0b2a2845561ee25bec181424fb2889 Yunlong Song 2017-10-30 1:33 ` [PATCH v2 RESEND] Revert "f2fs: handle dirty segments inside refresh_sit_entry" Yunlong Song 2017-10-30 3:36 ` Chao Yu 2017-10-30 9:39 ` Jaegeuk Kim
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).