diff mbox

btrfs: Fix bio leak

Message ID 1513165807-12261-1-git-send-email-nborisov@suse.com (mailing list archive)
State New, archived
Headers show

Commit Message

Nikolay Borisov Dec. 13, 2017, 11:50 a.m. UTC
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(-)

Comments

Liu Bo Dec. 13, 2017, 5 p.m. UTC | #1
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
--
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
David Sterba Dec. 14, 2017, 3:16 p.m. UTC | #2
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>
--
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
diff mbox

Patch

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);