Discussion:
[PATCH 1/1] btrfs-progs: introduce a proper structure on which cli will call register-device ioctl
(too old to reply)
Anand Jain
2014-10-15 00:46:14 UTC
Permalink
Raw Message
As of now commands mentioned below (with in [..]) are calling call register-device
ioctl BTRFS_IOC_SCAN_DEV for all the devices in the system.
Some issues with it:
BTRFS_IOC_SCAN_DEV: ioctl is a write operation, we don't want command like
btrfs-debug-tree threads to do that..
eg:
----
$ cat /proc/fs/btrfs/devlist | egrep fsid | wc -l
0
$ btrfs-debug-tree /dev/sde (num_device > 1)
$ cat /proc/fs/btrfs/devlist | egrep fsid | wc -l
5
----

btrfs_scan_fs_devices() ends up calling this ioctl only when num_device > 1.
That's inconsistency with in feature/bug.

We don't have to register _all_ the btrfs devices (again) in the system
without user consent.

Why its inconsistent:
function btrfs_scan_fs_devices() calls btrfs_scan_lblkid only when
num_devices is > 1, which in turn calls BTRFS_IOC_SCAN_DEV ioctl, if
conditions are met.

But main issue is we have too many consumers of btrfs_scan_fs_devices()
the names below with in [] is the cli leading to this function.

open_ctree_broken() [btrfs-find-root]
recover_prepare() [btrfs rescue super-recover]
__open_ctree_fd
(updates always except when flag OPEN_CTREE_RECOVER_SUPER is set and
flag OPEN_CTREE_RECOVER_SUPER is set only by 'btrfs rescue super-
recover' but still this thread sneaks through the open_ctree function
to call register-device-ioctl as show below).
open_ctree_fs_info
[btrfs-debug-tree]
[btrfs-image -r]
[btrfs check]
open_fs
[btrfs restore]
open_ctree
[calc-size]
[btrfs-corrupt-block]
[btrfs-image] (create)
[btrfs-map-logical]
[btrfs-select-super]
[btrfstune]
[btrfs-zero-log]
[tester]
[mkfs]
[quick-test.c]
[btrfs label set unmounted]
[btrfs get label unmounted]
[btrfs rescue super-recover]

open_ctree_fd
[btrfs-convert]

Fix:
In an effort to make register-device consistent, all calls to
btrfs_scan_fs_devices() will have 5th parameter set to 0. that means
we don't need 5th parameter at all. And with this function not calling
the register ioctl at all, finally we will have following two cli to call
the ioctl BTRFS_IOC_SCAN_DEV.
btrfs dev scan and
mkfs.btrfs
Threads needing to update kernel about a device would have to use
btrfs_register_one_device() separately.

Signed-off-by: Anand Jain <***@oracle.com>
---
btrfs-find-root.c | 2 +-
chunk-recover.c | 2 +-
disk-io.c | 5 ++---
disk-io.h | 2 +-
super-recover.c | 2 +-
5 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/btrfs-find-root.c b/btrfs-find-root.c
index 6fe4b7b..caf21ce 100644
--- a/btrfs-find-root.c
+++ b/btrfs-find-root.c
@@ -82,7 +82,7 @@ static struct btrfs_root *open_ctree_broken(int fd, const char *device)
return NULL;
}

- ret = btrfs_scan_fs_devices(fd, device, &fs_devices, 0, 1, 1);
+ ret = btrfs_scan_fs_devices(fd, device, &fs_devices, 0, 1);
if (ret)
goto out;

diff --git a/chunk-recover.c b/chunk-recover.c
index b4c62a8..7bae4f9 100644
--- a/chunk-recover.c
+++ b/chunk-recover.c
@@ -1372,7 +1372,7 @@ static int recover_prepare(struct recover_control *rc, char *path)
goto fail_free_sb;
}

- ret = btrfs_scan_fs_devices(fd, path, &fs_devices, 0, 1, 1);
+ ret = btrfs_scan_fs_devices(fd, path, &fs_devices, 0, 1);
if (ret)
goto fail_free_sb;

diff --git a/disk-io.c b/disk-io.c
index f9dbc85..7ef5ac4 100644
--- a/disk-io.c
+++ b/disk-io.c
@@ -994,7 +994,7 @@ void btrfs_cleanup_all_caches(struct btrfs_fs_info *fs_info)

int btrfs_scan_fs_devices(int fd, const char *path,
struct btrfs_fs_devices **fs_devices,
- u64 sb_bytenr, int run_ioctl, int super_recover)
+ u64 sb_bytenr, int super_recover)
{
u64 total_devs;
int ret;
@@ -1013,7 +1013,7 @@ int btrfs_scan_fs_devices(int fd, const char *path,
}

if (total_devs != 1) {
- ret = btrfs_scan_lblkid(run_ioctl);
+ ret = btrfs_scan_lblkid(!BTRFS_UPDATE_KERNEL);
if (ret)
return ret;
}
@@ -1094,7 +1094,6 @@ static struct btrfs_fs_info *__open_ctree_fd(int fp, const char *path,
fs_info->on_restoring = 1;

ret = btrfs_scan_fs_devices(fp, path, &fs_devices, sb_bytenr,
- !(flags & OPEN_CTREE_RECOVER_SUPER),
(flags & OPEN_CTREE_RECOVER_SUPER));
if (ret)
goto out;
diff --git a/disk-io.h b/disk-io.h
index c296055..4818109 100644
--- a/disk-io.h
+++ b/disk-io.h
@@ -68,7 +68,7 @@ void btrfs_release_all_roots(struct btrfs_fs_info *fs_info);
void btrfs_cleanup_all_caches(struct btrfs_fs_info *fs_info);
int btrfs_scan_fs_devices(int fd, const char *path,
struct btrfs_fs_devices **fs_devices, u64 sb_bytenr,
- int run_ioctl, int super_recover);
+ int super_recover);
int btrfs_setup_chunk_tree_and_device_map(struct btrfs_fs_info *fs_info);

struct btrfs_root *open_ctree(const char *filename, u64 sb_bytenr,
diff --git a/super-recover.c b/super-recover.c
index 419b86a..adb2c44 100644
--- a/super-recover.c
+++ b/super-recover.c
@@ -282,7 +282,7 @@ int btrfs_recover_superblocks(const char *dname,
}
init_recover_superblock(&recover);

- ret = btrfs_scan_fs_devices(fd, dname, &recover.fs_devices, 0, 0, 1);
+ ret = btrfs_scan_fs_devices(fd, dname, &recover.fs_devices, 0, 1);
close(fd);
if (ret) {
ret = 1;
--
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
David Sterba
2014-10-16 10:08:41 UTC
Permalink
Raw Message
On Wed, Oct 15, 2014 at 08:46:14AM +0800, Anand Jain wrote:
[...]
Thanks for the detailed changelog, the patch fixes xfstests/btrfs/006
failure, previously:

@@ -1,6 +1,10 @@
=3D=3D QA output created by 006
=3D=3D Set filesystem label to TestLabel.006
+ERROR: device scan failed '/dev/sdc2' - File exists
+ERROR: device scan failed '/dev/sdc3' - File exists
=3D=3D Get filesystem label
+ERROR: device scan failed '/dev/sdc2' - File exists
+ERROR: device scan failed '/dev/sdc3' - File exists

though I now see another message (but haven't analyzed the cause yet):

@@ -22,18 +22,21 @@
<NUMDEVS> [SCRATCH_DEV].read_io_errs <NUM>
<NUMDEVS> [SCRATCH_DEV].write_io_errs <NUM>
=3D=3D Show device stats by first/scratch dev
+ERROR: ioctl(BTRFS_IOC_GET_DEV_STATS) on =EF=BF=BDoh failed: No su=
ch device
[SCRATCH_DEV].corruption_errs <NUM>
[SCRATCH_DEV].flush_io_errs <NUM>
[SCRATCH_DEV].generation_errs <NUM>
--
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
David Sterba
2014-10-16 10:20:36 UTC
Permalink
Raw Message
Post by David Sterba
@@ -22,18 +22,21 @@
<NUMDEVS> [SCRATCH_DEV].read_io_errs <NUM>
<NUMDEVS> [SCRATCH_DEV].write_io_errs <NUM>
=3D=3D Show device stats by first/scratch dev
+ERROR: ioctl(BTRFS_IOC_GET_DEV_STATS) on =EF=BF=BDoh failed: No =
such device
Post by David Sterba
[SCRATCH_DEV].corruption_errs <NUM>
[SCRATCH_DEV].flush_io_errs <NUM>
[SCRATCH_DEV].generation_errs <NUM>
Additional info:

syslog:

[ 1332.330541] run xfstest btrfs/006
[ 1333.251419] BTRFS: device fsid 02ad1ce3-85e4-41c0-8d54-138a8d950b3d =
devid 1 transid 3 /dev/sda9
[ 1333.273512] BTRFS: device fsid 02ad1ce3-85e4-41c0-8d54-138a8d950b3d =
devid 2 transid 3 /dev/sda10
[ 1333.345285] BTRFS: device fsid 02ad1ce3-85e4-41c0-8d54-138a8d950b3d =
devid 3 transid 3 /dev/sda11
[ 1333.366160] BTRFS: device fsid 02ad1ce3-85e4-41c0-8d54-138a8d950b3d =
devid 4 transid 3 /dev/sda12
[ 1334.468250] BTRFS info (device sda12): disk space caching is enabled
[ 1334.476280] BTRFS: flagging fs with big metadata feature
[ 1334.491801] BTRFS: creating UUID tree
[ 1334.774684] BTRFS warning (device sda12): get dev_stats failed, devi=
ce not found
[ 1334.805125] BTRFS warning (device sda12): get dev_stats failed, devi=
ce not found
[ 1334.832683] BTRFS warning (device sda12): get dev_stats failed, devi=
ce not found

$ btrfs fi show

Label: 'TestLabel.006' uuid: 02ad1ce3-85e4-41c0-8d54-138a8d950b3d
Total devices 4 FS bytes used 192.00KiB
devid 1 size 10.00GiB used 20.00MiB path /dev/sda9
devid 2 size 10.00GiB used 256.00MiB path /dev/sda10
devid 3 size 10.00GiB used 0.00B path /dev/sda11
devid 4 size 10.00GiB used 0.00B path /dev/sda12

SCRATCH_DEV_POOL=3D/dev/sda9 /dev/sda10 /dev/sda11 /dev/sda12
--
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-16 14:56:37 UTC
Permalink
Raw Message
Post by David Sterba
Post by David Sterba
@@ -22,18 +22,21 @@
<NUMDEVS> [SCRATCH_DEV].read_io_errs <NUM>
<NUMDEVS> [SCRATCH_DEV].write_io_errs <NUM>
=3D=3D Show device stats by first/scratch dev
+ERROR: ioctl(BTRFS_IOC_GET_DEV_STATS) on =EF=BF=BDoh failed: N=
o such device
Post by David Sterba
Post by David Sterba
[SCRATCH_DEV].corruption_errs <NUM>
[SCRATCH_DEV].flush_io_errs <NUM>
[SCRATCH_DEV].generation_errs <NUM>
[ 1332.330541] run xfstest btrfs/006
[ 1333.251419] BTRFS: device fsid 02ad1ce3-85e4-41c0-8d54-138a8d950b3=
d devid 1 transid 3 /dev/sda9
Post by David Sterba
[ 1333.273512] BTRFS: device fsid 02ad1ce3-85e4-41c0-8d54-138a8d950b3=
d devid 2 transid 3 /dev/sda10
Post by David Sterba
[ 1333.345285] BTRFS: device fsid 02ad1ce3-85e4-41c0-8d54-138a8d950b3=
d devid 3 transid 3 /dev/sda11
Post by David Sterba
[ 1333.366160] BTRFS: device fsid 02ad1ce3-85e4-41c0-8d54-138a8d950b3=
d devid 4 transid 3 /dev/sda12
Post by David Sterba
[ 1334.468250] BTRFS info (device sda12): disk space caching is enabl=
ed
Post by David Sterba
[ 1334.476280] BTRFS: flagging fs with big metadata feature
[ 1334.491801] BTRFS: creating UUID tree
[ 1334.774684] BTRFS warning (device sda12): get dev_stats failed, de=
vice not found
Post by David Sterba
[ 1334.805125] BTRFS warning (device sda12): get dev_stats failed, de=
vice not found
Post by David Sterba
[ 1334.832683] BTRFS warning (device sda12): get dev_stats failed, de=
vice not found
Post by David Sterba
$ btrfs fi show
Label: 'TestLabel.006' uuid: 02ad1ce3-85e4-41c0-8d54-138a8d950b3d
Total devices 4 FS bytes used 192.00KiB
devid 1 size 10.00GiB used 20.00MiB path /dev/sda9
devid 2 size 10.00GiB used 256.00MiB path /dev/sda10
devid 3 size 10.00GiB used 0.00B path /dev/sda11
devid 4 size 10.00GiB used 0.00B path /dev/sda12
SCRATCH_DEV_POOL=3D/dev/sda9 /dev/sda10 /dev/sda11 /dev/sda12
Quite strange. I didn't see that problem here.
----
btrfs/006 12s ... 13s
Ran: btrfs/006
Passed all 1 tests
---

Can you retain the scratch mounted and check what is the output of
'btrfs fi show -m' (when the SCRATCH is mounted).

OR /proc/fs/btrfs/devlist (in case if you have that) would help
to debug this.

OR could you restart test with a wipe all disk ?

Thanks, Anand



--
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-16 15:19:43 UTC
Permalink
Raw Message
I have all 6 patches,

----
e56117f btrfs-progs: optimize btrfs_scan_lblkid() for multiple calls
4d76f5f btrfs-progs: introduce btrfs_register_all_device()
7dd7ef7 btrfs-progs: code optimize cmd_scan_dev() use=20
btrfs_register_one_device()
7158fe7 btrfs-progs: open RW to register device using btrfs-control
6bba89f btrfs-progs: introduce a proper structure on which cli will cal=
l=20
register-device ioctl
df572b7 btrfs-progs: mkfs should be consistent in calling register devi=
ce
----

you might have only below 3. I tried with that sub-set too but still
it does not fail here.

Thanks, Anand
Post by Anand Jain
Post by David Sterba
Post by David Sterba
@@ -22,18 +22,21 @@
<NUMDEVS> [SCRATCH_DEV].read_io_errs <NUM>
<NUMDEVS> [SCRATCH_DEV].write_io_errs <NUM>
=3D=3D Show device stats by first/scratch dev
+ERROR: ioctl(BTRFS_IOC_GET_DEV_STATS) on =EF=BF=BDoh failed: =
No such
Post by Anand Jain
Post by David Sterba
Post by David Sterba
device
[SCRATCH_DEV].corruption_errs <NUM>
[SCRATCH_DEV].flush_io_errs <NUM>
[SCRATCH_DEV].generation_errs <NUM>
[ 1332.330541] run xfstest btrfs/006
[ 1333.251419] BTRFS: device fsid 02ad1ce3-85e4-41c0-8d54-138a8d950b=
3d
Post by Anand Jain
Post by David Sterba
devid 1 transid 3 /dev/sda9
[ 1333.273512] BTRFS: device fsid 02ad1ce3-85e4-41c0-8d54-138a8d950b=
3d
Post by Anand Jain
Post by David Sterba
devid 2 transid 3 /dev/sda10
[ 1333.345285] BTRFS: device fsid 02ad1ce3-85e4-41c0-8d54-138a8d950b=
3d
Post by Anand Jain
Post by David Sterba
devid 3 transid 3 /dev/sda11
[ 1333.366160] BTRFS: device fsid 02ad1ce3-85e4-41c0-8d54-138a8d950b=
3d
Post by Anand Jain
Post by David Sterba
devid 4 transid 3 /dev/sda12
[ 1334.468250] BTRFS info (device sda12): disk space caching is enab=
led
Post by Anand Jain
Post by David Sterba
[ 1334.476280] BTRFS: flagging fs with big metadata feature
[ 1334.491801] BTRFS: creating UUID tree
[ 1334.774684] BTRFS warning (device sda12): get dev_stats failed,
device not found
[ 1334.805125] BTRFS warning (device sda12): get dev_stats failed,
device not found
[ 1334.832683] BTRFS warning (device sda12): get dev_stats failed,
device not found
$ btrfs fi show
Label: 'TestLabel.006' uuid: 02ad1ce3-85e4-41c0-8d54-138a8d950b3d
Total devices 4 FS bytes used 192.00KiB
devid 1 size 10.00GiB used 20.00MiB path /dev/sda9
devid 2 size 10.00GiB used 256.00MiB path /dev/sda10
devid 3 size 10.00GiB used 0.00B path /dev/sda11
devid 4 size 10.00GiB used 0.00B path /dev/sda12
SCRATCH_DEV_POOL=3D/dev/sda9 /dev/sda10 /dev/sda11 /dev/sda12
Quite strange. I didn't see that problem here.
----
btrfs/006 12s ... 13s
Ran: btrfs/006
Passed all 1 tests
---
Can you retain the scratch mounted and check what is the output of
'btrfs fi show -m' (when the SCRATCH is mounted).
OR /proc/fs/btrfs/devlist (in case if you have that) would help
to debug this.
OR could you restart test with a wipe all disk ?
Thanks, Anand
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs=
" in
Post by Anand Jain
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
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
David Sterba
2014-10-17 15:39:28 UTC
Permalink
Raw Message
Post by Anand Jain
Quite strange. I didn't see that problem here.
Thanks for checking.

Yeah, it must be some oddity in my setup. Wiping all the devices makes
no change and I don't see anyting strange when scratch is mounted:

# btrfs fi show -m
Label: 'TestLabel.006' uuid: 831545ca-35de-4485-94f0-dae2bb9fb6a7
Total devices 4 FS bytes used 192.00KiB
devid 1 size 10.00GiB used 20.00MiB path /dev/sda9
devid 2 size 10.00GiB used 256.00MiB path /dev/sda10
devid 3 size 10.00GiB used 0.00B path /dev/sda11
devid 4 size 10.00GiB used 0.00B path /dev/sda12

same without -m.
--
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...