Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752835AbbL1WwJ (ORCPT ); Mon, 28 Dec 2015 17:52:09 -0500 Received: from mail.kernel.org ([198.145.29.136]:41482 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752566AbbL1WwH (ORCPT ); Mon, 28 Dec 2015 17:52:07 -0500 Date: Mon, 28 Dec 2015 14:52:05 -0800 From: Jaegeuk Kim To: Chao Yu Cc: linux-f2fs-devel@lists.sourceforge.net, linux-kernel@vger.kernel.org Subject: Re: [RFC PATCH 2/2] f2fs: export a threshold in sysfs for controlling dio serialization Message-ID: <20151228225204.GB61500@jaegeuk.local> References: <006e01d14157$68bc0dd0$3a342970$@samsung.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <006e01d14157$68bc0dd0$3a342970$@samsung.com> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5975 Lines: 150 Hi Chao, On Mon, Dec 28, 2015 at 06:05:45PM +0800, Chao Yu wrote: > As Yunlei He reported when he test with the patch ("f2fs: enhance > multithread dio write performance"): > "Does share writepages mutex lock have an effect on cache write? > Here is AndroBench result on my phone: > > Before patch: > 1R1W 8R8W 16R16W > Sequential Write 161.31 163.85 154.67 > Random Write 9.48 17.66 18.09 > > After patch: > 1R1W 8R8W 16R16W > Sequential Write 159.61 157.24 160.11 > Random Write 9.17 8.51 8.8 > > Unit:Mb/s, File size: 64M, Buffer size: 4k" > > The turth is androidbench uses single thread with dio write to test performance > of sequential write, and use multi-threads with dio write to test performance > of random write. so we can not see any improvement in sequentail write test > since serializing dio page allocation can only improve performance in > multi-thread scenario, and there is a regression in multi-thread test with 4k > dio write, this is because grabbing sbi->writepages lock for serializing block > allocation stop the concurrency, so that less small dio bios could be merged, > moreover, when there are huge number of small dio writes, grabbing mutex lock > per dio increases the overhead. > > After all, serializing dio could only be used for concurrent scenario of > big dio, so this patch introduces a threshold in sysfs to provide user the > interface of defining 'a big dio' with specified page number, which could > be used to control wthether serialize or not that kind of dio with specified > page number. Can you merge two patches together? And, if this is correct, can we investigate the lock effect in f2fs_write_data_pages too? What if we add a condition for the lock like this? if (get_dirty_pages(inode) > serialzed_pages) mutex_lock(); Thanks, > > Though, this is only RFC patch since the optimization works in rare scenario. > > Signed-off-by: Chao Yu > --- > Documentation/ABI/testing/sysfs-fs-f2fs | 12 ++++++++++++ > fs/f2fs/data.c | 3 ++- > fs/f2fs/f2fs.h | 3 +++ > fs/f2fs/super.c | 3 +++ > 4 files changed, 20 insertions(+), 1 deletion(-) > > diff --git a/Documentation/ABI/testing/sysfs-fs-f2fs b/Documentation/ABI/testing/sysfs-fs-f2fs > index 0345f2d..560a4f1 100644 > --- a/Documentation/ABI/testing/sysfs-fs-f2fs > +++ b/Documentation/ABI/testing/sysfs-fs-f2fs > @@ -92,3 +92,15 @@ Date: October 2015 > Contact: "Chao Yu" > Description: > Controls the count of nid pages to be readaheaded. > + > +What: /sys/fs/f2fs//serialized_dio_pages > +Date: December 2015 > +Contact: "Chao Yu" > +Description: > + It is a threshold with the unit of page size. > + If DIO page count is equal or big than the threshold, > + whole process of block address allocation of dio pages > + will become atomic like buffered write. > + It is used to maximize bandwidth utilization in the > + scenario of concurrent write with dio vs buffered or > + dio vs dio. > diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c > index 6b24446..abcd100 100644 > --- a/fs/f2fs/data.c > +++ b/fs/f2fs/data.c > @@ -1660,7 +1660,8 @@ static ssize_t f2fs_direct_IO(struct kiocb *iocb, struct iov_iter *iter, > trace_f2fs_direct_IO_enter(inode, offset, count, rw); > > if (rw == WRITE) { > - bool serialized = (F2FS_BYTES_TO_BLK(count) >= 64); > + bool serialized = (F2FS_BYTES_TO_BLK(count) >= > + sbi->serialized_dio_pages); > > if (serialized) > mutex_lock(&sbi->writepages); > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h > index 3406e99..8f35dd7 100644 > --- a/fs/f2fs/f2fs.h > +++ b/fs/f2fs/f2fs.h > @@ -333,6 +333,8 @@ enum { > > #define MAX_DIR_RA_PAGES 4 /* maximum ra pages of dir */ > > +#define DEF_SERIALIZED_DIO_PAGES 64 /* default serialized dio pages */ > + > /* vector size for gang look-up from extent cache that consists of radix tree */ > #define EXT_TREE_VEC_SIZE 64 > > @@ -784,6 +786,7 @@ struct f2fs_sb_info { > unsigned int total_valid_inode_count; /* valid inode count */ > int active_logs; /* # of active logs */ > int dir_level; /* directory level */ > + int serialized_dio_pages; /* serialized direct IO pages */ > > block_t user_block_count; /* # of user blocks */ > block_t total_valid_block_count; /* # of valid blocks */ > diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c > index 75704d9..ebe9bd4 100644 > --- a/fs/f2fs/super.c > +++ b/fs/f2fs/super.c > @@ -218,6 +218,7 @@ F2FS_RW_ATTR(NM_INFO, f2fs_nm_info, ram_thresh, ram_thresh); > F2FS_RW_ATTR(NM_INFO, f2fs_nm_info, ra_nid_pages, ra_nid_pages); > F2FS_RW_ATTR(F2FS_SBI, f2fs_sb_info, max_victim_search, max_victim_search); > F2FS_RW_ATTR(F2FS_SBI, f2fs_sb_info, dir_level, dir_level); > +F2FS_RW_ATTR(F2FS_SBI, f2fs_sb_info, serialized_dio_pages, serialized_dio_pages); > F2FS_RW_ATTR(F2FS_SBI, f2fs_sb_info, cp_interval, cp_interval); > > #define ATTR_LIST(name) (&f2fs_attr_##name.attr) > @@ -234,6 +235,7 @@ static struct attribute *f2fs_attrs[] = { > ATTR_LIST(min_fsync_blocks), > ATTR_LIST(max_victim_search), > ATTR_LIST(dir_level), > + ATTR_LIST(serialized_dio_pages), > ATTR_LIST(ram_thresh), > ATTR_LIST(ra_nid_pages), > ATTR_LIST(cp_interval), > @@ -1125,6 +1127,7 @@ static void init_sb_info(struct f2fs_sb_info *sbi) > atomic_set(&sbi->nr_pages[i], 0); > > sbi->dir_level = DEF_DIR_LEVEL; > + sbi->serialized_dio_pages = DEF_SERIALIZED_DIO_PAGES; > sbi->cp_interval = DEF_CP_INTERVAL; > clear_sbi_flag(sbi, SBI_NEED_FSCK); > > -- > 2.6.3 > -- 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/