On Thu 14-05-15 23:36:31, Dmitry Monakhov wrote:
> Alex Shi <alex.shi@xxxxxxxxxx> 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 <dmonakhov@xxxxxxxxxx>
> > 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 <dmonakhov@xxxxxxxxxx>
> > Signed-off-by: "Theodore Ts'o" <tytso@xxxxxxx>
> > Reviewed-by: Jan Kara <jack@xxxxxxx>
> >
> > 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
--
Jan Kara <jack@xxxxxxx>
SUSE Labs, CR
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html