btrfs: don't double unlock on error in btrfs_punch_hole
diff mbox series

Message ID 20190503151007.75525-1-josef@toxicpanda.com
State New
Headers show
Series
  • btrfs: don't double unlock on error in btrfs_punch_hole
Related show

Commit Message

Josef Bacik May 3, 2019, 3:10 p.m. UTC
If we have an error writing out a delalloc range in
btrfs_punch_hole_lock_range we'll unlock the inode and then goto
out_only_mutex, where we will again unlock the inode.  This is bad,
don't do this.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 fs/btrfs/file.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

Comments

Filipe Manana May 3, 2019, 3:25 p.m. UTC | #1
On Fri, May 3, 2019 at 4:21 PM Josef Bacik <josef@toxicpanda.com> wrote:
>
> If we have an error writing out a delalloc range in
> btrfs_punch_hole_lock_range we'll unlock the inode and then goto
> out_only_mutex, where we will again unlock the inode.  This is bad,
> don't do this.
>
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>

Reviewed-by: Filipe Manana <fdmanana@suse.com>

Looks good, I introduced the double unlock accidentally.

> ---
>  fs/btrfs/file.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
> index 7f7833149cb7..d23ea0b388e0 100644
> --- a/fs/btrfs/file.c
> +++ b/fs/btrfs/file.c
> @@ -2554,10 +2554,8 @@ static int btrfs_punch_hole(struct inode *inode, loff_t offset, loff_t len)
>
>         ret = btrfs_punch_hole_lock_range(inode, lockstart, lockend,
>                                           &cached_state);
> -       if (ret) {
> -               inode_unlock(inode);
> +       if (ret)
>                 goto out_only_mutex;
> -       }
>
>         path = btrfs_alloc_path();
>         if (!path) {
> --
> 2.13.5
>
Rik van Riel May 3, 2019, 3:31 p.m. UTC | #2
On Fri, 2019-05-03 at 16:25 +0100, Filipe Manana wrote:
> On Fri, May 3, 2019 at 4:21 PM Josef Bacik <josef@toxicpanda.com>
> wrote:
> > If we have an error writing out a delalloc range in
> > btrfs_punch_hole_lock_range we'll unlock the inode and then goto
> > out_only_mutex, where we will again unlock the inode.  This is bad,
> > don't do this.
> > 
> > Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> 
> Reviewed-by: Filipe Manana <fdmanana@suse.com>
> 
> Looks good, I introduced the double unlock accidentally.

Does the patch need a Fixes: tag so the -stable
people know how far to backport it?
David Sterba May 3, 2019, 3:38 p.m. UTC | #3
On Fri, May 03, 2019 at 03:31:33PM +0000, Rik van Riel wrote:
> On Fri, 2019-05-03 at 16:25 +0100, Filipe Manana wrote:
> > On Fri, May 3, 2019 at 4:21 PM Josef Bacik <josef@toxicpanda.com>
> > wrote:
> > > If we have an error writing out a delalloc range in
> > > btrfs_punch_hole_lock_range we'll unlock the inode and then goto
> > > out_only_mutex, where we will again unlock the inode.  This is bad,
> > > don't do this.
> > > 
> > > Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> > 
> > Reviewed-by: Filipe Manana <fdmanana@suse.com>
> > 
> > Looks good, I introduced the double unlock accidentally.
> 
> Does the patch need a Fixes: tag so the -stable
> people know how far to backport it?

I add the tags.
David Sterba May 3, 2019, 4:18 p.m. UTC | #4
On Fri, May 03, 2019 at 11:10:06AM -0400, Josef Bacik wrote:
> If we have an error writing out a delalloc range in
> btrfs_punch_hole_lock_range we'll unlock the inode and then goto
> out_only_mutex, where we will again unlock the inode.  This is bad,
> don't do this.
> 
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>

Added to misc-next, thanks.

Patch
diff mbox series

diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
index 7f7833149cb7..d23ea0b388e0 100644
--- a/fs/btrfs/file.c
+++ b/fs/btrfs/file.c
@@ -2554,10 +2554,8 @@  static int btrfs_punch_hole(struct inode *inode, loff_t offset, loff_t len)
 
 	ret = btrfs_punch_hole_lock_range(inode, lockstart, lockend,
 					  &cached_state);
-	if (ret) {
-		inode_unlock(inode);
+	if (ret)
 		goto out_only_mutex;
-	}
 
 	path = btrfs_alloc_path();
 	if (!path) {