[PATCH] btrfs: Fix bio leak

[PATCH] btrfs: Fix bio leak

[Nikolay Borisov]

Commit e0ae99941423 ("btrfs: preallocate device flush bio") reworked
the way the flush bio is allocated and used. Concretely it allocates
the bio in __alloc_device and then re-uses it multiple times with a
very simple endio routine that just calls complete() without consuming
a reference. Allocated bios by default come with a ref count of 1,
which is then consumed by the endio routine (or not, in which case they
should be bio_put by the caller). The way the impleementation works now
is that the flush bio has a refcount of 2 and we only ever bio_put it
once, leaving it to hang indefinitely. Fix this by removing the extra
bio_get in __alloc_device.

Fixes: e0ae99941423 ("btrfs: preallocate device flush bio")
Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
 fs/btrfs/volumes.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 132966950f5f..3690822d469a 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -312,7 +312,6 @@ static struct btrfs_device *__alloc_device(void)
 		kfree(dev);
 		return ERR_PTR(-ENOMEM);
 	}
-	bio_get(dev->flush_bio);
 
 	INIT_LIST_HEAD(&dev->dev_list);
 	INIT_LIST_HEAD(&dev->dev_alloc_list);
-- 
2.7.4

Re: [PATCH] btrfs: Fix bio leak

[Liu Bo]

On Wed, Dec 13, 2017 at 01:50:07PM +0200, Nikolay Borisov wrote:
> Commit e0ae99941423 ("btrfs: preallocate device flush bio") reworked
> the way the flush bio is allocated and used. Concretely it allocates
> the bio in __alloc_device and then re-uses it multiple times with a
> very simple endio routine that just calls complete() without consuming
> a reference. Allocated bios by default come with a ref count of 1,
> which is then consumed by the endio routine (or not, in which case they
> should be bio_put by the caller). The way the impleementation works now
> is that the flush bio has a refcount of 2 and we only ever bio_put it
> once, leaving it to hang indefinitely. Fix this by removing the extra
> bio_get in __alloc_device.
> 

Reviewed-by: Liu Bo <bo.li.liu@oracle.com>

Thanks,

-liubo
> Fixes: e0ae99941423 ("btrfs: preallocate device flush bio")
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
> ---
>  fs/btrfs/volumes.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index 132966950f5f..3690822d469a 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -312,7 +312,6 @@ static struct btrfs_device *__alloc_device(void)
>  		kfree(dev);
>  		return ERR_PTR(-ENOMEM);
>  	}
> -	bio_get(dev->flush_bio);
>  
>  	INIT_LIST_HEAD(&dev->dev_list);
>  	INIT_LIST_HEAD(&dev->dev_alloc_list);
> -- 
> 2.7.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] btrfs: Fix bio leak

[David Sterba]

On Wed, Dec 13, 2017 at 01:50:07PM +0200, Nikolay Borisov wrote:
> Commit e0ae99941423 ("btrfs: preallocate device flush bio") reworked
> the way the flush bio is allocated and used. Concretely it allocates
> the bio in __alloc_device and then re-uses it multiple times with a
> very simple endio routine that just calls complete() without consuming
> a reference. Allocated bios by default come with a ref count of 1,
> which is then consumed by the endio routine (or not, in which case they
> should be bio_put by the caller). The way the impleementation works now
> is that the flush bio has a refcount of 2 and we only ever bio_put it
> once, leaving it to hang indefinitely. Fix this by removing the extra
> bio_get in __alloc_device.
> 
> Fixes: e0ae99941423 ("btrfs: preallocate device flush bio")

Sigh, so many bugs. Hopefully that was the last one.

> Signed-off-by: Nikolay Borisov <nborisov@suse.com>

Reviewed-by: David Sterba <dsterba@suse.com>