[PATCH 0/3] Minor UUID cleanup

[PATCH 0/3] Minor UUID cleanup

[Nikolay Borisov]

Here is a short series that removes one indirect call in btrfs_uuid_tree_iterate
and making uuid rescan functions private to disk-io.c since they are used only
during mount. 

Nikolay Borisov (3):
  btrfs: Call btrfs_check_uuid_tree_entry directly in
    btrfs_uuid_tree_iterate
  btrfs: export btrfs_uuid_scan_kthread
  btrfs: Make btrfs_check_uuid_tree private to disk-io.c

 fs/btrfs/ctree.h     |  4 +--
 fs/btrfs/disk-io.c   | 35 +++++++++++++++++++
 fs/btrfs/uuid-tree.c | 50 ++++++++++++++++++++++++---
 fs/btrfs/volumes.c   | 82 +-------------------------------------------
 fs/btrfs/volumes.h   |  2 +-
 5 files changed, 84 insertions(+), 89 deletions(-)

-- 
2.17.1

[PATCH 1/3] btrfs: Call btrfs_check_uuid_tree_entry directly in btrfs_uuid_tree_iterate

[Nikolay Borisov]

btrfs_uuid_tree_iterate is called from only once place and its 2nd
argument is always btrfs_check_uuid_tree_entry. Simplify
btrfs_uuid_tree_iterate's signature by removing its 2nd argument and
directly calling btrfs_check_uuid_tree_entry. Also move the latter
into uuid-tree.h. No functional changes.
---
 fs/btrfs/ctree.h     |  4 +---
 fs/btrfs/uuid-tree.c | 50 ++++++++++++++++++++++++++++++++++++++++----
 fs/btrfs/volumes.c   | 47 +----------------------------------------
 3 files changed, 48 insertions(+), 53 deletions(-)

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index bb237d577725..ad275d06e95f 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -2738,9 +2738,7 @@ int btrfs_uuid_tree_add(struct btrfs_trans_handle *trans, u8 *uuid, u8 type,
 			u64 subid);
 int btrfs_uuid_tree_remove(struct btrfs_trans_handle *trans, u8 *uuid, u8 type,
 			u64 subid);
-int btrfs_uuid_tree_iterate(struct btrfs_fs_info *fs_info,
-			    int (*check_func)(struct btrfs_fs_info *, u8 *, u8,
-					      u64));
+int btrfs_uuid_tree_iterate(struct btrfs_fs_info *fs_info);
 
 /* dir-item.c */
 int btrfs_check_dir_item_collision(struct btrfs_root *root, u64 dir,
diff --git a/fs/btrfs/uuid-tree.c b/fs/btrfs/uuid-tree.c
index 76b84f2397b1..e25776359fad 100644
--- a/fs/btrfs/uuid-tree.c
+++ b/fs/btrfs/uuid-tree.c
@@ -245,10 +245,51 @@ static int btrfs_uuid_iter_rem(struct btrfs_root *uuid_root, u8 *uuid, u8 type,
 out:
 	return ret;
 }
+/*
+ * returns:
+ * 0	check succeeded, the entry is not outdated.
+ * < 0	if an error occurred.
+ * > 0	if the check failed, which means the caller shall remove the entry.
+ */
+static int btrfs_check_uuid_tree_entry(struct btrfs_fs_info *fs_info,
+				       u8 *uuid, u8 type, u64 subid)
+{
+	struct btrfs_key key;
+	int ret = 0;
+	struct btrfs_root *subvol_root;
+
+	if (type != BTRFS_UUID_KEY_SUBVOL &&
+	    type != BTRFS_UUID_KEY_RECEIVED_SUBVOL)
+		goto out;
+
+	key.objectid = subid;
+	key.type = BTRFS_ROOT_ITEM_KEY;
+	key.offset = (u64)-1;
+	subvol_root = btrfs_get_fs_root(fs_info, &key, true);
+	if (IS_ERR(subvol_root)) {
+		ret = PTR_ERR(subvol_root);
+		if (ret == -ENOENT)
+			ret = 1;
+		goto out;
+	}
+
+	switch (type) {
+	case BTRFS_UUID_KEY_SUBVOL:
+		if (memcmp(uuid, subvol_root->root_item.uuid, BTRFS_UUID_SIZE))
+			ret = 1;
+		break;
+	case BTRFS_UUID_KEY_RECEIVED_SUBVOL:
+		if (memcmp(uuid, subvol_root->root_item.received_uuid,
+			   BTRFS_UUID_SIZE))
+			ret = 1;
+		break;
+	}
+	btrfs_put_root(subvol_root);
+out:
+	return ret;
+}
 
-int btrfs_uuid_tree_iterate(struct btrfs_fs_info *fs_info,
-			    int (*check_func)(struct btrfs_fs_info *, u8 *, u8,
-					      u64))
+int btrfs_uuid_tree_iterate(struct btrfs_fs_info *fs_info)
 {
 	struct btrfs_root *root = fs_info->uuid_root;
 	struct btrfs_key key;
@@ -305,7 +346,8 @@ int btrfs_uuid_tree_iterate(struct btrfs_fs_info *fs_info,
 			read_extent_buffer(leaf, &subid_le, offset,
 					   sizeof(subid_le));
 			subid_cpu = le64_to_cpu(subid_le);
-			ret = check_func(fs_info, uuid, key.type, subid_cpu);
+			ret = btrfs_check_uuid_tree_entry(fs_info, uuid,
+							  key.type, subid_cpu);
 			if (ret < 0)
 				goto out;
 			if (ret > 0) {
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 387f80656476..fdc0e52542dd 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -4403,51 +4403,6 @@ static int btrfs_uuid_scan_kthread(void *data)
 	return 0;
 }
 
-/*
- * Callback for btrfs_uuid_tree_iterate().
- * returns:
- * 0	check succeeded, the entry is not outdated.
- * < 0	if an error occurred.
- * > 0	if the check failed, which means the caller shall remove the entry.
- */
-static int btrfs_check_uuid_tree_entry(struct btrfs_fs_info *fs_info,
-				       u8 *uuid, u8 type, u64 subid)
-{
-	struct btrfs_key key;
-	int ret = 0;
-	struct btrfs_root *subvol_root;
-
-	if (type != BTRFS_UUID_KEY_SUBVOL &&
-	    type != BTRFS_UUID_KEY_RECEIVED_SUBVOL)
-		goto out;
-
-	key.objectid = subid;
-	key.type = BTRFS_ROOT_ITEM_KEY;
-	key.offset = (u64)-1;
-	subvol_root = btrfs_get_fs_root(fs_info, &key, true);
-	if (IS_ERR(subvol_root)) {
-		ret = PTR_ERR(subvol_root);
-		if (ret == -ENOENT)
-			ret = 1;
-		goto out;
-	}
-
-	switch (type) {
-	case BTRFS_UUID_KEY_SUBVOL:
-		if (memcmp(uuid, subvol_root->root_item.uuid, BTRFS_UUID_SIZE))
-			ret = 1;
-		break;
-	case BTRFS_UUID_KEY_RECEIVED_SUBVOL:
-		if (memcmp(uuid, subvol_root->root_item.received_uuid,
-			   BTRFS_UUID_SIZE))
-			ret = 1;
-		break;
-	}
-	btrfs_put_root(subvol_root);
-out:
-	return ret;
-}
-
 static int btrfs_uuid_rescan_kthread(void *data)
 {
 	struct btrfs_fs_info *fs_info = (struct btrfs_fs_info *)data;
@@ -4458,7 +4413,7 @@ static int btrfs_uuid_rescan_kthread(void *data)
 	 * to delete all entries that contain outdated data.
 	 * 2nd step is to add all missing entries to the UUID tree.
 	 */
-	ret = btrfs_uuid_tree_iterate(fs_info, btrfs_check_uuid_tree_entry);
+	ret = btrfs_uuid_tree_iterate(fs_info);
 	if (ret < 0) {
 		btrfs_warn(fs_info, "iterating uuid_tree failed %d", ret);
 		up(&fs_info->uuid_tree_rescan_sem);
-- 
2.17.1

[PATCH 2/3] btrfs: export btrfs_uuid_scan_kthread

[Nikolay Borisov]

This is preparation for the next patch that will move the rescan uuid
kthread into disk-io.c. No functional changes.
---
 fs/btrfs/volumes.c | 2 +-
 fs/btrfs/volumes.h | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index fdc0e52542dd..0b4da3557c27 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -4274,7 +4274,7 @@ int btrfs_cancel_balance(struct btrfs_fs_info *fs_info)
 	return 0;
 }
 
-static int btrfs_uuid_scan_kthread(void *data)
+int btrfs_uuid_scan_kthread(void *data)
 {
 	struct btrfs_fs_info *fs_info = data;
 	struct btrfs_root *root = fs_info->tree_root;
diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h
index f01552a0785e..311d03cf99a7 100644
--- a/fs/btrfs/volumes.h
+++ b/fs/btrfs/volumes.h
@@ -462,6 +462,7 @@ int btrfs_pause_balance(struct btrfs_fs_info *fs_info);
 int btrfs_cancel_balance(struct btrfs_fs_info *fs_info);
 int btrfs_create_uuid_tree(struct btrfs_fs_info *fs_info);
 int btrfs_check_uuid_tree(struct btrfs_fs_info *fs_info);
+int btrfs_uuid_scan_kthread(void *data);
 int btrfs_chunk_readonly(struct btrfs_fs_info *fs_info, u64 chunk_offset);
 int find_free_dev_extent(struct btrfs_device *device, u64 num_bytes,
 			 u64 *start, u64 *max_avail);
-- 
2.17.1

[PATCH 3/3] btrfs: Make btrfs_check_uuid_tree private to disk-io.c

[Nikolay Borisov]

It's used only during fs mount as such it can be made private to
disk-io.c file. Also use the occassion to move btrfs_uuid_rescan_kthread
as btrfs_check_uuid_tree is its sole caller. No functional changes.
---
 fs/btrfs/disk-io.c | 35 +++++++++++++++++++++++++++++++++++
 fs/btrfs/volumes.c | 35 -----------------------------------
 fs/btrfs/volumes.h |  1 -
 3 files changed, 35 insertions(+), 36 deletions(-)

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 9046bf0d4868..cf60385e0dfb 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -2836,6 +2836,41 @@ static int init_mount_fs_info(struct btrfs_fs_info *fs_info, struct super_block
 	return ret;
 }
 
+static int btrfs_uuid_rescan_kthread(void *data)
+{
+	struct btrfs_fs_info *fs_info = (struct btrfs_fs_info *)data;
+	int ret;
+
+	/*
+	 * 1st step is to iterate through the existing UUID tree and
+	 * to delete all entries that contain outdated data.
+	 * 2nd step is to add all missing entries to the UUID tree.
+	 */
+	ret = btrfs_uuid_tree_iterate(fs_info);
+	if (ret < 0) {
+		btrfs_warn(fs_info, "iterating uuid_tree failed %d", ret);
+		up(&fs_info->uuid_tree_rescan_sem);
+		return ret;
+	}
+	return btrfs_uuid_scan_kthread(data);
+}
+
+static int btrfs_check_uuid_tree(struct btrfs_fs_info *fs_info)
+{
+	struct task_struct *task;
+
+	down(&fs_info->uuid_tree_rescan_sem);
+	task = kthread_run(btrfs_uuid_rescan_kthread, fs_info, "btrfs-uuid");
+	if (IS_ERR(task)) {
+		/* fs_info->update_uuid_tree_gen remains 0 in all error case */
+		btrfs_warn(fs_info, "failed to start uuid_rescan task");
+		up(&fs_info->uuid_tree_rescan_sem);
+		return PTR_ERR(task);
+	}
+
+	return 0;
+}
+
 int __cold open_ctree(struct super_block *sb, struct btrfs_fs_devices *fs_devices,
 		      char *options)
 {
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 0b4da3557c27..91618be34e6c 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -4403,25 +4403,6 @@ int btrfs_uuid_scan_kthread(void *data)
 	return 0;
 }
 
-static int btrfs_uuid_rescan_kthread(void *data)
-{
-	struct btrfs_fs_info *fs_info = (struct btrfs_fs_info *)data;
-	int ret;
-
-	/*
-	 * 1st step is to iterate through the existing UUID tree and
-	 * to delete all entries that contain outdated data.
-	 * 2nd step is to add all missing entries to the UUID tree.
-	 */
-	ret = btrfs_uuid_tree_iterate(fs_info);
-	if (ret < 0) {
-		btrfs_warn(fs_info, "iterating uuid_tree failed %d", ret);
-		up(&fs_info->uuid_tree_rescan_sem);
-		return ret;
-	}
-	return btrfs_uuid_scan_kthread(data);
-}
-
 int btrfs_create_uuid_tree(struct btrfs_fs_info *fs_info)
 {
 	struct btrfs_trans_handle *trans;
@@ -4464,22 +4445,6 @@ int btrfs_create_uuid_tree(struct btrfs_fs_info *fs_info)
 	return 0;
 }
 
-int btrfs_check_uuid_tree(struct btrfs_fs_info *fs_info)
-{
-	struct task_struct *task;
-
-	down(&fs_info->uuid_tree_rescan_sem);
-	task = kthread_run(btrfs_uuid_rescan_kthread, fs_info, "btrfs-uuid");
-	if (IS_ERR(task)) {
-		/* fs_info->update_uuid_tree_gen remains 0 in all error case */
-		btrfs_warn(fs_info, "failed to start uuid_rescan task");
-		up(&fs_info->uuid_tree_rescan_sem);
-		return PTR_ERR(task);
-	}
-
-	return 0;
-}
-
 /*
  * shrinking a device means finding all of the device extents past
  * the new size, and then following the back refs to the chunks.
diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h
index 311d03cf99a7..bb0fc7b35ba0 100644
--- a/fs/btrfs/volumes.h
+++ b/fs/btrfs/volumes.h
@@ -461,7 +461,6 @@ int btrfs_recover_balance(struct btrfs_fs_info *fs_info);
 int btrfs_pause_balance(struct btrfs_fs_info *fs_info);
 int btrfs_cancel_balance(struct btrfs_fs_info *fs_info);
 int btrfs_create_uuid_tree(struct btrfs_fs_info *fs_info);
-int btrfs_check_uuid_tree(struct btrfs_fs_info *fs_info);
 int btrfs_uuid_scan_kthread(void *data);
 int btrfs_chunk_readonly(struct btrfs_fs_info *fs_info, u64 chunk_offset);
 int find_free_dev_extent(struct btrfs_device *device, u64 num_bytes,
-- 
2.17.1

Re: [PATCH 1/3] btrfs: Call btrfs_check_uuid_tree_entry directly in btrfs_uuid_tree_iterate

[Johannes Thumshirn]

Looks good,
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>

Re: [PATCH 2/3] btrfs: export btrfs_uuid_scan_kthread

[Johannes Thumshirn]

I'd squash this into the next patch

Re: [PATCH 1/3] btrfs: Call btrfs_check_uuid_tree_entry directly in btrfs_uuid_tree_iterate

[Josef Bacik]

On 2/18/20 9:56 AM, Nikolay Borisov wrote:
> btrfs_uuid_tree_iterate is called from only once place and its 2nd
> argument is always btrfs_check_uuid_tree_entry. Simplify
> btrfs_uuid_tree_iterate's signature by removing its 2nd argument and
> directly calling btrfs_check_uuid_tree_entry. Also move the latter
> into uuid-tree.h. No functional changes.

You forgot your signed-off-by

Reviewed-by: Josef Bacik <josef@toxicpanda.com>

Thanks,

Josef

Re: [PATCH 3/3] btrfs: Make btrfs_check_uuid_tree private to disk-io.c

[Josef Bacik]

On 2/18/20 9:56 AM, Nikolay Borisov wrote:
> It's used only during fs mount as such it can be made private to
> disk-io.c file. Also use the occassion to move btrfs_uuid_rescan_kthread
> as btrfs_check_uuid_tree is its sole caller. No functional changes.
> ---

Forgot your signed-off-by here as well.  And it's "occasion".

Reviewed-by: Josef Bacik <josef@toxicpanda.com>

Thanks,

Josef

Re: [PATCH 0/3] Minor UUID cleanup

[David Sterba]

On Tue, Feb 18, 2020 at 04:56:06PM +0200, Nikolay Borisov wrote:
> Here is a short series that removes one indirect call in btrfs_uuid_tree_iterate
> and making uuid rescan functions private to disk-io.c since they are used only
> during mount. 
> 
> Nikolay Borisov (3):
>   btrfs: Call btrfs_check_uuid_tree_entry directly in
>     btrfs_uuid_tree_iterate
>   btrfs: export btrfs_uuid_scan_kthread
>   btrfs: Make btrfs_check_uuid_tree private to disk-io.c

With the fixups applied, added to misc-next. Thanks.