From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pd0-f175.google.com ([209.85.192.175]:35979 "EHLO mail-pd0-f175.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751962AbbEUJ2d (ORCPT ); Thu, 21 May 2015 05:28:33 -0400 Received: by pdfh10 with SMTP id h10so101890679pdf.3 for ; Thu, 21 May 2015 02:28:32 -0700 (PDT) Message-ID: <555DA53C.3070707@linaro.org> Date: Thu, 21 May 2015 17:28:28 +0800 From: Alex Shi MIME-Version: 1.0 To: Jan Kara , Dmitry Monakhov , "gregkh@linuxfoundation.org" CC: Theodore Ts'o , Linaro Kernel , stable@vger.kernel.org, Mark Brown Subject: Re: a old issue of ext4 on lts 3.10 References: <5554B3E3.6090407@linaro.org> <87bnhmrj5c.fsf@openvz.org> <20150518062119.GB26351@quack.suse.cz> In-Reply-To: <20150518062119.GB26351@quack.suse.cz> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Sender: stable-owner@vger.kernel.org List-ID: Hi Greg, It was reported this commit could save few seconds sometime in consequence writing on smart phone. commit 7afe5aa59ed3da7b6161617e7f157c7c680dc41e ext4: convert write_begin methods to stable_page_writes semantics > The patch helps because most of storage today doesn't require that the > page isn't changed while IO is in flight. That is required only for > data checksumming or copy-on-write semantics but ext4 does neither of > those. So we don't have to wait for IO completion in ext4_write_begin() > unless underlying storage requires it. > > Honza Seems it is a very simple and useful patch for some stable kernel, like lts 3.10. Would you like to pick it up? Thanks Alex On 05/18/2015 02:21 PM, Jan Kara wrote: > On Thu 14-05-15 23:36:31, Dmitry Monakhov wrote: >> Alex Shi writes: >> >>> Hi Dmitry&Theodore, >>> >>> Someone said without the following patch on lts 3.10 kernel (which used >>> as android base kernel). the write maybe very very slow, needs 1 or 2 >>> seconds to finish. >> In fact this was an optimization. >> wait_for_stable_page() is actually and optimized wait_on_page_writeback() >> >> see: >> void wait_for_stable_page(struct page *page) >> { >> struct address_space *mapping = page_mapping(page); >> struct backing_dev_info *bdi = >> mapping->backing_dev_info; >> >> if (!bdi_cap_stable_pages_required(bdi)) >> return; >> >> wait_on_page_writeback(page); >> } >> It is very unlikely the patch provokes such huge slowdown. >> Can you please repeat your measurements and double check your evidence. > I think Alex meant that without the patch he is seeing long stalls. > That is possible when we wait for writeback and the storage is busy. > >>> I quick looked this patch, seems it's no harm for a normal fs function. >>> but still don't know why it is helpful. So do you remember why you >>> commit this change at that time? > The patch helps because most of storage today doesn't require that the > page isn't changed while IO is in flight. That is required only for > data checksumming or copy-on-write semantics but ext4 does neither of > those. So we don't have to wait for IO completion in ext4_write_begin() > unless underlying storage requires it. > > Honza > >>> ommit 7afe5aa59ed3da7b6161617e7f157c7c680dc41e >>> Author: Dmitry Monakhov >>> Date: Wed Aug 28 14:30:47 2013 -0400 >>> >>> ext4: convert write_begin methods to stable_page_writes semantics >>> >>> Use wait_for_stable_page() instead of wait_on_page_writeback() >>> >>> Signed-off-by: Dmitry Monakhov >>> Signed-off-by: "Theodore Ts'o" >>> Reviewed-by: Jan Kara >>> >>> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c >>> index fc4051e..47c8e46 100644 >>> --- a/fs/ext4/inode.c >>> +++ b/fs/ext4/inode.c >>> @@ -969,7 +969,8 @@ retry_journal: >>> ext4_journal_stop(handle); >>> goto retry_grab; >>> } >>> - wait_on_page_writeback(page); >>> + /* In case writeback began while the page was unlocked */ >>> + wait_for_stable_page(page); >>> >>> if (ext4_should_dioread_nolock(inode)) >>> ret = __block_write_begin(page, pos, len, >>> ext4_get_block_write); >>> @@ -2678,7 +2679,7 @@ retry_journal: >>> goto retry_grab; >>> } >>> /* In case writeback began while the page was unlocked */ >>> - wait_on_page_writeback(page); >>> + wait_for_stable_page(page); >>> >>> ret = __block_write_begin(page, pos, len, ext4_da_get_block_prep); >>> if (ret < 0) { >>> ~ >>> >>> -- >>> Thanks >>> Alex > >