f2fs: add a wait step when submit bio with {READ,WRITE}_SYNC
Subsystems (2)
PATCH 1/1 f2fs: add a wait step when submit bio with {READ,WRITE}_SYNC
When we submit bio with READ_SYNC or WRITE_SYNC, we need to wait a moment for the io completion, current codes only find_data_page() follows the rule, other places missing this step, so add it.
Further more, moving the PageUptodate check into f2fs_readpage() to clean up the codes.
--- fs/f2fs/checkpoint.c | 1 - fs/f2fs/data.c | 39 +++++++++++++++++---------------------- fs/f2fs/node.c | 1 - fs/f2fs/recovery.c | 2 -- fs/f2fs/segment.c | 2 +- 5 files changed, 18 insertions(+), 27 deletions(-)
fs/f2fs/checkpoint.c +0 -1
@@ -64,7 +64,6 @@ repeat:if (f2fs_readpage (sbi, page, index,READ_SYNC )) goto repeat; -lock_page (page);if (page->mapping != mapping) {f2fs_put_page (page,1 ); goto repeat;
fs/f2fs/data.c +17 -22
@@ -216,13 +216,11 @@ struct page *find_data_page(struct inode *inode, pgoff_t index, bool sync) err =f2fs_readpage (sbi, page, dn.data_blkaddr, sync ?READ_SYNC :READA ); -if (sync) { -wait_on_page_locked (page); -if (!PageUptodate (page)) { -f2fs_put_page (page,0 ); -return ERR_PTR (-EIO ); - } - } +if (err) +return ERR_PTR (err); + +if (sync) +unlock_page (page);return page; }
@@ -267,11 +265,6 @@ repeat:if (err)return ERR_PTR (err); -lock_page (page); -if (!PageUptodate (page)) { -f2fs_put_page (page,1 ); -return ERR_PTR (-EIO ); - }if (page->mapping != mapping) {f2fs_put_page (page,1 ); goto repeat;
@@ -325,11 +318,7 @@ repeat: err =f2fs_readpage (sbi, page, dn.data_blkaddr,READ_SYNC );if (err)return ERR_PTR (err); -lock_page (page); -if (!PageUptodate (page)) { -f2fs_put_page (page,1 ); -return ERR_PTR (-EIO ); - } +if (page->mapping != mapping) {f2fs_put_page (page,1 ); goto repeat;
@@ -399,6 +388,16 @@ int f2fs_readpage(struct f2fs_sb_info *sbi, struct page *page,submit_bio (type, bio);up_read (&sbi->bio_sem); + +if (type ==READ_SYNC ) { +wait_on_page_locked (page); +lock_page (page); +if (!PageUptodate (page)) { +f2fs_put_page (page,1 ); +return -EIO ; + } + } +return 0 ; }
@@ -679,11 +678,7 @@ repeat: err =f2fs_readpage (sbi, page, dn.data_blkaddr,READ_SYNC );if (err)return err; -lock_page (page); -if (!PageUptodate (page)) { -f2fs_put_page (page,1 ); -return -EIO ; - } +if (page->mapping != mapping) {f2fs_put_page (page,1 ); goto repeat;
fs/f2fs/node.c +0 -1
@@ -1534,7 +1534,6 @@ int restore_node_summary(struct f2fs_sb_info *sbi,if (f2fs_readpage (sbi, page, addr,READ_SYNC )) goto out; -lock_page (page); rn =F2FS_NODE (page); sum_entry->nid = rn->footer.nid; sum_entry->version =0 ;
fs/f2fs/recovery.c +0 -2
@@ -140,8 +140,6 @@ static int find_fsync_dnodes(struct f2fs_sb_info *sbi, struct list_head *head)if (err) goto out; -lock_page (page); -if (cp_ver !=cpver_of_node (page))break ;
fs/f2fs/segment.c +1 -1
@@ -639,7 +639,7 @@ static void do_submit_bio(struct f2fs_sb_info *sbi,trace_f2fs_do_submit_bio (sbi->sb, btype, sync, sbi->bio[btype]); -if (type ==META_FLUSH ) { +if ((type ==META_FLUSH ) || (rw &WRITE_SYNC )) {DECLARE_COMPLETION_ONSTACK (wait); p->is_sync = true; p->wait = &wait;
Signature · 1 line
1.7.7
Hi Gu,
The original read flow was to avoid redandunt lock/unlock_page() calls. And we should not wait for WRITE_SYNC, since it is just for write priority, not for synchronization of the file system. Thanks,
2013-07-30 (화), 18:06 +0800, Gu Zheng:
Quoted 144 lines
When we submit bio with
READ_SYNCorWRITE_SYNC, we need to wait a moment for the io completion, current codes onlyfind_data_page()follows the rule, other places missing this step, so add it.Further more, moving the PageUptodate check into
f2fs_readpage()to clean up the codes.Signed-off-by Gu Zheng <[email protected]>--- fs/f2fs/checkpoint.c | 1 - fs/f2fs/data.c | 39 +++++++++++++++++---------------------- fs/f2fs/node.c | 1 - fs/f2fs/recovery.c | 2 -- fs/f2fs/segment.c | 2 +- 5 files changed, 18 insertions(+), 27 deletions(-)fs/f2fs/checkpoint.c +0
-1@@ -64,7 +64,6 @@ repeat:if (f2fs_readpage (sbi, page, index,READ_SYNC )) goto repeat; -lock_page (page);if (page->mapping != mapping) {f2fs_put_page (page,1 ); goto repeat;fs/f2fs/data.c +17
-22@@ -216,13 +216,11 @@ struct page *find_data_page(struct inode *inode, pgoff_t index, bool sync) err =f2fs_readpage (sbi, page, dn.data_blkaddr, sync ?READ_SYNC :READA ); -if (sync) { -wait_on_page_locked (page); -if (!PageUptodate (page)) { -f2fs_put_page (page,0 ); -return ERR_PTR (-EIO ); - } - } +if (err) +return ERR_PTR (err); + +if (sync) +unlock_page (page);return page; }@@ -267,11 +265,6 @@ repeat:if (err)return ERR_PTR (err); -lock_page (page); -if (!PageUptodate (page)) { -f2fs_put_page (page,1 ); -return ERR_PTR (-EIO ); - }if (page->mapping != mapping) {f2fs_put_page (page,1 ); goto repeat;@@ -325,11 +318,7 @@ repeat: err =f2fs_readpage (sbi, page, dn.data_blkaddr,READ_SYNC );if (err)return ERR_PTR (err); -lock_page (page); -if (!PageUptodate (page)) { -f2fs_put_page (page,1 ); -return ERR_PTR (-EIO ); - } +if (page->mapping != mapping) {f2fs_put_page (page,1 ); goto repeat;@@ -399,6 +388,16 @@ int f2fs_readpage(struct f2fs_sb_info *sbi, struct page *page,submit_bio (type, bio);up_read (&sbi->bio_sem); + +if (type ==READ_SYNC ) { +wait_on_page_locked (page); +lock_page (page); +if (!PageUptodate (page)) { +f2fs_put_page (page,1 ); +return -EIO ; + } + } +return 0 ; }@@ -679,11 +678,7 @@ repeat: err =f2fs_readpage (sbi, page, dn.data_blkaddr,READ_SYNC );if (err)return err; -lock_page (page); -if (!PageUptodate (page)) { -f2fs_put_page (page,1 ); -return -EIO ; - } +if (page->mapping != mapping) {f2fs_put_page (page,1 ); goto repeat;fs/f2fs/node.c +0
-1@@ -1534,7 +1534,6 @@ int restore_node_summary(struct f2fs_sb_info *sbi,if (f2fs_readpage (sbi, page, addr,READ_SYNC )) goto out; -lock_page (page); rn =F2FS_NODE (page); sum_entry->nid = rn->footer.nid; sum_entry->version =0 ;fs/f2fs/recovery.c +0
-2@@ -140,8 +140,6 @@ static int find_fsync_dnodes(struct f2fs_sb_info *sbi, struct list_head *head)if (err) goto out; -lock_page (page); -if (cp_ver !=cpver_of_node (page))break ;fs/f2fs/segment.c +1
-1@@ -639,7 +639,7 @@ static void do_submit_bio(struct f2fs_sb_info *sbi,trace_f2fs_do_submit_bio (sbi->sb, btype, sync, sbi->bio[btype]); -if (type ==META_FLUSH ) { +if ((type ==META_FLUSH ) || (rw &WRITE_SYNC )) {DECLARE_COMPLETION_ONSTACK (wait); p->is_sync = true; p->wait = &wait;
Signature · 7 lines
Jaegeuk Kim Samsung -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Kim,
Quoted 3 lines · Jaegeuk Kim
Hi Gu,
The original read flow was to avoid redandunt lock/unlock_page() calls.
Right, this can gain better read performance. But is the wait step after submitting bio with READ_SYNC needless too?
Quoted 2 lines
And we should not wait for
WRITE_SYNC, since it is just for write priority, not for synchronization of the file system.
Got it, thanks for your explanation.:)
Best regards,
Gu
Quoted 140 lines
Thanks,
2013-07-30 (화), 18:06 +0800, Gu Zheng:
Quoted 137 lines
When we submit bio with
READ_SYNCorWRITE_SYNC, we need to wait a moment for the io completion, current codes onlyfind_data_page()follows the rule, other places missing this step, so add it.Further more, moving the PageUptodate check into
f2fs_readpage()to clean up the codes.Signed-off-by Gu Zheng <[email protected]>--- fs/f2fs/checkpoint.c | 1 - fs/f2fs/data.c | 39 +++++++++++++++++---------------------- fs/f2fs/node.c | 1 - fs/f2fs/recovery.c | 2 -- fs/f2fs/segment.c | 2 +- 5 files changed, 18 insertions(+), 27 deletions(-)fs/f2fs/checkpoint.c +0
-1@@ -64,7 +64,6 @@ repeat:if (f2fs_readpage (sbi, page, index,READ_SYNC )) goto repeat; -lock_page (page);if (page->mapping != mapping) {f2fs_put_page (page,1 ); goto repeat;fs/f2fs/data.c +17
-22@@ -216,13 +216,11 @@ struct page *find_data_page(struct inode *inode, pgoff_t index, bool sync) err =f2fs_readpage (sbi, page, dn.data_blkaddr, sync ?READ_SYNC :READA ); -if (sync) { -wait_on_page_locked (page); -if (!PageUptodate (page)) { -f2fs_put_page (page,0 ); -return ERR_PTR (-EIO ); - } - } +if (err) +return ERR_PTR (err); + +if (sync) +unlock_page (page);return page; }@@ -267,11 +265,6 @@ repeat:if (err)return ERR_PTR (err); -lock_page (page); -if (!PageUptodate (page)) { -f2fs_put_page (page,1 ); -return ERR_PTR (-EIO ); - }if (page->mapping != mapping) {f2fs_put_page (page,1 ); goto repeat;@@ -325,11 +318,7 @@ repeat: err =f2fs_readpage (sbi, page, dn.data_blkaddr,READ_SYNC );if (err)return ERR_PTR (err); -lock_page (page); -if (!PageUptodate (page)) { -f2fs_put_page (page,1 ); -return ERR_PTR (-EIO ); - } +if (page->mapping != mapping) {f2fs_put_page (page,1 ); goto repeat;@@ -399,6 +388,16 @@ int f2fs_readpage(struct f2fs_sb_info *sbi, struct page *page,submit_bio (type, bio);up_read (&sbi->bio_sem); + +if (type ==READ_SYNC ) { +wait_on_page_locked (page); +lock_page (page); +if (!PageUptodate (page)) { +f2fs_put_page (page,1 ); +return -EIO ; + } + } +return 0 ; }@@ -679,11 +678,7 @@ repeat: err =f2fs_readpage (sbi, page, dn.data_blkaddr,READ_SYNC );if (err)return err; -lock_page (page); -if (!PageUptodate (page)) { -f2fs_put_page (page,1 ); -return -EIO ; - } +if (page->mapping != mapping) {f2fs_put_page (page,1 ); goto repeat;fs/f2fs/node.c +0
-1@@ -1534,7 +1534,6 @@ int restore_node_summary(struct f2fs_sb_info *sbi,if (f2fs_readpage (sbi, page, addr,READ_SYNC )) goto out; -lock_page (page); rn =F2FS_NODE (page); sum_entry->nid = rn->footer.nid; sum_entry->version =0 ;fs/f2fs/recovery.c +0
-2@@ -140,8 +140,6 @@ static int find_fsync_dnodes(struct f2fs_sb_info *sbi, struct list_head *head)if (err) goto out; -lock_page (page); -if (cp_ver !=cpver_of_node (page))break ;fs/f2fs/segment.c +0
-0@@ -639,7 +639,7 @@ static void do_submit_bio(struct f2fs_sb_info *sbi,
Quoted 8 lines
fs/f2fs/segment.c static void do_submit_bio(struct f2fs_sb_info *sbi, · line 645
Quoted 7 lines
fs/f2fs/segment.c +1
-1trace_f2fs_do_submit_bio (sbi->sb, btype, sync, sbi->bio[btype]); -if (type ==META_FLUSH ) { +if ((type ==META_FLUSH ) || (rw &WRITE_SYNC )) {DECLARE_COMPLETION_ONSTACK (wait); p->is_sync = true; p->wait = &wait;
------------------------------------------------------------------------------
Get your SQL database under version control now!
Version control is standard for application code, but databases havent caught up. So what steps can you take to put your SQL databases under version control? Why should you start doing it? Read more to find out.
http://pubads.g.doubleclick.net/gampad/clk?id=49501711&iu=/4140/ostg.clktrk
_______________________________________________
Linux-f2fs-devel mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
2013-07-31 (수), 09:59 +0800, Gu Zheng:
Quoted 10 lines
Hi Kim,
Quoted 3 lines · Jaegeuk Kim
Hi Gu,
The original read flow was to avoid redandunt lock/unlock_page() calls.
Right, this can gain better read performance. But is the wait step after submitting bio with
READ_SYNCneedless too?
Correct, the READ_SYNC is also used for IO priority.
The basic read policy here is that the caller should lock the page only when it wants to manipulate there-in data.
Otherwise, we don't need to unnecessary lock and unlocks. Thanks,
Quoted 163 lines
Quoted 3 lines
And we should not wait for
WRITE_SYNC, since it is just for write priority, not for synchronization of the file system.Got it, thanks for your explanation.:)
Best regards,
GuQuoted 140 lines
Thanks,
2013-07-30 (화), 18:06 +0800, Gu Zheng:
Quoted 137 lines
When we submit bio with
READ_SYNCorWRITE_SYNC, we need to wait a moment for the io completion, current codes onlyfind_data_page()follows the rule, other places missing this step, so add it.Further more, moving the PageUptodate check into
f2fs_readpage()to clean up the codes.Signed-off-by Gu Zheng <[email protected]>--- fs/f2fs/checkpoint.c | 1 - fs/f2fs/data.c | 39 +++++++++++++++++---------------------- fs/f2fs/node.c | 1 - fs/f2fs/recovery.c | 2 -- fs/f2fs/segment.c | 2 +- 5 files changed, 18 insertions(+), 27 deletions(-)fs/f2fs/checkpoint.c +0
-1@@ -64,7 +64,6 @@ repeat:if (f2fs_readpage (sbi, page, index,READ_SYNC )) goto repeat; -lock_page (page);if (page->mapping != mapping) {f2fs_put_page (page,1 ); goto repeat;fs/f2fs/data.c +17
-22@@ -216,13 +216,11 @@ struct page *find_data_page(struct inode *inode, pgoff_t index, bool sync) err =f2fs_readpage (sbi, page, dn.data_blkaddr, sync ?READ_SYNC :READA ); -if (sync) { -wait_on_page_locked (page); -if (!PageUptodate (page)) { -f2fs_put_page (page,0 ); -return ERR_PTR (-EIO ); - } - } +if (err) +return ERR_PTR (err); + +if (sync) +unlock_page (page);return page; }@@ -267,11 +265,6 @@ repeat:if (err)return ERR_PTR (err); -lock_page (page); -if (!PageUptodate (page)) { -f2fs_put_page (page,1 ); -return ERR_PTR (-EIO ); - }if (page->mapping != mapping) {f2fs_put_page (page,1 ); goto repeat;@@ -325,11 +318,7 @@ repeat: err =f2fs_readpage (sbi, page, dn.data_blkaddr,READ_SYNC );if (err)return ERR_PTR (err); -lock_page (page); -if (!PageUptodate (page)) { -f2fs_put_page (page,1 ); -return ERR_PTR (-EIO ); - } +if (page->mapping != mapping) {f2fs_put_page (page,1 ); goto repeat;@@ -399,6 +388,16 @@ int f2fs_readpage(struct f2fs_sb_info *sbi, struct page *page,submit_bio (type, bio);up_read (&sbi->bio_sem); + +if (type ==READ_SYNC ) { +wait_on_page_locked (page); +lock_page (page); +if (!PageUptodate (page)) { +f2fs_put_page (page,1 ); +return -EIO ; + } + } +return 0 ; }@@ -679,11 +678,7 @@ repeat: err =f2fs_readpage (sbi, page, dn.data_blkaddr,READ_SYNC );if (err)return err; -lock_page (page); -if (!PageUptodate (page)) { -f2fs_put_page (page,1 ); -return -EIO ; - } +if (page->mapping != mapping) {f2fs_put_page (page,1 ); goto repeat;fs/f2fs/node.c +0
-1@@ -1534,7 +1534,6 @@ int restore_node_summary(struct f2fs_sb_info *sbi,if (f2fs_readpage (sbi, page, addr,READ_SYNC )) goto out; -lock_page (page); rn =F2FS_NODE (page); sum_entry->nid = rn->footer.nid; sum_entry->version =0 ;fs/f2fs/recovery.c +0
-2@@ -140,8 +140,6 @@ static int find_fsync_dnodes(struct f2fs_sb_info *sbi, struct list_head *head)if (err) goto out; -lock_page (page); -if (cp_ver !=cpver_of_node (page))break ;fs/f2fs/segment.c +0
-0@@ -639,7 +639,7 @@ static void do_submit_bio(struct f2fs_sb_info *sbi,Quoted 8 lines
fs/f2fs/segment.c static void do_submit_bio(struct f2fs_sb_info *sbi, · line 645
Quoted 7 lines
fs/f2fs/segment.c +1
-1trace_f2fs_do_submit_bio (sbi->sb, btype, sync, sbi->bio[btype]); -if (type ==META_FLUSH ) { +if ((type ==META_FLUSH ) || (rw &WRITE_SYNC )) {DECLARE_COMPLETION_ONSTACK (wait); p->is_sync = true; p->wait = &wait;--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Signature · 7 lines
Jaegeuk Kim Samsung -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoted 9 lines · Jaegeuk Kim
2013-07-31 (수), 09:59 +0800, Gu Zheng:
Quoted 8 lines
Hi Kim,
Quoted 3 lines · Jaegeuk Kim
Hi Gu,
The original read flow was to avoid redandunt lock/unlock_page() calls.
Quoted 8 lines
Quoted 2 lines
Right, this can gain better read performance. But is the wait step after submitting bio with
READ_SYNCneedless too?Correct, the
READ_SYNCis also used for IO priority.
The basic read policy here is that the caller should lock the page only when it wants to manipulate there-in data.Otherwise, we don't need to unnecessary lock and unlocks.
Got it, it seems that I had some miss reading originally, it's clear now, thanks very much for your explanation.:)
Regards,
Gu
Quoted 167 lines
Thanks,
Quoted 163 lines
Quoted 3 lines
And we should not wait for
WRITE_SYNC, since it is just for write priority, not for synchronization of the file system.Got it, thanks for your explanation.:)
Best regards,
GuQuoted 140 lines
Thanks,
2013-07-30 (화), 18:06 +0800, Gu Zheng:
Quoted 137 lines
When we submit bio with
READ_SYNCorWRITE_SYNC, we need to wait a moment for the io completion, current codes onlyfind_data_page()follows the rule, other places missing this step, so add it.Further more, moving the PageUptodate check into
f2fs_readpage()to clean up the codes.Signed-off-by Gu Zheng <[email protected]>--- fs/f2fs/checkpoint.c | 1 - fs/f2fs/data.c | 39 +++++++++++++++++---------------------- fs/f2fs/node.c | 1 - fs/f2fs/recovery.c | 2 -- fs/f2fs/segment.c | 2 +- 5 files changed, 18 insertions(+), 27 deletions(-)fs/f2fs/checkpoint.c +0
-1@@ -64,7 +64,6 @@ repeat:if (f2fs_readpage (sbi, page, index,READ_SYNC )) goto repeat; -lock_page (page);if (page->mapping != mapping) {f2fs_put_page (page,1 ); goto repeat;fs/f2fs/data.c +17
-22@@ -216,13 +216,11 @@ struct page *find_data_page(struct inode *inode, pgoff_t index, bool sync) err =f2fs_readpage (sbi, page, dn.data_blkaddr, sync ?READ_SYNC :READA ); -if (sync) { -wait_on_page_locked (page); -if (!PageUptodate (page)) { -f2fs_put_page (page,0 ); -return ERR_PTR (-EIO ); - } - } +if (err) +return ERR_PTR (err); + +if (sync) +unlock_page (page);return page; }@@ -267,11 +265,6 @@ repeat:if (err)return ERR_PTR (err); -lock_page (page); -if (!PageUptodate (page)) { -f2fs_put_page (page,1 ); -return ERR_PTR (-EIO ); - }if (page->mapping != mapping) {f2fs_put_page (page,1 ); goto repeat;@@ -325,11 +318,7 @@ repeat: err =f2fs_readpage (sbi, page, dn.data_blkaddr,READ_SYNC );if (err)return ERR_PTR (err); -lock_page (page); -if (!PageUptodate (page)) { -f2fs_put_page (page,1 ); -return ERR_PTR (-EIO ); - } +if (page->mapping != mapping) {f2fs_put_page (page,1 ); goto repeat;@@ -399,6 +388,16 @@ int f2fs_readpage(struct f2fs_sb_info *sbi, struct page *page,submit_bio (type, bio);up_read (&sbi->bio_sem); + +if (type ==READ_SYNC ) { +wait_on_page_locked (page); +lock_page (page); +if (!PageUptodate (page)) { +f2fs_put_page (page,1 ); +return -EIO ; + } + } +return 0 ; }@@ -679,11 +678,7 @@ repeat: err =f2fs_readpage (sbi, page, dn.data_blkaddr,READ_SYNC );if (err)return err; -lock_page (page); -if (!PageUptodate (page)) { -f2fs_put_page (page,1 ); -return -EIO ; - } +if (page->mapping != mapping) {f2fs_put_page (page,1 ); goto repeat;fs/f2fs/node.c +0
-1@@ -1534,7 +1534,6 @@ int restore_node_summary(struct f2fs_sb_info *sbi,if (f2fs_readpage (sbi, page, addr,READ_SYNC )) goto out; -lock_page (page); rn =F2FS_NODE (page); sum_entry->nid = rn->footer.nid; sum_entry->version =0 ;fs/f2fs/recovery.c +0
-2@@ -140,8 +140,6 @@ static int find_fsync_dnodes(struct f2fs_sb_info *sbi, struct list_head *head)if (err) goto out; -lock_page (page); -if (cp_ver !=cpver_of_node (page))break ;fs/f2fs/segment.c +0
-0@@ -639,7 +639,7 @@ static void do_submit_bio(struct f2fs_sb_info *sbi,Quoted 8 lines
fs/f2fs/segment.c static void do_submit_bio(struct f2fs_sb_info *sbi, · line 645
Quoted 7 lines
fs/f2fs/segment.c +1
-1trace_f2fs_do_submit_bio (sbi->sb, btype, sync, sbi->bio[btype]); -if (type ==META_FLUSH ) { +if ((type ==META_FLUSH ) || (rw &WRITE_SYNC )) {DECLARE_COMPLETION_ONSTACK (wait); p->is_sync = true; p->wait = &wait;--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html