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
signature.asc
Description: OpenPGP digital signature
