f2fs: avoid crash when trace f2fs_submit_page_mbio event in ra_sum_pages
Subsystems (2)
PATCH v2 1/1 f2fs: avoid crash when trace f2fs_submit_page_mbio event in ra_sum_pages
Previously we allocate pages with no mapping in ra_sum_pages(), so we may encounter a crash in event trace of f2fs_submit_page_mbio where we access mapping data of the page.
We'd better allocate pages in bd_inode mapping and invalidate these pages after we restore data from pages. It could avoid crash in above scenario.
Changes from V1
o remove redundant code in ra_sum_pages() suggested by Jaegeuk Kim.
Call Trace: [<f1031630>] ? ftrace_raw_event_f2fs_write_checkpoint+0x80/0x80 [f2fs] [<f10377bb>] f2fs_submit_page_mbio+0x1cb/0x200 [f2fs] [<f103c5da>] restore_node_summary+0x13a/0x280 [f2fs] [<f103e22d>] build_curseg+0x2bd/0x620 [f2fs] [<f104043b>] build_segment_manager+0x1cb/0x920 [f2fs] [<f1032c85>] f2fs_fill_super+0x535/0x8e0 [f2fs] [<c115b66a>] mount_bdev+0x16a/0x1a0 [<f102f63f>] f2fs_mount+0x1f/0x30 [f2fs] [<c115c096>] mount_fs+0x36/0x170 [<c1173635>] vfs_kern_mount+0x55/0xe0 [<c1175388>] do_mount+0x1e8/0x900 [<c1175d72>] SyS_mount+0x82/0xc0 [<c16059cc>] sysenter_do_call+0x12/0x22
--- fs/f2fs/node.c | 52 ++++++++++++++++++++++++---------------------------- 1 file changed, 24 insertions(+), 28 deletions(-)
fs/f2fs/node.c +24 -28
@@ -1658,35 +1658,29 @@ int recover_inode_page(struct f2fs_sb_info *sbi, struct page *page)/* * ra_sum_pages() merge contiguous pages into one bio and submit. -* these pre-readed pages are linked in pages list. +* these pre-readed pages are alloced in bd_inode's mapping tree. */ -static int ra_sum_pages (struct f2fs_sb_info *sbi,struct list_head *pages, +static int ra_sum_pages (struct f2fs_sb_info *sbi,struct page **pages,int start,int nrpages) { -struct page *page; -int page_idx = start; +struct inode *inode = sbi->sb->s_bdev->bd_inode; +struct address_space *mapping = inode->i_mapping; +int i, page_idx = start;struct f2fs_io_info fio = { .type =META , .rw =READ_SYNC |REQ_META |REQ_PRIO }; -for (; page_idx < start + nrpages; page_idx++) { -/* alloc temporal page for read node summary info*/ - page =alloc_page (GFP_F2FS_ZERO ); -if (!page) +for (i =0 ; page_idx < start + nrpages; page_idx++, i++) { +/* alloc page in bd_inode for reading node summary info */ + pages[i] =grab_cache_page (mapping, page_idx); +if (!pages[i])break ; - -lock_page (page); - page->index = page_idx; -list_add_tail (&page->lru, pages); +f2fs_submit_page_mbio (sbi, pages[i], page_idx, &fio); } -list_for_each_entry (page, pages, lru) -f2fs_submit_page_mbio (sbi, page, page->index, &fio); -f2fs_submit_merged_bio (sbi,META ,READ ); - -return page_idx - start; +return i; } int restore_node_summary(struct f2fs_sb_info *sbi,
@@ -1694,11 +1688,11 @@ int restore_node_summary(struct f2fs_sb_info *sbi, {struct f2fs_node *rn;struct f2fs_summary *sum_entry; -struct page *page, *tmp; +struct inode *inode = sbi->sb->s_bdev->bd_inode;block_t addr;int bio_blocks =MAX_BIO_BLOCKS (max_hw_blocks (sbi)); -int i, last_offset, nrpages, err =0 ; -LIST_HEAD (page_list); +struct page *pages[bio_blocks]; +int i, idx, last_offset, nrpages, err =0 ;/* scan the node segment */ last_offset = sbi->blocks_per_seg;
@@ -1709,29 +1703,31 @@ int restore_node_summary(struct f2fs_sb_info *sbi, nrpages =min (last_offset - i, bio_blocks);/* read ahead node pages */ - nrpages =ra_sum_pages (sbi, &page_list, addr, nrpages); + nrpages =ra_sum_pages (sbi, pages, addr, nrpages);if (!nrpages)return -ENOMEM ; -list_for_each_entry_safe (page, tmp, &page_list, lru) { +for (idx =0 ; idx < nrpages; idx++) {if (err) goto skip; -lock_page (page); -if (unlikely (!PageUptodate (page))) { +lock_page (pages[idx]); +if (unlikely (!PageUptodate (pages[idx]))) { err = -EIO ; }else { - rn =F2FS_NODE (page); + rn =F2FS_NODE (pages[idx]); sum_entry->nid = rn->footer.nid; sum_entry->version =0 ; sum_entry->ofs_in_node =0 ; sum_entry++; } -unlock_page (page); +unlock_page (pages[idx]); skip: -list_del (&page->lru); -__free_pages (page,0 ); +page_cache_release (pages[idx]); } + +invalidate_mapping_pages (inode->i_mapping, addr, + addr + nrpages); }return err; }
Signature · 1 line
1.7.9.5
Hi, Chao
Could you think about following once.
move node_inode in front of build_segment_manager, then use node_inode instead of bd_inode.
Quoted 152 lines · Chao Yu
Previously we allocate pages with no mapping in
ra_sum_pages(), so we may encounter a crash in event trace off2fs_submit_page_mbiowhere we access mapping data of the page.We'd better allocate pages in
bd_inodemapping and invalidate these pages after we restore data from pages. It could avoid crash in above scenario.Changes from V1
o remove redundant code inra_sum_pages()suggested by Jaegeuk Kim.Call Trace: [<f1031630>] ? ftrace_raw_event_f2fs_write_checkpoint+0x80/0x80 [f2fs] [<f10377bb>] f2fs_submit_page_mbio+0x1cb/0x200 [f2fs] [<f103c5da>] restore_node_summary+0x13a/0x280 [f2fs] [<f103e22d>] build_curseg+0x2bd/0x620 [f2fs] [<f104043b>] build_segment_manager+0x1cb/0x920 [f2fs] [<f1032c85>] f2fs_fill_super+0x535/0x8e0 [f2fs] [<c115b66a>] mount_bdev+0x16a/0x1a0 [<f102f63f>] f2fs_mount+0x1f/0x30 [f2fs] [<c115c096>] mount_fs+0x36/0x170 [<c1173635>] vfs_kern_mount+0x55/0xe0 [<c1175388>] do_mount+0x1e8/0x900 [<c1175d72>] SyS_mount+0x82/0xc0 [<c16059cc>] sysenter_do_call+0x12/0x22Suggested-by Jaegeuk Kim <[email protected]>Signed-off-by Chao Yu <[email protected]>--- fs/f2fs/node.c | 52 ++++++++++++++++++++++++---------------------------- 1 file changed, 24 insertions(+), 28 deletions(-)fs/f2fs/node.c +24
-28@@ -1658,35 +1658,29 @@ int recover_inode_page(struct f2fs_sb_info *sbi, struct page *page)/* * ra_sum_pages() merge contiguous pages into one bio and submit. -* these pre-readed pages are linked in pages list. +* these pre-readed pages are alloced in bd_inode's mapping tree. */ -static int ra_sum_pages (struct f2fs_sb_info *sbi,struct list_head *pages, +static int ra_sum_pages (struct f2fs_sb_info *sbi,struct page **pages,int start,int nrpages) { -struct page *page; -int page_idx = start; +struct inode *inode = sbi->sb->s_bdev->bd_inode; +struct address_space *mapping = inode->i_mapping; +int i, page_idx = start;struct f2fs_io_info fio = { .type =META , .rw =READ_SYNC |REQ_META |REQ_PRIO }; -for (; page_idx < start + nrpages; page_idx++) { -/* alloc temporal page for read node summary info*/ - page =alloc_page (GFP_F2FS_ZERO ); -if (!page) +for (i =0 ; page_idx < start + nrpages; page_idx++, i++) { +/* alloc page in bd_inode for reading node summary info */ + pages[i] =grab_cache_page (mapping, page_idx); +if (!pages[i])break ; - -lock_page (page); - page->index = page_idx; -list_add_tail (&page->lru, pages); +f2fs_submit_page_mbio (sbi, pages[i], page_idx, &fio); } -list_for_each_entry (page, pages, lru) -f2fs_submit_page_mbio (sbi, page, page->index, &fio); -f2fs_submit_merged_bio (sbi,META ,READ ); - -return page_idx - start; +return i; } int restore_node_summary(struct f2fs_sb_info *sbi,@@ -1694,11 +1688,11 @@ int restore_node_summary(struct f2fs_sb_info *sbi, {struct f2fs_node *rn;struct f2fs_summary *sum_entry; -struct page *page, *tmp; +struct inode *inode = sbi->sb->s_bdev->bd_inode;block_t addr;int bio_blocks =MAX_BIO_BLOCKS (max_hw_blocks (sbi)); -int i, last_offset, nrpages, err =0 ; -LIST_HEAD (page_list); +struct page *pages[bio_blocks]; +int i, idx, last_offset, nrpages, err =0 ;/* scan the node segment */ last_offset = sbi->blocks_per_seg;@@ -1709,29 +1703,31 @@ int restore_node_summary(struct f2fs_sb_info *sbi, nrpages =min (last_offset - i, bio_blocks);/* read ahead node pages */ - nrpages =ra_sum_pages (sbi, &page_list, addr, nrpages); + nrpages =ra_sum_pages (sbi, pages, addr, nrpages);if (!nrpages)return -ENOMEM ; -list_for_each_entry_safe (page, tmp, &page_list, lru) { +for (idx =0 ; idx < nrpages; idx++) {if (err) goto skip; -lock_page (page); -if (unlikely (!PageUptodate (page))) { +lock_page (pages[idx]); +if (unlikely (!PageUptodate (pages[idx]))) { err = -EIO ; }else { - rn =F2FS_NODE (page); + rn =F2FS_NODE (pages[idx]); sum_entry->nid = rn->footer.nid; sum_entry->version =0 ; sum_entry->ofs_in_node =0 ; sum_entry++; } -unlock_page (page); +unlock_page (pages[idx]); skip: -list_del (&page->lru); -__free_pages (page,0 ); +page_cache_release (pages[idx]); } + +invalidate_mapping_pages (inode->i_mapping, addr, + addr + nrpages); }return err; }Signature · 14 lines
1.7.9.5 ------------------------------------------------------------------------------ The best possible search technologies are now affordable for all companies. Download your FREE open source Enterprise Search Engine today! Our experts will assist you in its installation for $59/mo, no commitment. Test it for FREE on our Cloud platform anytime! http://pubads.g.doubleclick.net/gampad/clk?id=145328191&iu=/4140/ostg.clktrk _______________________________________________ Linux-f2fs-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
------------------------------------------------------------------------------
The best possible search technologies are now affordable for all companies. Download your FREE open source Enterprise Search Engine today! Our experts will assist you in its installation for $59/mo, no commitment. Test it for FREE on our Cloud platform anytime!
http://pubads.g.doubleclick.net/gampad/clk?id=145328191&iu=/4140/ostg.clktrk
Hi changman,
Quoted 9 lines
-----Original Message-----
From: Changman Lee [mailto:[email protected]]
Sent: Tuesday, May 27, 2014 9:25 AM
To: Chao Yu
Cc: Jaegeuk Kim; [email protected]; [email protected]; [email protected]
Subject: Re: [f2fs-dev] [PATCH v2] f2fs: avoid crash when tracef2fs_submit_page_mbioevent inra_sum_pages
Quoted 5 lines
Hi, Chao
Could you think about following once.
movenode_inodein front ofbuild_segment_manager, then usenode_inodeinstead ofbd_inode.
Jaegeuk and I discussed this solution previously in
[PATCH 3/3 V3] f2fs: introduce f2fs_cache_node_page() to add page into node_inode cache
You can see it from this url:
http://sourceforge.net/p/linux-f2fs/mailman/linux-f2fs-devel/?viewmonth=201312&page=5
And it seems not easy to change order of build_*_manager and make node_inode, because there are dependency between them.
Quoted 144 lines · Chao Yu
Quoted 142 lines
Previously we allocate pages with no mapping in
ra_sum_pages(), so we may encounter a crash in event trace off2fs_submit_page_mbiowhere we access mapping data of the page.We'd better allocate pages in
bd_inodemapping and invalidate these pages after we restore data from pages. It could avoid crash in above scenario.Changes from V1
o remove redundant code inra_sum_pages()suggested by Jaegeuk Kim.Call Trace: [<f1031630>] ? ftrace_raw_event_f2fs_write_checkpoint+0x80/0x80 [f2fs] [<f10377bb>] f2fs_submit_page_mbio+0x1cb/0x200 [f2fs] [<f103c5da>] restore_node_summary+0x13a/0x280 [f2fs] [<f103e22d>] build_curseg+0x2bd/0x620 [f2fs] [<f104043b>] build_segment_manager+0x1cb/0x920 [f2fs] [<f1032c85>] f2fs_fill_super+0x535/0x8e0 [f2fs] [<c115b66a>] mount_bdev+0x16a/0x1a0 [<f102f63f>] f2fs_mount+0x1f/0x30 [f2fs] [<c115c096>] mount_fs+0x36/0x170 [<c1173635>] vfs_kern_mount+0x55/0xe0 [<c1175388>] do_mount+0x1e8/0x900 [<c1175d72>] SyS_mount+0x82/0xc0 [<c16059cc>] sysenter_do_call+0x12/0x22Suggested-by Jaegeuk Kim <[email protected]>Signed-off-by Chao Yu <[email protected]>--- fs/f2fs/node.c | 52 ++++++++++++++++++++++++---------------------------- 1 file changed, 24 insertions(+), 28 deletions(-)fs/f2fs/node.c +24
-29@@ -1658,35 +1658,29 @@ int recover_inode_page(struct f2fs_sb_info *sbi, struct page *page)/* * ra_sum_pages() merge contiguous pages into one bio and submit. -* these pre-readed pages are linked in pages list. +* these pre-readed pages are alloced in bd_inode's mapping tree. */ -static int ra_sum_pages (struct f2fs_sb_info *sbi,struct list_head *pages, +static int ra_sum_pages (struct f2fs_sb_info *sbi,struct page **pages,int start,int nrpages) { -struct page *page; -int page_idx = start; +struct inode *inode = sbi->sb->s_bdev->bd_inode; +struct address_space *mapping = inode->i_mapping; +int i, page_idx = start;struct f2fs_io_info fio = { .type =META , .rw =READ_SYNC |REQ_META |REQ_PRIO }; -for (; page_idx < start + nrpages; page_idx++) { -/* alloc temporal page for read node summary info*/ - page =alloc_page (GFP_F2FS_ZERO ); -if (!page) +for (i =0 ; page_idx < start + nrpages; page_idx++, i++) { +/* alloc page in bd_inode for reading node summary info */ + pages[i] =grab_cache_page (mapping, page_idx); +if (!pages[i])break ; - -lock_page (page); - page->index = page_idx; -list_add_tail (&page->lru, pages); +f2fs_submit_page_mbio (sbi, pages[i], page_idx, &fio); } -list_for_each_entry (page, pages, lru) -f2fs_submit_page_mbio (sbi, page, page->index, &fio); -f2fs_submit_merged_bio (sbi,META ,READ ); - -return page_idx - start; +return i; } int restore_node_summary(struct f2fs_sb_info *sbi,@@ -1694,11 +1688,11 @@ int restore_node_summary(struct f2fs_sb_info *sbi, {struct f2fs_node *rn;struct f2fs_summary *sum_entry; -struct page *page, *tmp; +struct inode *inode = sbi->sb->s_bdev->bd_inode;block_t addr;int bio_blocks =MAX_BIO_BLOCKS (max_hw_blocks (sbi)); -int i, last_offset, nrpages, err =0 ; -LIST_HEAD (page_list); +struct page *pages[bio_blocks]; +int i, idx, last_offset, nrpages, err =0 ;/* scan the node segment */ last_offset = sbi->blocks_per_seg;@@ -1709,29 +1703,31 @@ int restore_node_summary(struct f2fs_sb_info *sbi, nrpages =min (last_offset - i, bio_blocks);/* read ahead node pages */ - nrpages =ra_sum_pages (sbi, &page_list, addr, nrpages); + nrpages =ra_sum_pages (sbi, pages, addr, nrpages);if (!nrpages)return -ENOMEM ; -list_for_each_entry_safe (page, tmp, &page_list, lru) { +for (idx =0 ; idx < nrpages; idx++) {if (err) goto skip; -lock_page (page); -if (unlikely (!PageUptodate (page))) { +lock_page (pages[idx]); +if (unlikely (!PageUptodate (pages[idx]))) { err = -EIO ; }else { - rn =F2FS_NODE (page); + rn =F2FS_NODE (pages[idx]); sum_entry->nid = rn->footer.nid; sum_entry->version =0 ; sum_entry->ofs_in_node =0 ; sum_entry++; } -unlock_page (page); +unlock_page (pages[idx]); skip: -list_del (&page->lru); -__free_pages (page,0 ); +page_cache_release (pages[idx]); } + +invalidate_mapping_pages (inode->i_mapping, addr, + addr + nrpages); }return err; } --1.7.9.5
Quoted 10 lines
Quoted 10 lines
------------------------------------------------------------------------------
The best possible search technologies are now affordable for all companies. Download your FREE open source Enterprise Search Engine today! Our experts will assist you in its installation for $59/mo, no commitment. Test it for FREE on our Cloud platform anytime!
http://pubads.g.doubleclick.net/gampad/clk?id=145328191&iu=/4140/ostg.clktrk
_______________________________________________
Linux-f2fs-devel mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
------------------------------------------------------------------------------
The best possible search technologies are now affordable for all companies. Download your FREE open source Enterprise Search Engine today! Our experts will assist you in its installation for $59/mo, no commitment. Test it for FREE on our Cloud platform anytime!
http://pubads.g.doubleclick.net/gampad/clk?id=145328191&iu=/4140/ostg.clktrk
Quoted 17 lines · Chao Yu
Hi changman,
Quoted 9 lines
-----Original Message-----
From: Changman Lee [mailto:[email protected]]
Sent: Tuesday, May 27, 2014 9:25 AM
To: Chao Yu
Cc: Jaegeuk Kim; [email protected]; [email protected]; [email protected]
Subject: Re: [f2fs-dev] [PATCH v2] f2fs: avoid crash when tracef2fs_submit_page_mbioevent inra_sum_pagesQuoted 5 lines
Hi, Chao
Could you think about following once.
movenode_inodein front ofbuild_segment_manager, then usenode_inodeinstead ofbd_inode.
Quoted 9 lines
Jaegeuk and I discussed this solution previously in
[PATCH 3/3 V3] f2fs: introducef2fs_cache_node_page()to add page intonode_inodecacheYou can see it from this url:
http://sourceforge.net/p/linux-f2fs/mailman/linux-f2fs-devel/?viewmonth=201312&page=5And it seems not easy to change order of build_*_manager and make
node_inode, because there are dependency between them.
Sorry to make a mess your patch thread.
I've understood it. In your patch, using NAT journal seems to be possible. Anyway, thanks for your answer.
Quoted 144 lines
Quoted 144 lines · Chao Yu
Quoted 142 lines
Previously we allocate pages with no mapping in
ra_sum_pages(), so we may encounter a crash in event trace off2fs_submit_page_mbiowhere we access mapping data of the page.We'd better allocate pages in
bd_inodemapping and invalidate these pages after we restore data from pages. It could avoid crash in above scenario.Changes from V1
o remove redundant code inra_sum_pages()suggested by Jaegeuk Kim.Call Trace: [<f1031630>] ? ftrace_raw_event_f2fs_write_checkpoint+0x80/0x80 [f2fs] [<f10377bb>] f2fs_submit_page_mbio+0x1cb/0x200 [f2fs] [<f103c5da>] restore_node_summary+0x13a/0x280 [f2fs] [<f103e22d>] build_curseg+0x2bd/0x620 [f2fs] [<f104043b>] build_segment_manager+0x1cb/0x920 [f2fs] [<f1032c85>] f2fs_fill_super+0x535/0x8e0 [f2fs] [<c115b66a>] mount_bdev+0x16a/0x1a0 [<f102f63f>] f2fs_mount+0x1f/0x30 [f2fs] [<c115c096>] mount_fs+0x36/0x170 [<c1173635>] vfs_kern_mount+0x55/0xe0 [<c1175388>] do_mount+0x1e8/0x900 [<c1175d72>] SyS_mount+0x82/0xc0 [<c16059cc>] sysenter_do_call+0x12/0x22Suggested-by Jaegeuk Kim <[email protected]>Signed-off-by Chao Yu <[email protected]>--- fs/f2fs/node.c | 52 ++++++++++++++++++++++++---------------------------- 1 file changed, 24 insertions(+), 28 deletions(-)fs/f2fs/node.c +24
-29@@ -1658,35 +1658,29 @@ int recover_inode_page(struct f2fs_sb_info *sbi, struct page *page)/* * ra_sum_pages() merge contiguous pages into one bio and submit. -* these pre-readed pages are linked in pages list. +* these pre-readed pages are alloced in bd_inode's mapping tree. */ -static int ra_sum_pages (struct f2fs_sb_info *sbi,struct list_head *pages, +static int ra_sum_pages (struct f2fs_sb_info *sbi,struct page **pages,int start,int nrpages) { -struct page *page; -int page_idx = start; +struct inode *inode = sbi->sb->s_bdev->bd_inode; +struct address_space *mapping = inode->i_mapping; +int i, page_idx = start;struct f2fs_io_info fio = { .type =META , .rw =READ_SYNC |REQ_META |REQ_PRIO }; -for (; page_idx < start + nrpages; page_idx++) { -/* alloc temporal page for read node summary info*/ - page =alloc_page (GFP_F2FS_ZERO ); -if (!page) +for (i =0 ; page_idx < start + nrpages; page_idx++, i++) { +/* alloc page in bd_inode for reading node summary info */ + pages[i] =grab_cache_page (mapping, page_idx); +if (!pages[i])break ; - -lock_page (page); - page->index = page_idx; -list_add_tail (&page->lru, pages); +f2fs_submit_page_mbio (sbi, pages[i], page_idx, &fio); } -list_for_each_entry (page, pages, lru) -f2fs_submit_page_mbio (sbi, page, page->index, &fio); -f2fs_submit_merged_bio (sbi,META ,READ ); - -return page_idx - start; +return i; } int restore_node_summary(struct f2fs_sb_info *sbi,@@ -1694,11 +1688,11 @@ int restore_node_summary(struct f2fs_sb_info *sbi, {struct f2fs_node *rn;struct f2fs_summary *sum_entry; -struct page *page, *tmp; +struct inode *inode = sbi->sb->s_bdev->bd_inode;block_t addr;int bio_blocks =MAX_BIO_BLOCKS (max_hw_blocks (sbi)); -int i, last_offset, nrpages, err =0 ; -LIST_HEAD (page_list); +struct page *pages[bio_blocks]; +int i, idx, last_offset, nrpages, err =0 ;/* scan the node segment */ last_offset = sbi->blocks_per_seg;@@ -1709,29 +1703,31 @@ int restore_node_summary(struct f2fs_sb_info *sbi, nrpages =min (last_offset - i, bio_blocks);/* read ahead node pages */ - nrpages =ra_sum_pages (sbi, &page_list, addr, nrpages); + nrpages =ra_sum_pages (sbi, pages, addr, nrpages);if (!nrpages)return -ENOMEM ; -list_for_each_entry_safe (page, tmp, &page_list, lru) { +for (idx =0 ; idx < nrpages; idx++) {if (err) goto skip; -lock_page (page); -if (unlikely (!PageUptodate (page))) { +lock_page (pages[idx]); +if (unlikely (!PageUptodate (pages[idx]))) { err = -EIO ; }else { - rn =F2FS_NODE (page); + rn =F2FS_NODE (pages[idx]); sum_entry->nid = rn->footer.nid; sum_entry->version =0 ; sum_entry->ofs_in_node =0 ; sum_entry++; } -unlock_page (page); +unlock_page (pages[idx]); skip: -list_del (&page->lru); -__free_pages (page,0 ); +page_cache_release (pages[idx]); } + +invalidate_mapping_pages (inode->i_mapping, addr, + addr + nrpages); }return err; } --1.7.9.5
Quoted 10 lines
Quoted 10 lines
Quoted 10 lines
------------------------------------------------------------------------------
The best possible search technologies are now affordable for all companies. Download your FREE open source Enterprise Search Engine today! Our experts will assist you in its installation for $59/mo, no commitment. Test it for FREE on our Cloud platform anytime!
http://pubads.g.doubleclick.net/gampad/clk?id=145328191&iu=/4140/ostg.clktrk
_______________________________________________
Linux-f2fs-devel mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
------------------------------------------------------------------------------
The best possible search technologies are now affordable for all companies. Download your FREE open source Enterprise Search Engine today! Our experts will assist you in its installation for $59/mo, no commitment. Test it for FREE on our Cloud platform anytime!
http://pubads.g.doubleclick.net/gampad/clk?id=145328191&iu=/4140/ostg.clktrk
Hi changman,
Quoted 30 lines
-----Original Message-----
From: Changman Lee [mailto:[email protected]]
Sent: Tuesday, May 27, 2014 3:21 PM
To: Chao Yu
Cc: 'Jaegeuk Kim'; [email protected]; [email protected]; [email protected]
Subject: Re: [f2fs-dev] [PATCH v2] f2fs: avoid crash when tracef2fs_submit_page_mbioevent inra_sum_pagesQuoted 20 lines · Chao Yu
Hi changman,
Quoted 9 lines
-----Original Message-----
From: Changman Lee [mailto:[email protected]]
Sent: Tuesday, May 27, 2014 9:25 AM
To: Chao Yu
Cc: Jaegeuk Kim; [email protected]; [email protected]; [email protected]
Subject: Re: [f2fs-dev] [PATCH v2] f2fs: avoid crash when tracef2fs_submit_page_mbioevent inra_sum_pagesQuoted 5 lines
Hi, Chao
Could you think about following once.
movenode_inodein front ofbuild_segment_manager, then usenode_inodeinstead ofbd_inode.Jaegeuk and I discussed this solution previously in
[PATCH 3/3 V3] f2fs: introducef2fs_cache_node_page()to add page intonode_inodecache
Quoted 8 lines
Quoted 6 lines
You can see it from this url:
http://sourceforge.net/p/linux-f2fs/mailman/linux-f2fs-devel/?viewmonth=201312&page=5And it seems not easy to change order of build_*_manager and make
node_inode, because there are dependency between them.Sorry to make a mess your patch thread.
It doesn't matter, I think any complaint and suggestion is welcomed here, and thanks for your review.
Quoted 2 lines
I've understood it. In your patch, using NAT journal seems to be possible. Anyway, thanks for your answer.
Maybe it could, but now we should discard that patch because previous implementation of ra_sum_pages() which the patch based could cause a crash.
Any problem founded here, please tell me, thanks. :)
Quoted 145 lines
Quoted 145 lines
Quoted 145 lines · Chao Yu
Quoted 142 lines
Previously we allocate pages with no mapping in
ra_sum_pages(), so we may encounter a crash in event trace off2fs_submit_page_mbiowhere we access mapping data of the page.We'd better allocate pages in
bd_inodemapping and invalidate these pages after we restore data from pages. It could avoid crash in above scenario.Changes from V1
o remove redundant code inra_sum_pages()suggested by Jaegeuk Kim.Call Trace: [<f1031630>] ? ftrace_raw_event_f2fs_write_checkpoint+0x80/0x80 [f2fs] [<f10377bb>] f2fs_submit_page_mbio+0x1cb/0x200 [f2fs] [<f103c5da>] restore_node_summary+0x13a/0x280 [f2fs] [<f103e22d>] build_curseg+0x2bd/0x620 [f2fs] [<f104043b>] build_segment_manager+0x1cb/0x920 [f2fs] [<f1032c85>] f2fs_fill_super+0x535/0x8e0 [f2fs] [<c115b66a>] mount_bdev+0x16a/0x1a0 [<f102f63f>] f2fs_mount+0x1f/0x30 [f2fs] [<c115c096>] mount_fs+0x36/0x170 [<c1173635>] vfs_kern_mount+0x55/0xe0 [<c1175388>] do_mount+0x1e8/0x900 [<c1175d72>] SyS_mount+0x82/0xc0 [<c16059cc>] sysenter_do_call+0x12/0x22Suggested-by Jaegeuk Kim <[email protected]>Signed-off-by Chao Yu <[email protected]>--- fs/f2fs/node.c | 52 ++++++++++++++++++++++++---------------------------- 1 file changed, 24 insertions(+), 28 deletions(-)fs/f2fs/node.c +24
-29@@ -1658,35 +1658,29 @@ int recover_inode_page(struct f2fs_sb_info *sbi, struct page *page)/* * ra_sum_pages() merge contiguous pages into one bio and submit. -* these pre-readed pages are linked in pages list. +* these pre-readed pages are alloced in bd_inode's mapping tree. */ -static int ra_sum_pages (struct f2fs_sb_info *sbi,struct list_head *pages, +static int ra_sum_pages (struct f2fs_sb_info *sbi,struct page **pages,int start,int nrpages) { -struct page *page; -int page_idx = start; +struct inode *inode = sbi->sb->s_bdev->bd_inode; +struct address_space *mapping = inode->i_mapping; +int i, page_idx = start;struct f2fs_io_info fio = { .type =META , .rw =READ_SYNC |REQ_META |REQ_PRIO }; -for (; page_idx < start + nrpages; page_idx++) { -/* alloc temporal page for read node summary info*/ - page =alloc_page (GFP_F2FS_ZERO ); -if (!page) +for (i =0 ; page_idx < start + nrpages; page_idx++, i++) { +/* alloc page in bd_inode for reading node summary info */ + pages[i] =grab_cache_page (mapping, page_idx); +if (!pages[i])break ; - -lock_page (page); - page->index = page_idx; -list_add_tail (&page->lru, pages); +f2fs_submit_page_mbio (sbi, pages[i], page_idx, &fio); } -list_for_each_entry (page, pages, lru) -f2fs_submit_page_mbio (sbi, page, page->index, &fio); -f2fs_submit_merged_bio (sbi,META ,READ ); - -return page_idx - start; +return i; } int restore_node_summary(struct f2fs_sb_info *sbi,@@ -1694,11 +1688,11 @@ int restore_node_summary(struct f2fs_sb_info *sbi, {struct f2fs_node *rn;struct f2fs_summary *sum_entry; -struct page *page, *tmp; +struct inode *inode = sbi->sb->s_bdev->bd_inode;block_t addr;int bio_blocks =MAX_BIO_BLOCKS (max_hw_blocks (sbi)); -int i, last_offset, nrpages, err =0 ; -LIST_HEAD (page_list); +struct page *pages[bio_blocks]; +int i, idx, last_offset, nrpages, err =0 ;/* scan the node segment */ last_offset = sbi->blocks_per_seg;@@ -1709,29 +1703,31 @@ int restore_node_summary(struct f2fs_sb_info *sbi, nrpages =min (last_offset - i, bio_blocks);/* read ahead node pages */ - nrpages =ra_sum_pages (sbi, &page_list, addr, nrpages); + nrpages =ra_sum_pages (sbi, pages, addr, nrpages);if (!nrpages)return -ENOMEM ; -list_for_each_entry_safe (page, tmp, &page_list, lru) { +for (idx =0 ; idx < nrpages; idx++) {if (err) goto skip; -lock_page (page); -if (unlikely (!PageUptodate (page))) { +lock_page (pages[idx]); +if (unlikely (!PageUptodate (pages[idx]))) { err = -EIO ; }else { - rn =F2FS_NODE (page); + rn =F2FS_NODE (pages[idx]); sum_entry->nid = rn->footer.nid; sum_entry->version =0 ; sum_entry->ofs_in_node =0 ; sum_entry++; } -unlock_page (page); +unlock_page (pages[idx]); skip: -list_del (&page->lru); -__free_pages (page,0 ); +page_cache_release (pages[idx]); } + +invalidate_mapping_pages (inode->i_mapping, addr, + addr + nrpages); }return err; } --1.7.9.5
Quoted 10 lines
Quoted 10 lines
Quoted 10 lines
Quoted 10 lines
------------------------------------------------------------------------------
The best possible search technologies are now affordable for all companies. Download your FREE open source Enterprise Search Engine today! Our experts will assist you in its installation for $59/mo, no commitment. Test it for FREE on our Cloud platform anytime!
http://pubads.g.doubleclick.net/gampad/clk?id=145328191&iu=/4140/ostg.clktrk
_______________________________________________
Linux-f2fs-devel mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
------------------------------------------------------------------------------
The best possible search technologies are now affordable for all companies. Download your FREE open source Enterprise Search Engine today! Our experts will assist you in its installation for $59/mo, no commitment. Test it for FREE on our Cloud platform anytime!
http://pubads.g.doubleclick.net/gampad/clk?id=145328191&iu=/4140/ostg.clktrk