Btrfs: hold the commit_root_sem when getting the commit root during send

We currently rely too heavily on roots being read-only to save us from just accessing root->commit_root. We can easily balance blocks out from underneath a read only root, so to save us from getting screwed make sure we only access root->commit_root under the commit root sem. Thanks, Signed-off-by: Josef Bacik <jbacik@fb.com> Signed-off-by: Chris Mason <clm@fb.com>

Btrfs: hold the commit_root_sem when getting the commit root during send
We currently rely too heavily on roots being read-only to save us from just accessing root->commit_root. We can easily balance blocks out from underneath a read only root, so to save us from getting screwed make sure we only access root->commit_root under the commit root sem. Thanks, Signed-off-by: Josef Bacik <jbacik@fb.com> Signed-off-by: Chris Mason <clm@fb.com>
Josef Bacik · Chris Mason
1 parent 9e351cc862
Showing 3 changed files with 39 additions and 16 deletions Side-by-side Diff
fs/btrfs/ctree.c
fs/btrfs/ctree.h
fs/btrfs/send.c
@@ -2769,9 +2769,13 @@
 		 * the commit roots are read only
 		 * so we always do read locks
 		 */
+		if (p->need_commit_sem)
+			down_read(&root->fs_info->commit_root_sem);
 		b = root->commit_root;
 		extent_buffer_get(b);
 		level = btrfs_header_level(b);
+		if (p->need_commit_sem)
+			up_read(&root->fs_info->commit_root_sem);
 		if (!p->skip_locking)
 			btrfs_tree_read_lock(b);
 	} else {
@@ -5436,6 +5440,7 @@
 	 *   the right if possible or go up and right.
 	 */
  
+	down_read(&left_root->fs_info->commit_root_sem);
 	left_level = btrfs_header_level(left_root->commit_root);
 	left_root_level = left_level;
 	left_path->nodes[left_level] = left_root->commit_root;
@@ -5445,6 +5450,7 @@
 	right_root_level = right_level;
 	right_path->nodes[right_level] = right_root->commit_root;
 	extent_buffer_get(right_path->nodes[right_level]);
+	up_read(&left_root->fs_info->commit_root_sem);
  
 	if (left_level == 0)
 		btrfs_item_key_to_cpu(left_path->nodes[left_level],
@@ -609,6 +609,7 @@
 	unsigned int skip_locking:1;
 	unsigned int leave_spinning:1;
 	unsigned int search_commit_root:1;
+	unsigned int need_commit_sem:1;
 };
  
 /*
@@ -493,6 +493,7 @@
 		return NULL;
 	path->search_commit_root = 1;
 	path->skip_locking = 1;
+	path->need_commit_sem = 1;
 	return path;
 }
  
  
  
  
  
@@ -771,29 +772,22 @@
 /*
  * Helper function to retrieve some fields from an inode item.
  */
-static int get_inode_info(struct btrfs_root *root,
-			  u64 ino, u64 *size, u64 *gen,
-			  u64 *mode, u64 *uid, u64 *gid,
-			  u64 *rdev)
+static int __get_inode_info(struct btrfs_root *root, struct btrfs_path *path,
+			  u64 ino, u64 *size, u64 *gen, u64 *mode, u64 *uid,
+			  u64 *gid, u64 *rdev)
 {
 	int ret;
 	struct btrfs_inode_item *ii;
 	struct btrfs_key key;
-	struct btrfs_path *path;
  
-	path = alloc_path_for_send();
-	if (!path)
-		return -ENOMEM;
-
 	key.objectid = ino;
 	key.type = BTRFS_INODE_ITEM_KEY;
 	key.offset = 0;
 	ret = btrfs_search_slot(NULL, root, &key, path, 0, 0);
-	if (ret < 0)
-		goto out;
 	if (ret) {
-		ret = -ENOENT;
-		goto out;
+		if (ret > 0)
+			ret = -ENOENT;
+		return ret;
 	}
  
 	ii = btrfs_item_ptr(path->nodes[0], path->slots[0],
@@ -811,7 +805,22 @@
 	if (rdev)
 		*rdev = btrfs_inode_rdev(path->nodes[0], ii);
  
-out:
+	return ret;
+}
+
+static int get_inode_info(struct btrfs_root *root,
+			  u64 ino, u64 *size, u64 *gen,
+			  u64 *mode, u64 *uid, u64 *gid,
+			  u64 *rdev)
+{
+	struct btrfs_path *path;
+	int ret;
+
+	path = alloc_path_for_send();
+	if (!path)
+		return -ENOMEM;
+	ret = __get_inode_info(root, path, ino, size, gen, mode, uid, gid,
+			       rdev);
 	btrfs_free_path(path);
 	return ret;
 }
@@ -1085,6 +1094,7 @@
 struct backref_ctx {
 	struct send_ctx *sctx;
  
+	struct btrfs_path *path;
 	/* number of total found references */
 	u64 found;
  
@@ -1155,8 +1165,9 @@
 	 * There are inodes that have extents that lie behind its i_size. Don't
 	 * accept clones from these extents.
 	 */
-	ret = get_inode_info(found->root, ino, &i_size, NULL, NULL, NULL, NULL,
-			NULL);
+	ret = __get_inode_info(found->root, bctx->path, ino, &i_size, NULL, NULL,
+			       NULL, NULL, NULL);
+	btrfs_release_path(bctx->path);
 	if (ret < 0)
 		return ret;
  
  
@@ -1235,11 +1246,16 @@
 	if (!tmp_path)
 		return -ENOMEM;
  
+	/* We only use this path under the commit sem */
+	tmp_path->need_commit_sem = 0;
+
 	backref_ctx = kmalloc(sizeof(*backref_ctx), GFP_NOFS);
 	if (!backref_ctx) {
 		ret = -ENOMEM;
 		goto out;
 	}
+
+	backref_ctx->path = tmp_path;
  
 	if (data_offset >= ino_size) {
 		/*
...	...	@@ -2769,9 +2769,13 @@
2769	2769	* the commit roots are read only
2770	2770	* so we always do read locks
2771	2771	*/
	2772	+ if (p->need_commit_sem)
	2773	+ down_read(&root->fs_info->commit_root_sem);
2772	2774	b = root->commit_root;
2773	2775	extent_buffer_get(b);
2774	2776	level = btrfs_header_level(b);
	2777	+ if (p->need_commit_sem)
	2778	+ up_read(&root->fs_info->commit_root_sem);
2775	2779	if (!p->skip_locking)
2776	2780	btrfs_tree_read_lock(b);
2777	2781	} else {
...	...	@@ -5436,6 +5440,7 @@
5436	5440	* the right if possible or go up and right.
5437	5441	*/
5438	5442
	5443	+ down_read(&left_root->fs_info->commit_root_sem);
5439	5444	left_level = btrfs_header_level(left_root->commit_root);
5440	5445	left_root_level = left_level;
5441	5446	left_path->nodes[left_level] = left_root->commit_root;
...	...	@@ -5445,6 +5450,7 @@
5445	5450	right_root_level = right_level;
5446	5451	right_path->nodes[right_level] = right_root->commit_root;
5447	5452	extent_buffer_get(right_path->nodes[right_level]);
	5453	+ up_read(&left_root->fs_info->commit_root_sem);
5448	5454
5449	5455	if (left_level == 0)
5450	5456	btrfs_item_key_to_cpu(left_path->nodes[left_level],
...	...	@@ -609,6 +609,7 @@
609	609	unsigned int skip_locking:1;
610	610	unsigned int leave_spinning:1;
611	611	unsigned int search_commit_root:1;
	612	+ unsigned int need_commit_sem:1;
612	613	};
613	614
614	615	/*
...	...	@@ -493,6 +493,7 @@
493	493	return NULL;
494	494	path->search_commit_root = 1;
495	495	path->skip_locking = 1;
	496	+ path->need_commit_sem = 1;
496	497	return path;
497	498	}
498	499
499	500
500	501
501	502
502	503
...	...	@@ -771,29 +772,22 @@
771	772	/*
772	773	* Helper function to retrieve some fields from an inode item.
773	774	*/
774		-static int get_inode_info(struct btrfs_root *root,
775		- u64 ino, u64 size, u64 gen,
776		- u64 mode, u64 uid, u64 *gid,
777		- u64 *rdev)
	775	+static int __get_inode_info(struct btrfs_root root, struct btrfs_path path,
	776	+ u64 ino, u64 size, u64 gen, u64 mode, u64 uid,
	777	+ u64 gid, u64 rdev)
778	778	{
779	779	int ret;
780	780	struct btrfs_inode_item *ii;
781	781	struct btrfs_key key;
782		- struct btrfs_path *path;
783	782
784		- path = alloc_path_for_send();
785		- if (!path)
786		- return -ENOMEM;
787		-
788	783	key.objectid = ino;
789	784	key.type = BTRFS_INODE_ITEM_KEY;
790	785	key.offset = 0;
791	786	ret = btrfs_search_slot(NULL, root, &key, path, 0, 0);
792		- if (ret < 0)
793		- goto out;
794	787	if (ret) {
795		- ret = -ENOENT;
796		- goto out;
	788	+ if (ret > 0)
	789	+ ret = -ENOENT;
	790	+ return ret;
797	791	}
798	792
799	793	ii = btrfs_item_ptr(path->nodes[0], path->slots[0],
...	...	@@ -811,7 +805,22 @@
811	805	if (rdev)
812	806	*rdev = btrfs_inode_rdev(path->nodes[0], ii);
813	807
814		-out:
	808	+ return ret;
	809	+}
	810	+
	811	+static int get_inode_info(struct btrfs_root *root,
	812	+ u64 ino, u64 size, u64 gen,
	813	+ u64 mode, u64 uid, u64 *gid,
	814	+ u64 *rdev)
	815	+{
	816	+ struct btrfs_path *path;
	817	+ int ret;
	818	+
	819	+ path = alloc_path_for_send();
	820	+ if (!path)
	821	+ return -ENOMEM;
	822	+ ret = __get_inode_info(root, path, ino, size, gen, mode, uid, gid,
	823	+ rdev);
815	824	btrfs_free_path(path);
816	825	return ret;
817	826	}
...	...	@@ -1085,6 +1094,7 @@
1085	1094	struct backref_ctx {
1086	1095	struct send_ctx *sctx;
1087	1096
	1097	+ struct btrfs_path *path;
1088	1098	/* number of total found references */
1089	1099	u64 found;
1090	1100
...	...	@@ -1155,8 +1165,9 @@
1155	1165	* There are inodes that have extents that lie behind its i_size. Don't
1156	1166	* accept clones from these extents.
1157	1167	*/
1158		- ret = get_inode_info(found->root, ino, &i_size, NULL, NULL, NULL, NULL,
1159		- NULL);
	1168	+ ret = __get_inode_info(found->root, bctx->path, ino, &i_size, NULL, NULL,
	1169	+ NULL, NULL, NULL);
	1170	+ btrfs_release_path(bctx->path);
1160	1171	if (ret < 0)
1161	1172	return ret;
1162	1173
1163	1174
...	...	@@ -1235,11 +1246,16 @@
1235	1246	if (!tmp_path)
1236	1247	return -ENOMEM;
1237	1248
	1249	+ /* We only use this path under the commit sem */
	1250	+ tmp_path->need_commit_sem = 0;
	1251	+
1238	1252	backref_ctx = kmalloc(sizeof(*backref_ctx), GFP_NOFS);
1239	1253	if (!backref_ctx) {
1240	1254	ret = -ENOMEM;
1241	1255	goto out;
1242	1256	}
	1257	+
	1258	+ backref_ctx->path = tmp_path;
1243	1259
1244	1260	if (data_offset >= ino_size) {
1245	1261	/*