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 <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
>
>
--
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