On 2019/2/15 下午3:18, Qu Wenruo wrote:
>
>
> 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;"Facepalm, I'm always too quick to draw a conclusion. With proper test, the epd->bio is freed by its endio function. So at flush_write_bio() call time, no matter whether submit_one_bio() returns error or not, endio of epd->bio will be or has already been triggered thus free the bio. Either by successful bio submission, or by manual bio_endio(bio) call in submit_bio_hook. So in fact just setting "epd->bio = NULL" is completely fine in this context. Thanks, Qu > > Thanks for pointing this problem out, > Qu >
signature.asc
Description: OpenPGP digital signature
