Hi, > -----Original Message----- > From: Jaegeuk Kim [mailto:jaegeuk.kim@xxxxxxxxxxx] > Sent: Thursday, November 21, 2013 12:46 PM > To: Chao Yu > Cc: linux-fsdevel@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; linux-f2fs-devel@xxxxxxxxxxxxxxxxxxxxx; '谭姝' > Subject: RE: [f2fs-dev] [PATCH V2 2/2 RESEND] f2fs: read contiguous sit entry pages by merging for mount performance > > Hi, > > 2013-11-21 (목), 11:18 +0800, Chao Yu: > > Hi, > > > > > -----Original Message----- > > > From: Jaegeuk Kim [mailto:jaegeuk.kim@xxxxxxxxxxx] > > > Sent: Thursday, November 21, 2013 9:32 AM > > > To: Chao Yu > > > Cc: linux-fsdevel@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; linux-f2fs-devel@xxxxxxxxxxxxxxxxxxxxx; 谭姝 > > > Subject: Re: [f2fs-dev] [PATCH V2 2/2 RESEND] f2fs: read contiguous sit entry pages by merging for mount performance > > > > > > Hi, > > > > > > It seems that ra_sit_pages() is too tightly coupled with > > > build_sit_entries(). > > > > This code could be improved. > > > > > Is there another way not to use *is_order? > > > > Previously the code is like this: > > -build_sit_entries() > > next_setp: > > for(start = 0; start < TOTAL_SEGS(sbi); start++) > > /*step#1 readahead all sit entries blocks*/ > > if(start % SIT_ENTRY_PER_BLOCK == 0) { > > blk_addr = current_sit_addr(sbi, start); > > /* grab and submit_read_page */ > > } > > if(start == TOTAL_SEGS(sbi) - 1) > > f2fs_submit_read_bio(); > > continue; > > /*step#2 fill sit entries info*/ > > /*step#3 cover sit entries with journal*/ > > > > But I think its weakness is that it will cost lots of memory to read > > ahead all sit entry pages when f2fs mount, and also it's serious waste > > that we read them again after these pages are released by VM when > > out of memory. > > > > > > > > The ra_sit_pages() tries to read consecutive sit pages as many as > > > possible. > > > So then, what about just checking whether its block address is > > > contiguous or not? > > > > > > Something like this: > > > -ra_sit_pages() > > > blkno = start; > > > while (blkno < sit_i->sit_blocks) { > > > blk_addr = current_sit_addr(sbi, blkno); > > > if (blkno != start && prev_blk_addr + 1 != blk_addr) > > > break; > > > > > > /* grab and submit_read_page */ > > > > > > prev_blk_addr = blk_addr; > > > blkno++; > > > } > > > > Agreed, this method could remove *order. > > Shouldn't we add nrpages for readahead policy as VM? > > Aha, agreed. > We need nrpages to avoid too many reads on sit blocks. > > But, still it needs to change the nrpages in its caller. > In your patch, it was sit_i->sit_blocks that is total # of sit blocks. > I think 128 or 256 is quite reasonable number. Hmm, Originally in [PATCH V1] it was be set to MAX_BIO_BLOCKS(max_hw_blocks(sbi)). So it could be "#define SIT_ENTRIES_RA_NUM 128"? BTW, maybe we should send dynamical nrpages which depend on memory state of system as I mention in previous thread. How do you think? > > Anyway, how about implementing ra_sit_pages() with a blk_plug likewise > ra_node_pages()? So we use this structure to plug multi bios submitting in ra_sit_pages(), right? -build_sit_entries() blk_start_plug(&plug); ra_sit_pages(); blk_finish_plug(&plug); > > -- > Jaegeuk Kim > Samsung -- 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
