[31/44] btrfs: hold a ref on the root in btrfs_ioctl_send

Commit Message

Josef Bacik Jan. 24, 2020, 2:32 p.m. UTC

We lookup all the clone roots and the parent root for send, so we need
to hold refs on all of these roots while we're processing them.

Signed-off-by: Josef Bacik <[e-mail]>
---
 fs/btrfs/send.c | 28 ++++++++++++++++++++++++----
 1 file changed, 24 insertions(+), 4 deletions(-)

Comments

David Sterba Feb. 5, 2020, 3:16 p.m. UTC | #1

On Fri, Jan 24, 2020 at 09:32:48AM -0500, Josef Bacik wrote:
> --- a/fs/btrfs/send.c
> +++ b/fs/btrfs/send.c
> @@ -7200,11 +7200,17 @@ long btrfs_ioctl_send(struct file *mnt_file, struct btrfs_ioctl_send_args *arg)
>  				ret = PTR_ERR(clone_root);
>  				goto out;
>  			}
> +			if (!btrfs_grab_fs_root(clone_root)) {
> +				srcu_read_unlock(&fs_info->subvol_srcu, index);
> +				ret = -ENOENT;
> +				goto out;
> +			}
>  			spin_lock(&clone_root->root_item_lock);
>  			if (!btrfs_root_readonly(clone_root) ||
>  			    btrfs_root_dead(clone_root)) {
>  				spin_unlock(&clone_root->root_item_lock);
>  				srcu_read_unlock(&fs_info->subvol_srcu, index);
> +				btrfs_put_fs_root(clone_root);

Here and

>  				ret = -EPERM;
>  				goto out;
>  			}
> @@ -7212,6 +7218,7 @@ long btrfs_ioctl_send(struct file *mnt_file, struct btrfs_ioctl_send_args *arg)
>  				dedupe_in_progress_warn(clone_root);
>  				spin_unlock(&clone_root->root_item_lock);
>  				srcu_read_unlock(&fs_info->subvol_srcu, index);
> +				btrfs_put_fs_root(clone_root);

here, the order is srcu, put ref. As it's on error handling path anyway
it's no big deal, but I'd rather swap them so the nesting is proper,
tree refs inside srcu section.

Patch

diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
index 57eae56dd743..ee2fc9ea9d7e 100644
--- a/fs/btrfs/send.c
+++ b/fs/btrfs/send.c
@@ -7200,11 +7200,17 @@  long btrfs_ioctl_send(struct file *mnt_file, struct btrfs_ioctl_send_args *arg)
 				ret = PTR_ERR(clone_root);
 				goto out;
 			}
+			if (!btrfs_grab_fs_root(clone_root)) {
+				srcu_read_unlock(&fs_info->subvol_srcu, index);
+				ret = -ENOENT;
+				goto out;
+			}
 			spin_lock(&clone_root->root_item_lock);
 			if (!btrfs_root_readonly(clone_root) ||
 			    btrfs_root_dead(clone_root)) {
 				spin_unlock(&clone_root->root_item_lock);
 				srcu_read_unlock(&fs_info->subvol_srcu, index);
+				btrfs_put_fs_root(clone_root);
 				ret = -EPERM;
 				goto out;
 			}
@@ -7212,6 +7218,7 @@  long btrfs_ioctl_send(struct file *mnt_file, struct btrfs_ioctl_send_args *arg)
 				dedupe_in_progress_warn(clone_root);
 				spin_unlock(&clone_root->root_item_lock);
 				srcu_read_unlock(&fs_info->subvol_srcu, index);
+				btrfs_put_fs_root(clone_root);
 				ret = -EAGAIN;
 				goto out;
 			}
@@ -7239,6 +7246,12 @@  long btrfs_ioctl_send(struct file *mnt_file, struct btrfs_ioctl_send_args *arg)
 			ret = PTR_ERR(sctx->parent_root);
 			goto out;
 		}
+		if (!btrfs_grab_fs_root(sctx->parent_root)) {
+			srcu_read_unlock(&fs_info->subvol_srcu, index);
+			ret = -ENOENT;
+			sctx->parent_root = ERR_PTR(ret);
+			goto out;
+		}
 
 		spin_lock(&sctx->parent_root->root_item_lock);
 		sctx->parent_root->send_in_progress++;
@@ -7266,7 +7279,8 @@  long btrfs_ioctl_send(struct file *mnt_file, struct btrfs_ioctl_send_args *arg)
 	 * is behind the current send position. This is checked while searching
 	 * for possible clone sources.
 	 */
-	sctx->clone_roots[sctx->clone_roots_cnt++].root = sctx->send_root;
+	sctx->clone_roots[sctx->clone_roots_cnt++].root =
+		btrfs_grab_fs_root(sctx->send_root);
 
 	/* We do a bsearch later */
 	sort(sctx->clone_roots, sctx->clone_roots_cnt,
@@ -7351,18 +7365,24 @@  long btrfs_ioctl_send(struct file *mnt_file, struct btrfs_ioctl_send_args *arg)
 	}
 
 	if (sort_clone_roots) {
-		for (i = 0; i < sctx->clone_roots_cnt; i++)
+		for (i = 0; i < sctx->clone_roots_cnt; i++) {
 			btrfs_root_dec_send_in_progress(
 					sctx->clone_roots[i].root);
+			btrfs_put_fs_root(sctx->clone_roots[i].root);
+		}
 	} else {
-		for (i = 0; sctx && i < clone_sources_to_rollback; i++)
+		for (i = 0; sctx && i < clone_sources_to_rollback; i++) {
 			btrfs_root_dec_send_in_progress(
 					sctx->clone_roots[i].root);
+			btrfs_put_fs_root(sctx->clone_roots[i].root);
+		}
 
 		btrfs_root_dec_send_in_progress(send_root);
 	}
-	if (sctx && !IS_ERR_OR_NULL(sctx->parent_root))
+	if (sctx && !IS_ERR_OR_NULL(sctx->parent_root)) {
 		btrfs_root_dec_send_in_progress(sctx->parent_root);
+		btrfs_put_fs_root(sctx->parent_root);
+	}
 
 	kvfree(clone_sources_tmp);