Discussion:
[PATCH 1/2] btrfs-progs: introduce btrfs_register_all_device()
(too old to reply)
Anand Jain
2014-10-15 00:51:09 UTC
Permalink
This function is to register all devices found after scanning
the system. Before we had this functionality with in the
btrfs_scan_lblkid(), however scanning and registering are two
different distinct operation its better keep them separate.
Also we want to optimize btrfs_scan_lblkid and avoid multiple
system scans unless needed. As of now device scan uses this function.

Signed-off-by: Anand Jain <***@oracle.com>
---
utils.h | 1 +
volumes.c | 25 +++++++++++++++++++++++++
volumes.h | 1 +
3 files changed, 27 insertions(+)

diff --git a/utils.h b/utils.h
index 3f7b388..237cf64 100644
--- a/utils.h
+++ b/utils.h
@@ -162,5 +162,6 @@ static inline u64 btrfs_min_dev_size(u32 leafsize)
return 2 * (BTRFS_MKFS_SYSTEM_GROUP_SIZE +
btrfs_min_global_blk_rsv_size(leafsize));
}
+int btrfs_register_one_device(char *fname);

#endif
diff --git a/volumes.c b/volumes.c
index 266a474..c8057c8 100644
--- a/volumes.c
+++ b/volumes.c
@@ -30,6 +30,7 @@
#include "print-tree.h"
#include "volumes.h"
#include "math.h"
+#include "utils.h"

struct stripe {
struct btrfs_device *dev;
@@ -1533,6 +1534,30 @@ btrfs_find_device_by_devid(struct btrfs_fs_devices *fs_devices,
return NULL;
}

+/*
+ * Register all devices in the fs_uuid list created in the user
+ * space. Ensure btrfs_scan_lblkid() is called before this func.
+ */
+int btrfs_register_all_devices(void)
+{
+ int err;
+ struct btrfs_fs_devices *fs_devices;
+ struct btrfs_device *device;
+
+ list_for_each_entry(fs_devices, &fs_uuids, list) {
+ list_for_each_entry(device, &fs_devices->devices, dev_list) {
+ if (strlen(device->name) != 0) {
+ err = btrfs_register_one_device(device->name);
+ if (err < 0)
+ return err;
+ if (err > 0)
+ return -err;
+ }
+ }
+ }
+ return 0;
+}
+
int btrfs_chunk_readonly(struct btrfs_root *root, u64 chunk_offset)
{
struct cache_extent *ce;
diff --git a/volumes.h b/volumes.h
index 447dd4b..ccba34b 100644
--- a/volumes.h
+++ b/volumes.h
@@ -195,4 +195,5 @@ int write_raid56_with_parity(struct btrfs_fs_info *info,
struct extent_buffer *eb,
struct btrfs_multi_bio *multi,
u64 stripe_len, u64 *raid_map);
+int btrfs_register_all_devices(void);
#endif
--
2.0.0.153.g79dcccc

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Anand Jain
2014-10-15 00:51:10 UTC
Permalink
btrfs_scan_lblikd is called by most the device related command functions.
And btrfs_scan_lblkid is most expensive function and it becomes more expensive
as number of devices in the system increase.

This patch will:
move out calling register_one_device with in btrfs_scan_lblkid()
and so function setting the BTRFS_UPDATE_KERNEL to yes will
call btrfs_register_all_devices() separately.

introduce a global variable scan_done, which is set when scan is
done succssfully per thread. So that following calls to this function
will just return success.

Further if any function needs to force scan after scan_done is set,
then it can be done when there is such a requirement, but as of now there
isn't any such requirement.

Signed-off-by: Anand Jain <***@oracle.com>
---
cmds-device.c | 6 +++++-
cmds-filesystem.c | 2 +-
disk-io.c | 2 +-
utils.c | 14 ++++++++++----
utils.h | 2 +-
5 files changed, 18 insertions(+), 8 deletions(-)

diff --git a/cmds-device.c b/cmds-device.c
index 57ad0b2..d4605fd 100644
--- a/cmds-device.c
+++ b/cmds-device.c
@@ -29,6 +29,7 @@
#include "ioctl.h"
#include "utils.h"
#include "cmds-fi-disk_usage.h"
+#include "volumes.h"

#include "commands.h"

@@ -236,9 +237,12 @@ static int cmd_scan_dev(int argc, char **argv)

if (all || argc == 1) {
printf("Scanning for Btrfs filesystems\n");
- ret = btrfs_scan_lblkid(BTRFS_UPDATE_KERNEL);
+ ret = btrfs_scan_lblkid();
if (ret)
fprintf(stderr, "ERROR: error %d while scanning\n", ret);
+ ret = btrfs_register_all_devices();
+ if (ret)
+ fprintf(stderr, "ERROR: error %d while registering\n", ret);
goto out;
}

diff --git a/cmds-filesystem.c b/cmds-filesystem.c
index 6fcf111..4a3c09c 100644
--- a/cmds-filesystem.c
+++ b/cmds-filesystem.c
@@ -661,7 +661,7 @@ static int cmd_show(int argc, char **argv)
goto out;

devs_only:
- ret = btrfs_scan_lblkid(!BTRFS_UPDATE_KERNEL);
+ ret = btrfs_scan_lblkid();

if (ret) {
fprintf(stderr, "ERROR: %d while scanning\n", ret);
diff --git a/disk-io.c b/disk-io.c
index 7ef5ac4..6a053ea 100644
--- a/disk-io.c
+++ b/disk-io.c
@@ -1013,7 +1013,7 @@ int btrfs_scan_fs_devices(int fd, const char *path,
}

if (total_devs != 1) {
- ret = btrfs_scan_lblkid(!BTRFS_UPDATE_KERNEL);
+ ret = btrfs_scan_lblkid();
if (ret)
return ret;
}
diff --git a/utils.c b/utils.c
index 017c513..f18cc46 100644
--- a/utils.c
+++ b/utils.c
@@ -54,6 +54,8 @@
#define BLKDISCARD _IO(0x12,119)
#endif

+int scan_done = 0;
+
static char argv0_buf[ARGV0_BUF_SIZE] = "btrfs";

void fixup_argv0(char **argv, const char *token)
@@ -1183,7 +1185,7 @@ int check_mounted_where(int fd, const char *file, char *where, int size,

/* scan other devices */
if (is_btrfs && total_devs > 1) {
- ret = btrfs_scan_lblkid(!BTRFS_UPDATE_KERNEL);
+ ret = btrfs_scan_lblkid();
if (ret)
return ret;
}
@@ -2093,7 +2095,7 @@ int test_dev_for_mkfs(char *file, int force_overwrite, char *estr)
return 0;
}

-int btrfs_scan_lblkid(int update_kernel)
+int btrfs_scan_lblkid()
{
int fd = -1;
int ret;
@@ -2104,6 +2106,9 @@ int btrfs_scan_lblkid(int update_kernel)
blkid_cache cache = NULL;
char path[PATH_MAX];

+ if (scan_done)
+ return 0;
+
if (blkid_get_cache(&cache, 0) < 0) {
printf("ERROR: lblkid cache get failed\n");
return 1;
@@ -2132,11 +2137,12 @@ int btrfs_scan_lblkid(int update_kernel)
}

close(fd);
- if (update_kernel)
- btrfs_register_one_device(path);
}
blkid_dev_iterate_end(iter);
blkid_put_cache(cache);
+
+ scan_done = 1;
+
return 0;
}

diff --git a/utils.h b/utils.h
index 237cf64..ab0c49e 100644
--- a/utils.h
+++ b/utils.h
@@ -126,7 +126,7 @@ int csum_tree_block(struct btrfs_root *root, struct extent_buffer *buf,
int verify);
int ask_user(char *question);
int lookup_ino_rootid(int fd, u64 *rootid);
-int btrfs_scan_lblkid(int update_kernel);
+int btrfs_scan_lblkid(void);
int get_btrfs_mount(const char *dev, char *mp, size_t mp_size);
int find_mount_root(const char *path, char **mount_root);
int get_device_info(int fd, u64 devid,
--
2.0.0.153.g79dcccc

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Anand Jain
2014-10-23 06:30:08 UTC
Permalink
btrfs_scan_lblikd() is called by most the device related command functions.
And btrfs_scan_lblkid() is most expensive function and it becomes more expensive
as number of devices in the system increase. Further some threads call this
function more than once for absolutely no extra benefit and the real waste of
resources.

btrfs-find-root 1
btrfs rescue super-recover 2
btrfs-debug-tree 1
btrfs-image -r 2
btrfs check 2
btrfs restore 2
calc-size NC
btrfs-corrupt-block NC
btrfs-image NC
btrfs-map-logical 1
btrfs-select-super NC
btrfstune 2
btrfs-zero-log NC
tester NC
quick-test.c NC
btrfs-convert 0
mkfs #number of devices to be mkfs
btrfs label set unmounted 2
btrfs get label unmounted 2

This patch will:
move out calling register_one_device with in btrfs_scan_lblkid()
and so function setting the BTRFS_UPDATE_KERNEL to yes will
call btrfs_register_all_devices() separately.

introduce a global variable scan_done, which is set when scan is
done succssfully per thread. So that following calls to this function
will just return success.

Further if any function needs to force scan after scan_done is set,
then it can be done when there is such a requirement, but as of now there
isn't any such requirement.

Signed-off-by: Anand Jain <***@oracle.com>
---
v2: rebase on 3.17. And add more info to commit

cmds-device.c | 7 ++++++-
cmds-filesystem.c | 2 +-
disk-io.c | 2 +-
utils.c | 14 ++++++++++----
utils.h | 2 +-
5 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/cmds-device.c b/cmds-device.c
index 8a33634..519314b 100644
--- a/cmds-device.c
+++ b/cmds-device.c
@@ -28,6 +28,8 @@
#include "ctree.h"
#include "ioctl.h"
#include "utils.h"
+#include "volumes.h"
+

#include "commands.h"

@@ -235,9 +237,12 @@ static int cmd_scan_dev(int argc, char **argv)

if (all || argc == 1) {
printf("Scanning for Btrfs filesystems\n");
- ret = btrfs_scan_lblkid(BTRFS_UPDATE_KERNEL);
+ ret = btrfs_scan_lblkid();
if (ret)
fprintf(stderr, "ERROR: error %d while scanning\n", ret);
+ ret = btrfs_register_all_devices();
+ if (ret)
+ fprintf(stderr, "ERROR: error %d while registering\n", ret);
goto out;
}

diff --git a/cmds-filesystem.c b/cmds-filesystem.c
index 695a3d5..011907d 100644
--- a/cmds-filesystem.c
+++ b/cmds-filesystem.c
@@ -698,7 +698,7 @@ static int cmd_show(int argc, char **argv)
goto out;

devs_only:
- ret = btrfs_scan_lblkid(!BTRFS_UPDATE_KERNEL);
+ ret = btrfs_scan_lblkid();

if (ret) {
fprintf(stderr, "ERROR: %d while scanning\n", ret);
diff --git a/disk-io.c b/disk-io.c
index 77fc610..2fb50f9 100644
--- a/disk-io.c
+++ b/disk-io.c
@@ -1012,7 +1012,7 @@ int btrfs_scan_fs_devices(int fd, const char *path,
}

if (total_devs != 1) {
- ret = btrfs_scan_lblkid(!BTRFS_UPDATE_KERNEL);
+ ret = btrfs_scan_lblkid();
if (ret)
return ret;
}
diff --git a/utils.c b/utils.c
index 90e228b..a8691fe 100644
--- a/utils.c
+++ b/utils.c
@@ -52,6 +52,8 @@
#define BLKDISCARD _IO(0x12,119)
#endif

+int scan_done = 0;
+
static char argv0_buf[ARGV0_BUF_SIZE] = "btrfs";

void fixup_argv0(char **argv, const char *token)
@@ -1186,7 +1188,7 @@ int check_mounted_where(int fd, const char *file, char *where, int size,

/* scan other devices */
if (is_btrfs && total_devs > 1) {
- ret = btrfs_scan_lblkid(!BTRFS_UPDATE_KERNEL);
+ ret = btrfs_scan_lblkid();
if (ret)
return ret;
}
@@ -2169,7 +2171,7 @@ int test_dev_for_mkfs(char *file, int force_overwrite, char *estr)
return 0;
}

-int btrfs_scan_lblkid(int update_kernel)
+int btrfs_scan_lblkid()
{
int fd = -1;
int ret;
@@ -2180,6 +2182,9 @@ int btrfs_scan_lblkid(int update_kernel)
blkid_cache cache = NULL;
char path[PATH_MAX];

+ if (scan_done)
+ return 0;
+
if (blkid_get_cache(&cache, 0) < 0) {
printf("ERROR: lblkid cache get failed\n");
return 1;
@@ -2208,11 +2213,12 @@ int btrfs_scan_lblkid(int update_kernel)
}

close(fd);
- if (update_kernel)
- btrfs_register_one_device(path);
}
blkid_dev_iterate_end(iter);
blkid_put_cache(cache);
+
+ scan_done = 1;
+
return 0;
}

diff --git a/utils.h b/utils.h
index fea26ef..d1af33d 100644
--- a/utils.h
+++ b/utils.h
@@ -127,7 +127,7 @@ int csum_tree_block(struct btrfs_root *root, struct extent_buffer *buf,
int verify);
int ask_user(char *question);
int lookup_ino_rootid(int fd, u64 *rootid);
-int btrfs_scan_lblkid(int update_kernel);
+int btrfs_scan_lblkid(void);
int get_btrfs_mount(const char *dev, char *mp, size_t mp_size);
int find_mount_root(const char *path, char **mount_root);
int get_device_info(int fd, u64 devid,
--
2.0.0.153.g79dcccc

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Loading...