lkml.org 
[lkml]   [2013]   [Nov]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRE: [f2fs-dev] [PATCH V2 2/2 RESEND] f2fs: read contiguous sit entry pages by merging for mount performance
Date
Hi,

> -----Original Message-----
> From: Jaegeuk Kim [mailto:jaegeuk.kim@samsung.com]
> Sent: Thursday, November 21, 2013 12:46 PM
> To: Chao Yu
> Cc: linux-fsdevel@vger.kernel.org; linux-kernel@vger.kernel.org; linux-f2fs-devel@lists.sourceforge.net; '谭姝'
> 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@samsung.com]
> > > Sent: Thursday, November 21, 2013 9:32 AM
> > > To: Chao Yu
> > > Cc: linux-fsdevel@vger.kernel.org; linux-kernel@vger.kernel.org; linux-f2fs-devel@lists.sourceforge.net; 谭姝
> > > 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-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2013-11-21 08:01    [W:0.040 / U:0.548 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site