[PATCH] btrfs: Remove pair of bio_get/put in btrfs_schedule_bio

[PATCH] btrfs: Remove pair of bio_get/put in btrfs_schedule_bio

[Nikolay Borisov]

This code was added in 492bb6deee34 ("Btrfs: Hold a reference on bios
during submit_bio, add some extra bio checks"). However, holding a
reference on a bio is necessary only if it's going to be referenced
after the submit_bio returns and the bio is completed. In this
particular instance this is not the case so there is no need to hold
an extra reference since we directly return.

Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
 fs/btrfs/volumes.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 49810b70afd3..13827ff648ec 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -6069,9 +6069,7 @@ static noinline void btrfs_schedule_bio(struct btrfs_device *device,
 
 	/* don't bother with additional async steps for reads, right now */
 	if (bio_op(bio) == REQ_OP_READ) {
-		bio_get(bio);
 		btrfsic_submit_bio(bio);
-		bio_put(bio);
 		return;
 	}
 
-- 
2.7.4

Re: [PATCH] btrfs: Remove pair of bio_get/put in btrfs_schedule_bio

[Liu Bo]

On Mon, Dec 11, 2017 at 04:38:48PM +0200, Nikolay Borisov wrote:
> This code was added in 492bb6deee34 ("Btrfs: Hold a reference on bios
> during submit_bio, add some extra bio checks"). However, holding a
> reference on a bio is necessary only if it's going to be referenced
> after the submit_bio returns and the bio is completed. In this
> particular instance this is not the case so there is no need to hold
> an extra reference since we directly return.
>

Looks good.  If possible, please also drop other unnecessary bio_get()
in btrfs.

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

-liubo
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
> ---
>  fs/btrfs/volumes.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index 49810b70afd3..13827ff648ec 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -6069,9 +6069,7 @@ static noinline void btrfs_schedule_bio(struct btrfs_device *device,
>  
>  	/* don't bother with additional async steps for reads, right now */
>  	if (bio_op(bio) == REQ_OP_READ) {
> -		bio_get(bio);
>  		btrfsic_submit_bio(bio);
> -		bio_put(bio);
>  		return;
>  	}
>  
> -- 
> 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: Remove pair of bio_get/put in btrfs_schedule_bio

[David Sterba]

On Mon, Dec 11, 2017 at 02:57:56PM -0800, Liu Bo wrote:
> On Mon, Dec 11, 2017 at 04:38:48PM +0200, Nikolay Borisov wrote:
> > This code was added in 492bb6deee34 ("Btrfs: Hold a reference on bios
> > during submit_bio, add some extra bio checks"). However, holding a
> > reference on a bio is necessary only if it's going to be referenced
> > after the submit_bio returns and the bio is completed. In this
> > particular instance this is not the case so there is no need to hold
> > an extra reference since we directly return.
> >
> 
> Looks good.  If possible, please also drop other unnecessary bio_get()
> in btrfs.

You mean everywhere with similar context like in this patch?

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

Added to next, thanks.

Re: [PATCH] btrfs: Remove pair of bio_get/put in btrfs_schedule_bio

[Liu Bo]

On Tue, Dec 12, 2017 at 07:52:38PM +0100, David Sterba wrote:
> On Mon, Dec 11, 2017 at 02:57:56PM -0800, Liu Bo wrote:
> > On Mon, Dec 11, 2017 at 04:38:48PM +0200, Nikolay Borisov wrote:
> > > This code was added in 492bb6deee34 ("Btrfs: Hold a reference on bios
> > > during submit_bio, add some extra bio checks"). However, holding a
> > > reference on a bio is necessary only if it's going to be referenced
> > > after the submit_bio returns and the bio is completed. In this
> > > particular instance this is not the case so there is no need to hold
> > > an extra reference since we directly return.
> > >
> > 
> > Looks good.  If possible, please also drop other unnecessary bio_get()
> > in btrfs.
> 
> You mean everywhere with similar context like in this patch?
>

Right, there're several places where bio_get() is put before running
functions but bio_put() is also put immediately without referencing
bio.

thanks,
-liubo

Re: [PATCH] btrfs: Remove pair of bio_get/put in btrfs_schedule_bio

[Nikolay Borisov]

On 13.12.2017 00:09, Liu Bo wrote:
> On Tue, Dec 12, 2017 at 07:52:38PM +0100, David Sterba wrote:
>> On Mon, Dec 11, 2017 at 02:57:56PM -0800, Liu Bo wrote:
>>> On Mon, Dec 11, 2017 at 04:38:48PM +0200, Nikolay Borisov wrote:
>>>> This code was added in 492bb6deee34 ("Btrfs: Hold a reference on bios
>>>> during submit_bio, add some extra bio checks"). However, holding a
>>>> reference on a bio is necessary only if it's going to be referenced
>>>> after the submit_bio returns and the bio is completed. In this
>>>> particular instance this is not the case so there is no need to hold
>>>> an extra reference since we directly return.
>>>>
>>>
>>> Looks good.  If possible, please also drop other unnecessary bio_get()
>>> in btrfs.
>>
>> You mean everywhere with similar context like in this patch?
>>
> 
> Right, there're several places where bio_get() is put before running
> functions but bio_put() is also put immediately without referencing
> bio.

I've already created patches for those and put them through fstests.
Will send them later

> 
> thanks,
> -liubo
>