Hi Gu, > -----Original Message----- > From: Gu Zheng [mailto:guz.fnst@xxxxxxxxxxxxxx] > Sent: Wednesday, November 20, 2013 5:38 PM > To: Chao Yu > Cc: '???'; linux-fsdevel@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; linux-f2fs-devel@xxxxxxxxxxxxxxxxxxxxx; '谭姝' > Subject: Re: [f2fs-dev] [PATCH V2 2/2] f2fs: read contiguous sit entry pages by merging for mount performance > > Hi Yu, > On 11/20/2013 01:37 PM, Chao Yu wrote: > > > Hi Gu, > > > >> -----Original Message----- > >> From: Gu Zheng [mailto:guz.fnst@xxxxxxxxxxxxxx] > >> Sent: Monday, November 18, 2013 7:16 PM > >> To: Chao Yu > >> Cc: '???'; linux-fsdevel@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; linux-f2fs-devel@xxxxxxxxxxxxxxxxxxxxx; 谭 姝 > >> Subject: Re: [f2fs-dev] [PATCH V2 2/2] f2fs: read contiguous sit entry pages by merging for mount performance > >> > >> Hi Yu, > >> One more comment, please refer to inline. > >> On 11/16/2013 02:15 PM, Chao Yu wrote: > >> > >>> Previously we read sit entries page one by one, this method lost the chance of reading contiguous page together. > >>> So we read pages as contiguous as possible for better mount performance. > >>> > >>> v1-->v2: > >>> o merge judgements/use 'Continue' or 'Break' instead of 'Goto' as Gu Zheng suggested. > >>> o add mark_page_accessed () before release page to delay VM reclaiming them. > >>> > >>> Signed-off-by: Chao Yu <chao2.yu@xxxxxxxxxxx> > >>> --- > >>> fs/f2fs/segment.c | 108 ++++++++++++++++++++++++++++++++++++++++------------- > >>> fs/f2fs/segment.h | 2 + > >>> 2 files changed, 84 insertions(+), 26 deletions(-) > >>> > >>> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c > >>> index fa284d3..656fe40 100644 > >>> --- a/fs/f2fs/segment.c > >>> +++ b/fs/f2fs/segment.c > >>> @@ -14,6 +14,7 @@ > >>> #include <linux/blkdev.h> > >>> #include <linux/prefetch.h> > >>> #include <linux/vmalloc.h> > >>> +#include <linux/swap.h> > >>> > >>> #include "f2fs.h" > >>> #include "segment.h" > >>> @@ -1480,41 +1481,96 @@ static int build_curseg(struct f2fs_sb_info *sbi) > >>> return restore_curseg_summaries(sbi); > >>> } > >>> > >>> +static int ra_sit_pages(struct f2fs_sb_info *sbi, int start, > >>> + int nrpages, bool *is_order) > >>> +{ > >>> + struct address_space *mapping = sbi->meta_inode->i_mapping; > >>> + struct sit_info *sit_i = SIT_I(sbi); > >>> + struct page *page; > >>> + block_t blk_addr; > >>> + int blkno = start, readcnt = 0; > >>> + int sit_blk_cnt = SIT_BLK_CNT(sbi); > >>> + > >>> + for (; blkno < start + nrpages && blkno < sit_blk_cnt; blkno++) { > >>> + > >>> + if ((!f2fs_test_bit(blkno, sit_i->sit_bitmap) ^ !*is_order)) { > >>> + *is_order = !*is_order; > >>> + break; > >>> + } > >>> + > >>> + blk_addr = sit_i->sit_base_addr + blkno; > >>> + if (*is_order) > >>> + blk_addr += sit_i->sit_blocks; > >>> +repeat: > >>> + page = grab_cache_page(mapping, blk_addr); > >>> + if (!page) { > >>> + cond_resched(); > >>> + goto repeat; > >>> + } > >>> + if (PageUptodate(page)) { > >>> + mark_page_accessed(page); > >>> + f2fs_put_page(page, 1); > >>> + readcnt++; > >>> + continue; > >>> + } > >>> + > >>> + submit_read_page(sbi, page, blk_addr, READ_SYNC); > >>> + > >>> + mark_page_accessed(page); > >>> + f2fs_put_page(page, 0); > >>> + readcnt++; > >>> + } > >>> + > >>> + f2fs_submit_read_bio(sbi, READ_SYNC); > >>> + return readcnt; > >>> +} > >>> + > >>> static void build_sit_entries(struct f2fs_sb_info *sbi) > >>> { > >>> struct sit_info *sit_i = SIT_I(sbi); > >>> struct curseg_info *curseg = CURSEG_I(sbi, CURSEG_COLD_DATA); > >>> struct f2fs_summary_block *sum = curseg->sum_blk; > >>> - unsigned int start; > >>> - > >>> - for (start = 0; start < TOTAL_SEGS(sbi); start++) { > >>> - struct seg_entry *se = &sit_i->sentries[start]; > >>> - struct f2fs_sit_block *sit_blk; > >>> - struct f2fs_sit_entry sit; > >>> - struct page *page; > >>> - int i; > >>> + bool is_order = f2fs_test_bit(0, sit_i->sit_bitmap) ? true : false; > >>> + int sit_blk_cnt = SIT_BLK_CNT(sbi); > >>> + unsigned int i, start, end; > >>> + unsigned int readed, start_blk = 0; > >>> > >>> - mutex_lock(&curseg->curseg_mutex); > >>> - for (i = 0; i < sits_in_cursum(sum); i++) { > >>> - if (le32_to_cpu(segno_in_journal(sum, i)) == start) { > >>> - sit = sit_in_journal(sum, i); > >>> - mutex_unlock(&curseg->curseg_mutex); > >>> - goto got_it; > >>> + do { > >> > >> How about using find_next_bit to get the suitable start_blk if the next blk > >> is not ordered here? And it also can simplify the logic of ra_sit_pages(). > > > > That's a good idea. > > But I thought there maybe endianness problem between test_bit and > > f2fs_test_bit, so find_next_bit may get wrong result. Am I right? > > IMO, find_next_bit can do well with endianness issue internally, if > it's not so, that may be a weakness. Right, I mean that if we want to set first position in bitmap, we cloud use f2fs_set_bit(0, bitmap) or set_bit(7, bitmap). Two types of interface could not be used mixedly. So could we use {set,test,clear}_bit intead of f2fs_{set,test,clear}_bit? > On the other side, why not introduce a 'f2fs_find_next_bit' if it's > seriously needed?:) Agreed, that's a good point, they were really needed for uniform style and readability. > > Regards, > Gu > > > > > Thanks, > > Yu > >> > >> Thanks, > >> Gu > >> > >>> + readed = ra_sit_pages(sbi, start_blk, sit_blk_cnt, &is_order); > >>> + > >>> + start = start_blk * sit_i->sents_per_block; > >>> + end = (start_blk + readed) * sit_i->sents_per_block; > >>> + > >>> + for (; start < end && start < TOTAL_SEGS(sbi); start++) { > >>> + struct seg_entry *se = &sit_i->sentries[start]; > >>> + struct f2fs_sit_block *sit_blk; > >>> + struct f2fs_sit_entry sit; > >>> + struct page *page; > >>> + > >>> + mutex_lock(&curseg->curseg_mutex); > >>> + for (i = 0; i < sits_in_cursum(sum); i++) { > >>> + if (le32_to_cpu(segno_in_journal(sum, i)) == start) { > >>> + sit = sit_in_journal(sum, i); > >>> + mutex_unlock(&curseg->curseg_mutex); > >>> + goto got_it; > >>> + } > >>> } > >>> - } > >>> - mutex_unlock(&curseg->curseg_mutex); > >>> - page = get_current_sit_page(sbi, start); > >>> - sit_blk = (struct f2fs_sit_block *)page_address(page); > >>> - sit = sit_blk->entries[SIT_ENTRY_OFFSET(sit_i, start)]; > >>> - f2fs_put_page(page, 1); > >>> + mutex_unlock(&curseg->curseg_mutex); > >>> + > >>> + page = get_current_sit_page(sbi, start); > >>> + sit_blk = (struct f2fs_sit_block *)page_address(page); > >>> + sit = sit_blk->entries[SIT_ENTRY_OFFSET(sit_i, start)]; > >>> + f2fs_put_page(page, 1); > >>> got_it: > >>> - check_block_count(sbi, start, &sit); > >>> - seg_info_from_raw_sit(se, &sit); > >>> - if (sbi->segs_per_sec > 1) { > >>> - struct sec_entry *e = get_sec_entry(sbi, start); > >>> - e->valid_blocks += se->valid_blocks; > >>> + check_block_count(sbi, start, &sit); > >>> + seg_info_from_raw_sit(se, &sit); > >>> + if (sbi->segs_per_sec > 1) { > >>> + struct sec_entry *e = get_sec_entry(sbi, start); > >>> + e->valid_blocks += se->valid_blocks; > >>> + } > >>> } > >>> - } > >>> + start_blk += readed; > >> > >> > >> > >> > >>> + } while (start_blk < sit_blk_cnt); > >>> } > >>> > >>> static void init_free_segmap(struct f2fs_sb_info *sbi) > >>> diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h > >>> index 269f690..ad5b9f1 100644 > >>> --- a/fs/f2fs/segment.h > >>> +++ b/fs/f2fs/segment.h > >>> @@ -83,6 +83,8 @@ > >>> (segno / SIT_ENTRY_PER_BLOCK) > >>> #define START_SEGNO(sit_i, segno) \ > >>> (SIT_BLOCK_OFFSET(sit_i, segno) * SIT_ENTRY_PER_BLOCK) > >>> +#define SIT_BLK_CNT(sbi) \ > >>> + ((TOTAL_SEGS(sbi) + SIT_ENTRY_PER_BLOCK - 1) / SIT_ENTRY_PER_BLOCK) > >>> #define f2fs_bitmap_size(nr) \ > >>> (BITS_TO_LONGS(nr) * sizeof(unsigned long)) > >>> #define TOTAL_SEGS(sbi) (SM_I(sbi)->main_segments) > > > > > > > > > = -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
