From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754368AbbIPKQj (ORCPT ); Wed, 16 Sep 2015 06:16:39 -0400 Received: from mailout2.samsung.com ([203.254.224.25]:48183 "EHLO mailout2.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753554AbbIPKQg (ORCPT ); Wed, 16 Sep 2015 06:16:36 -0400 X-AuditID: cbfee61b-f79d56d0000048c5-14-55f94183e8a9 From: Chao Yu To: "'Jaegeuk Kim'" Cc: linux-f2fs-devel@lists.sourceforge.net, linux-kernel@vger.kernel.org References: <000001d0ec5d$0bbd1880$23374980$@samsung.com> <20150915212044.GA84514@jaegeuk-mac02.mot.com> In-reply-to: <20150915212044.GA84514@jaegeuk-mac02.mot.com> Subject: RE: [PATCH 5/7] f2fs: enhance multithread dio write performance Date: Wed, 16 Sep 2015 18:15:55 +0800 Message-id: <000a01d0f068$c46012c0$4d203840$@samsung.com> MIME-version: 1.0 Content-type: text/plain; charset=us-ascii Content-transfer-encoding: 7bit X-Mailer: Microsoft Outlook 14.0 Thread-index: AQJhlFBL6MN/I6SqoI+Ygc+R5qpxRgLy5sWInQWRCxA= Content-language: zh-cn X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFnrELMWRmVeSWpSXmKPExsVy+t9jAd1mx5+hBkvPqVg8WT+L2eLSIneL y7vmsDkwe2xa1cnmsXvBZyaPz5vkApijuGxSUnMyy1KL9O0SuDJmvn3BXDBbrWLlsldsDYwn 5boYOTkkBEwkNs5axAxhi0lcuLeerYuRi0NIYCmjxK5n+1hAEkICrxgl3n7gBLHZBFQklnf8 ZwKxRQTUJHr3TQGzmQU8JBo7vrNC1GdJ/HzxE8zmFLCWmLjxEdAcDg5hoJpzTdwgYRYBVYln /Z/AxvMKWEp83NoCZQtK/Jh8jwVipJbE+p3HocbLS2xe8xbqTgWJHWdfM0KcYCXx5NsXqHpx iY1HbrFMYBSahWTULCSjZiEZNQtJywJGllWMEqkFyQXFSem5Rnmp5XrFibnFpXnpesn5uZsY wSH/THoH4+Fd7ocYBTgYlXh4HV7+CBViTSwrrsw9xCjBwawkwvvI6meoEG9KYmVValF+fFFp TmrxIUZpDhYlcV7Zlc9ChQTSE0tSs1NTC1KLYLJMHJxSDYzR7VMvPHj89/WEQ6cm8YfeLvn/ 0PfZxD9aqRdllr6YPyWxQCHN83FYHre8aoLmHF4eH69zK1jNXyf0bff4xpdrInixeH1H32n5 T4taN1uedN666OJmg4+uxsxyK14d2up1x/KBSqSv2Fef04kvvy1x8Jou8GpqhFr/titJ0bbn 832kW2/MefNRiaU4I9FQi7moOBEAPqRxY3UCAAA= Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Jaegeuk, > -----Original Message----- > From: Jaegeuk Kim [mailto:jaegeuk@kernel.org] > Sent: Wednesday, September 16, 2015 5:21 AM > To: Chao Yu > Cc: linux-f2fs-devel@lists.sourceforge.net; linux-kernel@vger.kernel.org > Subject: Re: [PATCH 5/7] f2fs: enhance multithread dio write performance > > Hi Chao, > > On Fri, Sep 11, 2015 at 02:41:53PM +0800, Chao Yu wrote: > > When dio writes perform concurrently, our performace will be low because of > > Thread A's allocation of multi continuous blocks will be break by Thread B, > > there are two cases as below: > > - In Thread B, we may change current segment to a new segment for LFS > > allocation if we dio write in the beginning of the file. > > - In Thread B, we may allocate blocks in the middle of Thread A's > > allocation, which make blocks which allocated in Thread A being > > discontinuous. > > > > This patch adds writepages mutex lock to make block allocation in dio write > > atomic to avoid above issues. > > > > Test environment: > > ubuntu os with linux kernel 4.2+, intel i7-3770, 16g memory, > > 32g kingston sd card. > > > > fio --name seqw --ioengine=sync --invalidate=1 --rw=write --directory=/mnt/f2fs > --filesize=256m --size=16m --bs=2m --direct=1 > > --numjobs=10 > > > > before: > > WRITE: io=163840KB, aggrb=3145KB/s, minb=314KB/s, maxb=411KB/s, mint=39836msec, > maxt=52083msec > > > > patched: > > WRITE: io=163840KB, aggrb=10033KB/s, minb=1003KB/s, maxb=1124KB/s, mint=14565msec, > maxt=16329msec > > > > Signed-off-by: Chao Yu > > --- > > fs/f2fs/data.c | 13 ++++++++++--- > > 1 file changed, 10 insertions(+), 3 deletions(-) > > > > diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c > > index a737ca5..a0a5849 100644 > > --- a/fs/f2fs/data.c > > +++ b/fs/f2fs/data.c > > @@ -1536,7 +1536,9 @@ static ssize_t f2fs_direct_IO(struct kiocb *iocb, struct iov_iter *iter, > > struct file *file = iocb->ki_filp; > > struct address_space *mapping = file->f_mapping; > > struct inode *inode = mapping->host; > > + struct f2fs_sb_info *sbi = F2FS_I_SB(inode); > > size_t count = iov_iter_count(iter); > > + int rw = iov_iter_rw(iter); > > int err; > > > > /* we don't need to use inline_data strictly */ > > @@ -1555,12 +1557,17 @@ static ssize_t f2fs_direct_IO(struct kiocb *iocb, struct iov_iter > *iter, > > > > trace_f2fs_direct_IO_enter(inode, offset, count, iov_iter_rw(iter)); > > > > - if (iov_iter_rw(iter) == WRITE) > > + if (rw == WRITE) { > > + mutex_lock(&sbi->writepages); > > Why do we have to share sbi->writepages? The root cause of this issue is that: in f2fs, we have no suitable dispatcher which can do the following things as an atomic operation: a) allocate position(s) in flash device for current block(s); b) submit user data in allocated position(s) in block layer. Without the dispatcher, we will suffer performance issue in following scenario: Thread A Thread B Thread C allocate pos+1 allocate pos+2 allocate pos+3 submit pos+1 submit pos+3 submit pos+2 Our final submitting series will: pos+1, pos+3, pos+2, this makes f2fs running into non-LFS mode, therefore resulting in bad performance. writepages mutex lock supply us with a good solution for above issue. It not only make the allocating and submitting pair executing atomically, but also reduce the fragmentation for one file since we submit blocks belong to single inode as continuous as possible. So here I choose to use writepages mutex lock to fix the performance issue caused by both dio write vs dio write and dio write vs buffered write. If I'm missing something, please correct me. > > > __allocate_data_blocks(inode, offset, count); > > If the problem lies on the misaligned blocks, how about calling mutex_unlock > here? When changing to unlock here, I got regression when testing with following command: fio --name seqw --ioengine=sync --invalidate=1 --rw=write --directory=/mnt/f2fs --filesize=256m --size=4m --bs=64k --direct=1 --numjobs=20 unlock here: WRITE: io=81920KB, aggrb=5802KB/s, minb=290KB/s, maxb=292KB/s, mint=14010msec, maxt=14119msec unlock after dio finished: WRITE: io=81920KB, aggrb=6088KB/s, minb=304KB/s, maxb=1081KB/s, mint=3786msec, maxt=13454msec So how about keep it in original place in this patch? Thanks, > > Thanks, > > > + } > > > > err = blockdev_direct_IO(iocb, inode, iter, offset, get_data_block_dio); > > - if (err < 0 && iov_iter_rw(iter) == WRITE) > > - f2fs_write_failed(mapping, offset + count); > > + if (rw == WRITE) { > > + mutex_unlock(&sbi->writepages); > > + if (err) > > + f2fs_write_failed(mapping, offset + count); > > + } > > > > trace_f2fs_direct_IO_exit(inode, offset, count, iov_iter_rw(iter), err); > > > > -- > > 2.4.2