Re: [PATCH] f2fs: avoid issuing small bios due to several dirty node pages

[Namjae Jeon <linkinjeon@gmail.com>]

2013/1/18, Jaegeuk Kim <jaegeuk.kim@samsung.com>:
> 2013-01-18 (금), 16:58 +0900, Namjae Jeon:
>> 2013/1/18, Jaegeuk Kim <jaegeuk.kim@samsung.com>:
>> > If some small bios of dirty node pages are supposed to be issued during
>> > the
>> > sequential data writes, there-in well-produced consecutive data bios
>> > are
>> > able
>> > to be split by the small node bios, resulting in performance
>> > degradation.
>> > So, let's collect a number of dirty node pages until reaching a
>> > threshold.
>> > And, by default, I set the threshold as 2MB, a segment size.
>> >
>> > This improves sequential write performance on i5, 512GB SSD (830 w/
>> > SATA2)
>> > as
>> > follows.
>> > Before: 231 MB/s -> After: 255 MB/s
>> >
>> > Signed-off-by: Jaegeuk Kim <jaegeuk.kim@samsung.com>
>> > ---
>> >  fs/f2fs/node.c | 17 +++++++++++------
>> >  1 file changed, 11 insertions(+), 6 deletions(-)
>> >
>> > diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
>> > index f177c01..9bda63c 100644
>> > --- a/fs/f2fs/node.c
>> > +++ b/fs/f2fs/node.c
>> > @@ -1124,6 +1124,12 @@ static int f2fs_write_node_page(struct page
>> > *page,
>> >  	return 0;
>> >  }
>> >
>> > +/*
>> > + * It is very important to gather dirty pages and write at once, so
>> > that we
>> > can
>> > + * submit a big bio without interfering other data writes.
>> > + * Be default, 512 pages (2MB), a segment size, is quite reasonable.
>> > + */
>> > +#define COLLECT_DIRTY_NODES	512
>> Hi Jaeguek.
>> It is just my opinion.
>> One of f2fs advantages is that user can set segment size to fit own
>> device.
>> For future, Is it not good to use segment size set by user(from
>> superblock) instead of fixed size ?
>
> At this moment, it's very hard to change the segment size due to many
> hardcoded data structures such as SSA, bitmaps, etc.
> Instead, I proposed something like section and zone which are based on
> segment.
> But, somebody will be able to do later. :)
Reviewed-by: Namjae Jeon <namjae.jeon@samsung.com>
Okay, I think It is better if you add TODO comment about this to be
fixed by the other.

Talk Incidentally, let me ask you other thing.
get_meta_page never return NULL about allocate page and readpage.
Is there any reason it try to infinitely retry about two case ?

Thanks for reply!
>

>>
>> Thanks.
>>
>> >  static int f2fs_write_node_pages(struct address_space *mapping,
>> >  			    struct writeback_control *wbc)
>> >  {
>> > @@ -1131,17 +1137,16 @@ static int f2fs_write_node_pages(struct
>> > address_space *mapping,
>> >  	struct block_device *bdev = sbi->sb->s_bdev;
>> >  	long nr_to_write = wbc->nr_to_write;
>> >
>> > -	if (wbc->for_kupdate)
>> > -		return 0;
>> > -
>> > -	if (get_pages(sbi, F2FS_DIRTY_NODES) == 0)
>> > -		return 0;
>> > -
>> > +	/* First check balancing cached NAT entries */
>> >  	if (try_to_free_nats(sbi, NAT_ENTRY_PER_BLOCK)) {
>> >  		write_checkpoint(sbi, false, false);
>> >  		return 0;
>> >  	}
>> >
>> > +	/* collect a number of dirty node pages and write together */
>> > +	if (get_pages(sbi, F2FS_DIRTY_NODES) < COLLECT_DIRTY_NODES)
>> > +		return 0;
>> > +
>> >  	/* if mounting is failed, skip writing node pages */
>> >  	wbc->nr_to_write = bio_get_nr_vecs(bdev);
>> >  	sync_node_pages(sbi, 0, wbc);
>> > --
>> > 1.8.0.1.250.gb7973fb
>> >
>> > --
>> > To unsubscribe from this list: send the line "unsubscribe linux-fsdevel"
>> > in
>> > the body of a message to majordomo@vger.kernel.org
>> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
>> >
>
> --
> Jaegeuk Kim
> Samsung
>
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html