On 2019/1/31 下午10:36, David Sterba wrote:
> On Thu, Jan 31, 2019 at 08:45:42AM +0800, Qu Wenruo wrote:
>>
>>
>> On 2019/1/30 下午11:19, David Sterba wrote:
>>> On Fri, Jan 25, 2019 at 01:09:17PM +0800, Qu Wenruo wrote:
>>>> +static int __must_check flush_write_bio(struct extent_page_data *epd)
>>>>  {
>>>> -	if (epd->bio) {
>>>> -		int ret;
>>>> +	int ret = 0;
>>>>  
>>>> +	if (epd->bio) {
>>>>  		ret = submit_one_bio(epd->bio, 0, 0);
>>>> -		BUG_ON(ret < 0); /* -ENOMEM */
>>>>  		epd->bio = NULL;
>>>
>>> I'm not sure if resetting epd->bio to NULL is all that needs to be done
>>> here. With the BUG_ON the error case never happens so if all goes fine
>>> it's also ok to set it to NULL and continue. But the callers might need
>>> to send the flush again.
>>
>> If flush_write_bio() get called again on the failed one, it will just
>> get skipped as  epd->bio is NULL, submit_one_bio() will not be triggered.
> 
> Yes that's clear, but what is the state of the bio that went to
> flush_write_bio and submit_one_bio failed?
> 

You got me.

In fact current simple "epd->bio = NULL" on failure will just leak epd->bio.

We need to at least call "bio_put(epd->bio);" before setting "epd->bio =
NULL;"

Thanks for pointing this problem out,
Qu