diff mbox

Btrfs: init device stats for new devices

Message ID a1938a2cc8a66c24293b19aa91526d6eac433b10.1381497087.git.sbehrens@giantdisaster.de (mailing list archive)
State Accepted, archived
Headers show

Commit Message

Stefan Behrens Oct. 11, 2013, 1:20 p.m. UTC
Device stats are only initialized (read from tree items) on mount.
Trying to read device stats after adding or replacing new devices will
return errors.

btrfs_init_new_device() and btrfs_init_dev_replace_tgtdev() are the two
functions that allocate and initialize new btrfs_device structures after
a filesystem is mounted. They set the device stats to zero by using
kzalloc() which is correct for new devices. The only missing thing was
to declare these stats as being valid (device->dev_stats_valid = 1) and
this patch adds this missing code.

This is the reproducer:

TEST_DEV1=/dev/sdzzzzz1
TEST_DEV2=/dev/sdzzzzz2
TEST_DEV3=/dev/sdzzzzz3
TEST_MNT=/mnt
mkfs.btrfs $TEST_DEV1
mount $TEST_DEV1 $TEST_MNT
btrfs device add $TEST_DEV2 $TEST_MNT
btrfs device stat $TEST_MNT
btrfs replace start -B $TEST_DEV2 $TEST_DEV3 $TEST_MNT
btrfs device stat $TEST_MNT
umount $TEST_MNT

Reported-by: Ondrej Kunc <kunc88@gmail.com>
Signed-off-by: Stefan Behrens <sbehrens@giantdisaster.de>
---
The idea to fix this issue, the subject line of the patch and parts
of the commit log are reused from a patch that Zach Brown has sent.

 fs/btrfs/volumes.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

Josef Bacik Oct. 11, 2013, 1:43 p.m. UTC | #1
On Fri, Oct 11, 2013 at 03:20:42PM +0200, Stefan Behrens wrote:
> Device stats are only initialized (read from tree items) on mount.
> Trying to read device stats after adding or replacing new devices will
> return errors.
> 
> btrfs_init_new_device() and btrfs_init_dev_replace_tgtdev() are the two
> functions that allocate and initialize new btrfs_device structures after
> a filesystem is mounted. They set the device stats to zero by using
> kzalloc() which is correct for new devices. The only missing thing was
> to declare these stats as being valid (device->dev_stats_valid = 1) and
> this patch adds this missing code.
> 
> This is the reproducer:
> 
> TEST_DEV1=/dev/sdzzzzz1
> TEST_DEV2=/dev/sdzzzzz2
> TEST_DEV3=/dev/sdzzzzz3
> TEST_MNT=/mnt
> mkfs.btrfs $TEST_DEV1
> mount $TEST_DEV1 $TEST_MNT
> btrfs device add $TEST_DEV2 $TEST_MNT
> btrfs device stat $TEST_MNT
> btrfs replace start -B $TEST_DEV2 $TEST_DEV3 $TEST_MNT
> btrfs device stat $TEST_MNT
> umount $TEST_MNT
> 

Sounds like something that would look great in xfstests,

Josef
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Zach Brown Oct. 11, 2013, 5:32 p.m. UTC | #2
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index 646d10d..9837439 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -2041,6 +2041,7 @@ int btrfs_init_new_device(struct btrfs_root *root, char *device_path)
>  	device->in_fs_metadata = 1;
>  	device->is_tgtdev_for_dev_replace = 0;
>  	device->mode = FMODE_EXCL;
> +	device->dev_stats_valid = 1;
>  	set_blocksize(device->bdev, 4096);

Doesn't valid mean that the in-memory atomics reflect the counts in the
btree items?  Am I misunderstanding?

Seems like a reproducer would be: get non-zero stats, unmount, mount,
get stats strictly >=.

- z
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Zach Brown Oct. 11, 2013, 5:36 p.m. UTC | #3
> a filesystem is mounted. They set the device stats to zero by using
> kzalloc() which is correct for new devices.

Oh, right, got it :)

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

Patch

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 646d10d..9837439 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -2041,6 +2041,7 @@  int btrfs_init_new_device(struct btrfs_root *root, char *device_path)
 	device->in_fs_metadata = 1;
 	device->is_tgtdev_for_dev_replace = 0;
 	device->mode = FMODE_EXCL;
+	device->dev_stats_valid = 1;
 	set_blocksize(device->bdev, 4096);
 
 	if (seeding_dev) {
@@ -2208,6 +2209,7 @@  int btrfs_init_dev_replace_tgtdev(struct btrfs_root *root, char *device_path,
 	device->in_fs_metadata = 1;
 	device->is_tgtdev_for_dev_replace = 1;
 	device->mode = FMODE_EXCL;
+	device->dev_stats_valid = 1;
 	set_blocksize(device->bdev, 4096);
 	device->fs_devices = fs_info->fs_devices;
 	list_add(&device->dev_list, &fs_info->fs_devices->devices);