diff mbox

[2/2,btrfs] btrfs_rename(): don't ignore btrfs_end_transaction() return

Message ID 1435637715-16332-1-git-send-email-dccitaliano@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Davide Italiano June 30, 2015, 4:15 a.m. UTC 
From: Davide Italiano <dccitaliano@gmail.com>

btrfs_end_transaction() can return an error -- this happens, e.g.
if it tries to commit and the transaction was aborted in the meanhwile.
Swallowing the error is wrong, so explicitly return it.

Signed-off-by: Davide Italiano <dccitaliano@gmail.com>
---
 fs/btrfs/inode.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Filipe Manana June 30, 2015, 8:24 a.m. UTC | #1 
On Tue, Jun 30, 2015 at 5:15 AM, Davide C. C. Italiano
<dccitaliano@gmail.com> wrote:
> From: Davide Italiano <dccitaliano@gmail.com>
>
> btrfs_end_transaction() can return an error -- this happens, e.g.
> if it tries to commit and the transaction was aborted in the meanhwile.
> Swallowing the error is wrong, so explicitly return it.
>
> Signed-off-by: Davide Italiano <dccitaliano@gmail.com>
> ---
>  fs/btrfs/inode.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index 59c475c..61b26be 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -9199,7 +9199,8 @@ static int btrfs_rename(struct inode *old_dir, struct dentry *old_dentry,
>                 btrfs_end_log_trans(root);
>         }
>  out_fail:
> -       btrfs_end_transaction(trans, root);
> +       if (!ret)
> +               ret = btrfs_end_transaction(trans, root);

So it an error happened before, we still want to call
btrfs_end_transaction(), otherwise the transaction's refcount never
drops to 0 and its resources are never freed (memory).

Something like this:

if (ret)
    btrfs_end_transaction(trans, root);
else
    ret = btrfs_end_transaction(trans, root);

or

int ret2;

ret2 = btrfs_end_transaction(trans, root);
if (!ret)
    ret = ret2;

Also don't forget to target your patches with V<number> and describe
and what changed between patch versions (see
https://btrfs.wiki.kernel.org/index.php/Writing_patch_for_btrfs for
instructions about how to do it).

thanks

>  out_notrans:
>         if (old_ino == BTRFS_FIRST_FREE_OBJECTID)
>                 up_read(&root->fs_info->subvol_sem);
> --
> 2.4.3
>
diff mbox

Patch

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 59c475c..61b26be 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -9199,7 +9199,8 @@  static int btrfs_rename(struct inode *old_dir, struct dentry *old_dentry,
 		btrfs_end_log_trans(root);
 	}
 out_fail:
-	btrfs_end_transaction(trans, root);
+	if (!ret)
+		ret = btrfs_end_transaction(trans, root);
 out_notrans:
 	if (old_ino == BTRFS_FIRST_FREE_OBJECTID)
 		up_read(&root->fs_info->subvol_sem);