f2fs: use ra_meta_pages to simplify readahead code in restore_node_summary
Subsystems (2)
PATCH v2 1/1 f2fs: use ra_meta_pages to simplify readahead code in restore_node_summary
Use more common function ra_meta_pages() with META_POR to readahead node blocks in restore_node_summary() instead of ra_sum_pages(), hence we can simplify the readahead code there, and also we can remove unused function ra_sum_pages().
changes from v1:
o fix one bug when using truncate_inode_pages_range which is pointed out by Jaegeuk Kim.
--- fs/f2fs/node.c | 68 +++++++++++++--------------------------------------------- 1 file changed, 15 insertions(+), 53 deletions(-)
fs/f2fs/node.c +15 -53
@@ -1726,80 +1726,42 @@ int recover_inode_page(struct f2fs_sb_info *sbi, struct page *page)return 0 ; } -/* -* ra_sum_pages() merge contiguous pages into one bio and submit. -* these pre-read pages are allocated in bd_inode's mapping tree. -*/ -static int ra_sum_pages (struct f2fs_sb_info *sbi,struct page **pages, -int start,int nrpages) -{ -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 (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 ; -f2fs_submit_page_mbio (sbi, pages[i], page_idx, &fio); - } - -f2fs_submit_merged_bio (sbi,META ,READ ); -return i; -} -int restore_node_summary (struct f2fs_sb_info *sbi,unsigned int segno,struct f2fs_summary_block *sum) {struct f2fs_node *rn;struct f2fs_summary *sum_entry; -struct inode *inode = sbi->sb->s_bdev->bd_inode;block_t addr;int bio_blocks =MAX_BIO_BLOCKS (sbi); -struct page *pages[bio_blocks]; -int i, idx, last_offset, nrpages, err =0 ; +int i, idx, last_offset, nrpages;/* scan the node segment */ last_offset = sbi->blocks_per_seg; addr =START_BLOCK (sbi, segno); sum_entry = &sum->entries[0 ]; -for (i =0 ; !err && i < last_offset; i += nrpages, addr += nrpages) { +for (i =0 ; i < last_offset; i += nrpages, addr += nrpages) { nrpages =min (last_offset - i, bio_blocks);/* readahead node pages */ - nrpages =ra_sum_pages (sbi, pages, addr, nrpages); -if (!nrpages) -return -ENOMEM ; +ra_meta_pages (sbi, addr, nrpages,META_POR ); -for (idx =0 ; idx < nrpages; idx++) { -if (err) - goto skip; +for (idx = addr; idx < addr + nrpages; idx++) { +struct page *page =get_meta_page (sbi, idx); -lock_page (pages[idx]); -if (unlikely (!PageUptodate (pages[idx]))) { - err = -EIO ; - }else { - 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 (pages[idx]); -skip: -page_cache_release (pages[idx]); + rn =F2FS_NODE (page); + sum_entry->nid = rn->footer.nid; + sum_entry->version =0 ; + sum_entry->ofs_in_node =0 ; + sum_entry++; +f2fs_put_page (page,1 ); } -invalidate_mapping_pages (inode->i_mapping, addr, - addr + nrpages); +truncate_inode_pages_range (META_MAPPING (sbi), + addr <<PAGE_CACHE_SHIFT , + ((addr + nrpages) <<PAGE_CACHE_SHIFT ) -1 ); } -return err; +return 0 ; }static void remove_nats_in_journal (struct f2fs_sb_info *sbi)
Signature · 1 line
2.1.2
Hi,
Is there any reason to use truncate_inode_pages_range instead of invalidate_mapping_pages?
IMHO, it seems nice to just use invalidate_mapping_pages because pages of meta_inode shouldn't be dirty, locked, under writeback or mapped in this function.
If there is my misunderstanding, let me know.
Thanks,
2014-12-17 19:10 GMT+09:00 Chao Yu <[email protected]>:
Quoted 118 lines
Use more common function
ra_meta_pages()withMETA_PORto readahead node blocks inrestore_node_summary()instead ofra_sum_pages(), hence we can simplify the readahead code there, and also we can remove unused functionra_sum_pages().changes from v1:
o fix one bug when usingtruncate_inode_pages_rangewhich is pointed out by Jaegeuk Kim.Signed-off-by Chao Yu <[email protected]>--- fs/f2fs/node.c | 68 +++++++++++++--------------------------------------------- 1 file changed, 15 insertions(+), 53 deletions(-)fs/f2fs/node.c +15
-54@@ -1726,80 +1726,42 @@ int recover_inode_page(struct f2fs_sb_info *sbi, struct page *page)return 0 ; } -/* -* ra_sum_pages() merge contiguous pages into one bio and submit. -* these pre-read pages are allocated in bd_inode's mapping tree. -*/ -static int ra_sum_pages (struct f2fs_sb_info *sbi,struct page **pages, -int start,int nrpages) -{ -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 (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 ; -f2fs_submit_page_mbio (sbi, pages[i], page_idx, &fio); - } - -f2fs_submit_merged_bio (sbi,META ,READ ); -return i; -} -int restore_node_summary (struct f2fs_sb_info *sbi,unsigned int segno,struct f2fs_summary_block *sum) {struct f2fs_node *rn;struct f2fs_summary *sum_entry; -struct inode *inode = sbi->sb->s_bdev->bd_inode;block_t addr;int bio_blocks =MAX_BIO_BLOCKS (sbi); -struct page *pages[bio_blocks]; -int i, idx, last_offset, nrpages, err =0 ; +int i, idx, last_offset, nrpages;/* scan the node segment */ last_offset = sbi->blocks_per_seg; addr =START_BLOCK (sbi, segno); sum_entry = &sum->entries[0 ]; -for (i =0 ; !err && i < last_offset; i += nrpages, addr += nrpages) { +for (i =0 ; i < last_offset; i += nrpages, addr += nrpages) { nrpages =min (last_offset - i, bio_blocks);/* readahead node pages */ - nrpages =ra_sum_pages (sbi, pages, addr, nrpages); -if (!nrpages) -return -ENOMEM ; +ra_meta_pages (sbi, addr, nrpages,META_POR ); -for (idx =0 ; idx < nrpages; idx++) { -if (err) - goto skip; +for (idx = addr; idx < addr + nrpages; idx++) { +struct page *page =get_meta_page (sbi, idx); -lock_page (pages[idx]); -if (unlikely (!PageUptodate (pages[idx]))) { - err = -EIO ; - }else { - 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 (pages[idx]); -skip: -page_cache_release (pages[idx]); + rn =F2FS_NODE (page); + sum_entry->nid = rn->footer.nid; + sum_entry->version =0 ; + sum_entry->ofs_in_node =0 ; + sum_entry++; +f2fs_put_page (page,1 ); } -invalidate_mapping_pages (inode->i_mapping, addr, - addr + nrpages); +truncate_inode_pages_range (META_MAPPING (sbi), + addr <<PAGE_CACHE_SHIFT , + ((addr + nrpages) <<PAGE_CACHE_SHIFT ) -1 ); } -return err; +return 0 ; }static void remove_nats_in_journal (struct f2fs_sb_info *sbi) --2.1.2
Quoted 10 lines
------------------------------------------------------------------------------
Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server from Actuate! Instantly Supercharge Your Business Reports and Dashboards with Interactivity, Sharing, Native Excel Exports, App Integration & more Get technology previously reserved for billion-dollar corporations, FREE
http://pubads.g.doubleclick.net/gampad/clk?id=164703151&iu=/4140/ostg.clktrk
_______________________________________________
Linux-f2fs-devel mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
------------------------------------------------------------------------------
Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server from Actuate! Instantly Supercharge Your Business Reports and Dashboards with Interactivity, Sharing, Native Excel Exports, App Integration & more Get technology previously reserved for billion-dollar corporations, FREE
http://pubads.g.doubleclick.net/gampad/clk?id=164703151&iu=/4140/ostg.clktrk
Hi Changman,
Quoted 9 lines
-----Original Message-----
From: Changman Lee [mailto:[email protected]]
Sent: Wednesday, December 17, 2014 11:20 PM
To: Chao Yu
Cc: Jaegeuk Kim; Changman Lee; [email protected]; [email protected]; [email protected]
Subject: Re: [f2fs-dev] [PATCH v2] f2fs: usera_meta_pagesto simplify readahead code inrestore_node_summary
Quoted 8 lines
Hi,
Is there any reason to use
truncate_inode_pages_rangeinstead ofinvalidate_mapping_pages?
IMHO, it seems nice to just useinvalidate_mapping_pagesbecause pages ofmeta_inodeshouldn't be dirty, locked, under writeback or mapped in this function.
If there is my misunderstanding, let me know.
I think you're right, I use truncate_inode_pages_range because just following the usage of readahead-invalidate pair in recovery flow, but without considering the difference between them.
I will fix this as you suggested.
Quoted 4 lines
Thanks,
Reviewed-by Changman Lee <[email protected]>
And thanks for your review! :)
Regards,
Yu