On 2019/3/13 下午7:31, David Sterba wrote:
> On Tue, Mar 12, 2019 at 08:42:12AM +0800, Qu Wenruo wrote:
>>
>>
>> On 2019/3/12 上午8:33, David Sterba wrote:
>>> On Mon, Feb 18, 2019 at 01:27:51PM +0800, Qu Wenruo wrote:
>>>> Signed-off-by: Qu Wenruo <wqu@xxxxxxxx>
>>>> Reviewed-by: Johannes Thumshirn <jthumshirn@xxxxxxx>
>>>> ---
>>>> fs/btrfs/extent_io.c | 10 ++++++++--
>>>> 1 file changed, 8 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
>>>> index 1572e892ec7b..480e138051f0 100644
>>>> --- a/fs/btrfs/extent_io.c
>>>> +++ b/fs/btrfs/extent_io.c
>>>> @@ -3998,7 +3998,10 @@ static int extent_write_cache_pages(struct address_space *mapping,
>>>> */
>>>> if (!trylock_page(page)) {
>>>> ret = flush_write_bio(epd);
>>>> - BUG_ON(ret < 0);
>>>> + if (ret < 0) {
>>>
>>> This needs some more explanation why it's correct, there's conditional
>>> locking and writeback status manipulation. Thanks.
>>
>> Because flush_write_bio() handles the cleanup in its endio function.
>>
>> Thus we don't need extra cleanup in this case.
>
> Non-trivial changes need changelog, that's nothing new. Especially for
> error handling you have to provide a proof that things don't get broken.
>
> If you spend a week working on the code, everything is trivial, but only
> for you and for a short period of time. That's why we need good
> changelogs that make sense to other people and after a long period of
> time. Thanks.
That completely makes sense, since even myself need some time to get to
the point why the error out works.
I'll update the change log for a comprehensive call path why it works.
Thanks,
Qu
Attachment:
signature.asc
Description: OpenPGP digital signature
