| Message ID | 20220810072021.4539-1-hmsjwzb@zoho.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs: Fix btrfs_find_device for btrfs/249 | expand |
On 2022/8/10 15:20, Flint.Wang wrote: > testcase btrfs249 failed. > [How to reproduce] > mkfs.btrfs -f -d raid1 -m raid1 /dev/sdb /dev/sdc > btrfstune -S 1 /dev/sdb IIRC I have submitted a patch to reject seed creation on multiple devices: https://www.spinics.net/lists/linux-btrfs/msg123545.html I think this would be a much simpler solution, if we just don't support that use-case at all. Thanks, Qu > wipefs -a /dev/sdb > mount -o degraded /dev/sdc /mnt/scratch > btrfs device add -f /dev/sdd /mnt/scratch > btrfs filesystem usage /mnt/scratch > > [Root cause] > mkfs.btrfs -f -d raid1 -m raid1 /dev/sdb /dev/sdc > btrfstune -S 1 /dev/sdb > wipefs -a /dev/sdb > mount -o degraded /dev/sdc /mnt/scratch > > In the above commands, btrfstune command set the sdb and sdc to seeding device. > After that you wipe the filesystem on sdb. After mount, you will find the status of sdb is missing. > > btrfs device add -f /dev/sdd /mnt/scratch: > This command will invoke btrfs_setup_sprout to do the job. > It put the devices on fs_devices->devices to seed_devices list. > So only sdd is on the fs_devices->devices list. sdb(missing), sdc on the seed_devices list. > But when we look into the btrfs_find_devices function, it find devices both in devices list and seed_devices list. > > btrfs filesystem usage /mnt/scratch > This command use ioctl to get device info. The assertion is triggered because it finds the number of devices is inconsistent. > > [My fix solution] > 1. Add noseed argument to btrfs_find_device. It force the function only look into devices list. > 2. Add a new ioctl request(BTRFS_IOC_DEV_INFO_NOSEED) in case some application may depend the original ioctl behavior on BTRFS_IOC_DEV_INFO > 3. Modify load_device_info and load_chunk_and_device_info in btrfs-prog for appropriate ioctl call. > > After the change, btrfs249 passed. > > Signed-off-by: Flint.Wang <hmsjwzb@zoho.com> > --- > fs/btrfs/dev-replace.c | 8 ++++---- > fs/btrfs/ioctl.c | 10 ++++++---- > fs/btrfs/scrub.c | 4 ++-- > fs/btrfs/volumes.c | 22 ++++++++++++---------- > fs/btrfs/volumes.h | 5 ++++- > include/uapi/linux/btrfs.h | 2 ++ > 6 files changed, 30 insertions(+), 21 deletions(-) > > diff --git a/fs/btrfs/dev-replace.c b/fs/btrfs/dev-replace.c > index f43196a893ca3..49d3c587c2948 100644 > --- a/fs/btrfs/dev-replace.c > +++ b/fs/btrfs/dev-replace.c > @@ -101,7 +101,7 @@ int btrfs_init_dev_replace(struct btrfs_fs_info *fs_info) > * We don't have a replace item or it's corrupted. If there is > * a replace target, fail the mount. > */ > - if (btrfs_find_device(fs_info->fs_devices, &args)) { > + if (btrfs_find_device(fs_info->fs_devices, &args, false)) { > btrfs_err(fs_info, > "found replace target device without a valid replace item"); > ret = -EUCLEAN; > @@ -163,7 +163,7 @@ int btrfs_init_dev_replace(struct btrfs_fs_info *fs_info) > * We don't have an active replace item but if there is a > * replace target, fail the mount. > */ > - if (btrfs_find_device(fs_info->fs_devices, &args)) { > + if (btrfs_find_device(fs_info->fs_devices, &args, false)) { > btrfs_err(fs_info, > "replace devid present without an active replace item"); > ret = -EUCLEAN; > @@ -174,9 +174,9 @@ int btrfs_init_dev_replace(struct btrfs_fs_info *fs_info) > break; > case BTRFS_IOCTL_DEV_REPLACE_STATE_STARTED: > case BTRFS_IOCTL_DEV_REPLACE_STATE_SUSPENDED: > - dev_replace->tgtdev = btrfs_find_device(fs_info->fs_devices, &args); > + dev_replace->tgtdev = btrfs_find_device(fs_info->fs_devices, &args, false); > args.devid = src_devid; > - dev_replace->srcdev = btrfs_find_device(fs_info->fs_devices, &args); > + dev_replace->srcdev = btrfs_find_device(fs_info->fs_devices, &args, false); > > /* > * allow 'btrfs dev replace_cancel' if src/tgt device is > diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c > index fe0cc816b4eba..bdf1578839c99 100644 > --- a/fs/btrfs/ioctl.c > +++ b/fs/btrfs/ioctl.c > @@ -2039,7 +2039,7 @@ static noinline int btrfs_ioctl_resize(struct file *file, > } > > args.devid = devid; > - device = btrfs_find_device(fs_info->fs_devices, &args); > + device = btrfs_find_device(fs_info->fs_devices, &args, false); > if (!device) { > btrfs_info(fs_info, "resizer unable to find device %llu", > devid); > @@ -3721,7 +3721,7 @@ static long btrfs_ioctl_fs_info(struct btrfs_fs_info *fs_info, > } > > static long btrfs_ioctl_dev_info(struct btrfs_fs_info *fs_info, > - void __user *arg) > + void __user *arg, bool noseed) > { > BTRFS_DEV_LOOKUP_ARGS(args); > struct btrfs_ioctl_dev_info_args *di_args; > @@ -3737,7 +3737,7 @@ static long btrfs_ioctl_dev_info(struct btrfs_fs_info *fs_info, > args.uuid = di_args->uuid; > > rcu_read_lock(); > - dev = btrfs_find_device(fs_info->fs_devices, &args); > + dev = btrfs_find_device(fs_info->fs_devices, &args, noseed); > if (!dev) { > ret = -ENODEV; > goto out; > @@ -5468,7 +5468,7 @@ long btrfs_ioctl(struct file *file, unsigned int > case BTRFS_IOC_FS_INFO: > return btrfs_ioctl_fs_info(fs_info, argp); > case BTRFS_IOC_DEV_INFO: > - return btrfs_ioctl_dev_info(fs_info, argp); > + return btrfs_ioctl_dev_info(fs_info, argp, false); > case BTRFS_IOC_TREE_SEARCH: > return btrfs_ioctl_tree_search(inode, argp); > case BTRFS_IOC_TREE_SEARCH_V2: > @@ -5570,6 +5570,8 @@ long btrfs_ioctl(struct file *file, unsigned int > case BTRFS_IOC_ENCODED_WRITE_32: > return btrfs_ioctl_encoded_write(file, argp, true); > #endif > + case BTRFS_IOC_DEV_INFO_NOSEED: > + return btrfs_ioctl_dev_info(fs_info, argp, true); > } > > return -ENOTTY; > diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c > index 3afe5fa50a631..4b734d76776ca 100644 > --- a/fs/btrfs/scrub.c > +++ b/fs/btrfs/scrub.c > @@ -4143,7 +4143,7 @@ int btrfs_scrub_dev(struct btrfs_fs_info *fs_info, u64 devid, u64 start, > goto out_free_ctx; > > mutex_lock(&fs_info->fs_devices->device_list_mutex); > - dev = btrfs_find_device(fs_info->fs_devices, &args); > + dev = btrfs_find_device(fs_info->fs_devices, &args, false); > if (!dev || (test_bit(BTRFS_DEV_STATE_MISSING, &dev->dev_state) && > !is_dev_replace)) { > mutex_unlock(&fs_info->fs_devices->device_list_mutex); > @@ -4321,7 +4321,7 @@ int btrfs_scrub_progress(struct btrfs_fs_info *fs_info, u64 devid, > struct scrub_ctx *sctx = NULL; > > mutex_lock(&fs_info->fs_devices->device_list_mutex); > - dev = btrfs_find_device(fs_info->fs_devices, &args); > + dev = btrfs_find_device(fs_info->fs_devices, &args, false); > if (dev) > sctx = dev->scrub_ctx; > if (sctx) > diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c > index 272901514b0c1..1abd75e90cd9e 100644 > --- a/fs/btrfs/volumes.c > +++ b/fs/btrfs/volumes.c > @@ -808,7 +808,7 @@ static noinline struct btrfs_device *device_list_add(const char *path, > }; > > mutex_lock(&fs_devices->device_list_mutex); > - device = btrfs_find_device(fs_devices, &args); > + device = btrfs_find_device(fs_devices, &args, false); > > /* > * If this disk has been pulled into an fs devices created by > @@ -2075,7 +2075,7 @@ int btrfs_rm_device(struct btrfs_fs_info *fs_info, > if (ret) > return ret; > > - device = btrfs_find_device(fs_info->fs_devices, args); > + device = btrfs_find_device(fs_info->fs_devices, args, false); > if (!device) { > if (args->missing) > ret = BTRFS_ERROR_DEV_MISSING_NOT_FOUND; > @@ -2381,7 +2381,7 @@ struct btrfs_device *btrfs_find_device_by_devspec( > > if (devid) { > args.devid = devid; > - device = btrfs_find_device(fs_info->fs_devices, &args); > + device = btrfs_find_device(fs_info->fs_devices, &args, false); > if (!device) > return ERR_PTR(-ENOENT); > return device; > @@ -2390,7 +2390,7 @@ struct btrfs_device *btrfs_find_device_by_devspec( > ret = btrfs_get_dev_args_from_path(fs_info, &args, device_path); > if (ret) > return ERR_PTR(ret); > - device = btrfs_find_device(fs_info->fs_devices, &args); > + device = btrfs_find_device(fs_info->fs_devices, &args, false); > btrfs_put_dev_args_from_path(&args); > if (!device) > return ERR_PTR(-ENOENT); > @@ -2551,7 +2551,7 @@ static int btrfs_finish_sprout(struct btrfs_trans_handle *trans) > BTRFS_FSID_SIZE); > args.uuid = dev_uuid; > args.fsid = fs_uuid; > - device = btrfs_find_device(fs_info->fs_devices, &args); > + device = btrfs_find_device(fs_info->fs_devices, &args, false); > BUG_ON(!device); /* Logic error */ > > if (device->fs_devices->seeding) { > @@ -6821,7 +6821,7 @@ static bool dev_args_match_device(const struct btrfs_dev_lookup_args *args, > * only devid is used. > */ > struct btrfs_device *btrfs_find_device(const struct btrfs_fs_devices *fs_devices, > - const struct btrfs_dev_lookup_args *args) > + const struct btrfs_dev_lookup_args *args, bool noseed) > { > struct btrfs_device *device; > struct btrfs_fs_devices *seed_devs; > @@ -6832,6 +6832,8 @@ struct btrfs_device *btrfs_find_device(const struct btrfs_fs_devices *fs_devices > return device; > } > } > + if (noseed) > + return NULL; > > list_for_each_entry(seed_devs, &fs_devices->seed_list, seed_list) { > if (!dev_args_match_fs_devices(args, seed_devs)) > @@ -7095,7 +7097,7 @@ static int read_one_chunk(struct btrfs_key *key, struct extent_buffer *leaf, > btrfs_stripe_dev_uuid_nr(chunk, i), > BTRFS_UUID_SIZE); > args.uuid = uuid; > - map->stripes[i].dev = btrfs_find_device(fs_info->fs_devices, &args); > + map->stripes[i].dev = btrfs_find_device(fs_info->fs_devices, &args, false); > if (!map->stripes[i].dev) { > map->stripes[i].dev = handle_missing_device(fs_info, > devid, uuid); > @@ -7226,7 +7228,7 @@ static int read_one_dev(struct extent_buffer *leaf, > return PTR_ERR(fs_devices); > } > > - device = btrfs_find_device(fs_info->fs_devices, &args); > + device = btrfs_find_device(fs_info->fs_devices, &args, false); > if (!device) { > if (!btrfs_test_opt(fs_info, DEGRADED)) { > btrfs_report_missing_device(fs_info, devid, > @@ -7884,7 +7886,7 @@ int btrfs_get_dev_stats(struct btrfs_fs_info *fs_info, > > mutex_lock(&fs_devices->device_list_mutex); > args.devid = stats->devid; > - dev = btrfs_find_device(fs_info->fs_devices, &args); > + dev = btrfs_find_device(fs_info->fs_devices, &args, false); > mutex_unlock(&fs_devices->device_list_mutex); > > if (!dev) { > @@ -8026,7 +8028,7 @@ static int verify_one_dev_extent(struct btrfs_fs_info *fs_info, > } > > /* Make sure no dev extent is beyond device boundary */ > - dev = btrfs_find_device(fs_info->fs_devices, &args); > + dev = btrfs_find_device(fs_info->fs_devices, &args, false); > if (!dev) { > btrfs_err(fs_info, "failed to find devid %llu", devid); > ret = -EUCLEAN; > diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h > index 5639961b3626f..4b6bcc777f752 100644 > --- a/fs/btrfs/volumes.h > +++ b/fs/btrfs/volumes.h > @@ -609,7 +609,10 @@ int btrfs_num_copies(struct btrfs_fs_info *fs_info, u64 logical, u64 len); > int btrfs_grow_device(struct btrfs_trans_handle *trans, > struct btrfs_device *device, u64 new_size); > struct btrfs_device *btrfs_find_device(const struct btrfs_fs_devices *fs_devices, > - const struct btrfs_dev_lookup_args *args); > + const struct btrfs_dev_lookup_args *args, > + bool noseed); > +struct btrfs_device *btrfs_find_device_noseed(const struct btrfs_fs_devices *fs_devices, > + const struct btrfs_dev_lookup_args *args); > int btrfs_shrink_device(struct btrfs_device *device, u64 new_size); > int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *path); > int btrfs_balance(struct btrfs_fs_info *fs_info, > diff --git a/include/uapi/linux/btrfs.h b/include/uapi/linux/btrfs.h > index 7ada84e4a3ed1..880b565479a12 100644 > --- a/include/uapi/linux/btrfs.h > +++ b/include/uapi/linux/btrfs.h > @@ -1078,6 +1078,8 @@ enum btrfs_err_code { > struct btrfs_ioctl_scrub_args) > #define BTRFS_IOC_DEV_INFO _IOWR(BTRFS_IOCTL_MAGIC, 30, \ > struct btrfs_ioctl_dev_info_args) > +#define BTRFS_IOC_DEV_INFO_NOSEED _IOR(BTRFS_IOCTL_MAGIC, 30, \ > + struct btrfs_ioctl_dev_info_args) > #define BTRFS_IOC_FS_INFO _IOR(BTRFS_IOCTL_MAGIC, 31, \ > struct btrfs_ioctl_fs_info_args) > #define BTRFS_IOC_BALANCE_V2 _IOWR(BTRFS_IOCTL_MAGIC, 32, \
On 2022/8/10 15:20, Flint.Wang wrote: > testcase btrfs249 failed. > [How to reproduce] > mkfs.btrfs -f -d raid1 -m raid1 /dev/sdb /dev/sdc > btrfstune -S 1 /dev/sdb > wipefs -a /dev/sdb > mount -o degraded /dev/sdc /mnt/scratch > btrfs device add -f /dev/sdd /mnt/scratch > btrfs filesystem usage /mnt/scratch > > [Root cause] > mkfs.btrfs -f -d raid1 -m raid1 /dev/sdb /dev/sdc > btrfstune -S 1 /dev/sdb > wipefs -a /dev/sdb > mount -o degraded /dev/sdc /mnt/scratch > > In the above commands, btrfstune command set the sdb and sdc to seeding device. > After that you wipe the filesystem on sdb. After mount, you will find the status of sdb is missing. > > btrfs device add -f /dev/sdd /mnt/scratch: > This command will invoke btrfs_setup_sprout to do the job. > It put the devices on fs_devices->devices to seed_devices list. > So only sdd is on the fs_devices->devices list. sdb(missing), sdc on the seed_devices list. > But when we look into the btrfs_find_devices function, it find devices both in devices list and seed_devices list. > > btrfs filesystem usage /mnt/scratch > This command use ioctl to get device info. The assertion is triggered because it finds the number of devices is inconsistent. > > [My fix solution] > 1. Add noseed argument to btrfs_find_device. It force the function only look into devices list. You can put the new @noseed argument into btrfs_dev_lookup_args, then no need to change all callers. > 2. Add a new ioctl request(BTRFS_IOC_DEV_INFO_NOSEED) in case some application may depend the original ioctl behavior on BTRFS_IOC_DEV_INFO This is too heavy just for this particular case, I stronglly doubt if we really need to go this way. > 3. Modify load_device_info and load_chunk_and_device_info in btrfs-prog for appropriate ioctl call. To me, we can still handle the work just inside btrfs-progs, thus no need to introducing a completely new ioctl which has the same functionality. E.g, go tree-search ioctl to search chunk tree to get a correct number of devices, instead of just relying on fi_args->num_devices. A fix in btrfs-progs is much simpler and easier to maintain than a full new ioctl. Thanks, Qu > > After the change, btrfs249 passed. > > Signed-off-by: Flint.Wang <hmsjwzb@zoho.com> > --- > fs/btrfs/dev-replace.c | 8 ++++---- > fs/btrfs/ioctl.c | 10 ++++++---- > fs/btrfs/scrub.c | 4 ++-- > fs/btrfs/volumes.c | 22 ++++++++++++---------- > fs/btrfs/volumes.h | 5 ++++- > include/uapi/linux/btrfs.h | 2 ++ > 6 files changed, 30 insertions(+), 21 deletions(-) > > diff --git a/fs/btrfs/dev-replace.c b/fs/btrfs/dev-replace.c > index f43196a893ca3..49d3c587c2948 100644 > --- a/fs/btrfs/dev-replace.c > +++ b/fs/btrfs/dev-replace.c > @@ -101,7 +101,7 @@ int btrfs_init_dev_replace(struct btrfs_fs_info *fs_info) > * We don't have a replace item or it's corrupted. If there is > * a replace target, fail the mount. > */ > - if (btrfs_find_device(fs_info->fs_devices, &args)) { > + if (btrfs_find_device(fs_info->fs_devices, &args, false)) { > btrfs_err(fs_info, > "found replace target device without a valid replace item"); > ret = -EUCLEAN; > @@ -163,7 +163,7 @@ int btrfs_init_dev_replace(struct btrfs_fs_info *fs_info) > * We don't have an active replace item but if there is a > * replace target, fail the mount. > */ > - if (btrfs_find_device(fs_info->fs_devices, &args)) { > + if (btrfs_find_device(fs_info->fs_devices, &args, false)) { > btrfs_err(fs_info, > "replace devid present without an active replace item"); > ret = -EUCLEAN; > @@ -174,9 +174,9 @@ int btrfs_init_dev_replace(struct btrfs_fs_info *fs_info) > break; > case BTRFS_IOCTL_DEV_REPLACE_STATE_STARTED: > case BTRFS_IOCTL_DEV_REPLACE_STATE_SUSPENDED: > - dev_replace->tgtdev = btrfs_find_device(fs_info->fs_devices, &args); > + dev_replace->tgtdev = btrfs_find_device(fs_info->fs_devices, &args, false); > args.devid = src_devid; > - dev_replace->srcdev = btrfs_find_device(fs_info->fs_devices, &args); > + dev_replace->srcdev = btrfs_find_device(fs_info->fs_devices, &args, false); > > /* > * allow 'btrfs dev replace_cancel' if src/tgt device is > diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c > index fe0cc816b4eba..bdf1578839c99 100644 > --- a/fs/btrfs/ioctl.c > +++ b/fs/btrfs/ioctl.c > @@ -2039,7 +2039,7 @@ static noinline int btrfs_ioctl_resize(struct file *file, > } > > args.devid = devid; > - device = btrfs_find_device(fs_info->fs_devices, &args); > + device = btrfs_find_device(fs_info->fs_devices, &args, false); > if (!device) { > btrfs_info(fs_info, "resizer unable to find device %llu", > devid); > @@ -3721,7 +3721,7 @@ static long btrfs_ioctl_fs_info(struct btrfs_fs_info *fs_info, > } > > static long btrfs_ioctl_dev_info(struct btrfs_fs_info *fs_info, > - void __user *arg) > + void __user *arg, bool noseed) > { > BTRFS_DEV_LOOKUP_ARGS(args); > struct btrfs_ioctl_dev_info_args *di_args; > @@ -3737,7 +3737,7 @@ static long btrfs_ioctl_dev_info(struct btrfs_fs_info *fs_info, > args.uuid = di_args->uuid; > > rcu_read_lock(); > - dev = btrfs_find_device(fs_info->fs_devices, &args); > + dev = btrfs_find_device(fs_info->fs_devices, &args, noseed); > if (!dev) { > ret = -ENODEV; > goto out; > @@ -5468,7 +5468,7 @@ long btrfs_ioctl(struct file *file, unsigned int > case BTRFS_IOC_FS_INFO: > return btrfs_ioctl_fs_info(fs_info, argp); > case BTRFS_IOC_DEV_INFO: > - return btrfs_ioctl_dev_info(fs_info, argp); > + return btrfs_ioctl_dev_info(fs_info, argp, false); > case BTRFS_IOC_TREE_SEARCH: > return btrfs_ioctl_tree_search(inode, argp); > case BTRFS_IOC_TREE_SEARCH_V2: > @@ -5570,6 +5570,8 @@ long btrfs_ioctl(struct file *file, unsigned int > case BTRFS_IOC_ENCODED_WRITE_32: > return btrfs_ioctl_encoded_write(file, argp, true); > #endif > + case BTRFS_IOC_DEV_INFO_NOSEED: > + return btrfs_ioctl_dev_info(fs_info, argp, true); > } > > return -ENOTTY; > diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c > index 3afe5fa50a631..4b734d76776ca 100644 > --- a/fs/btrfs/scrub.c > +++ b/fs/btrfs/scrub.c > @@ -4143,7 +4143,7 @@ int btrfs_scrub_dev(struct btrfs_fs_info *fs_info, u64 devid, u64 start, > goto out_free_ctx; > > mutex_lock(&fs_info->fs_devices->device_list_mutex); > - dev = btrfs_find_device(fs_info->fs_devices, &args); > + dev = btrfs_find_device(fs_info->fs_devices, &args, false); > if (!dev || (test_bit(BTRFS_DEV_STATE_MISSING, &dev->dev_state) && > !is_dev_replace)) { > mutex_unlock(&fs_info->fs_devices->device_list_mutex); > @@ -4321,7 +4321,7 @@ int btrfs_scrub_progress(struct btrfs_fs_info *fs_info, u64 devid, > struct scrub_ctx *sctx = NULL; > > mutex_lock(&fs_info->fs_devices->device_list_mutex); > - dev = btrfs_find_device(fs_info->fs_devices, &args); > + dev = btrfs_find_device(fs_info->fs_devices, &args, false); > if (dev) > sctx = dev->scrub_ctx; > if (sctx) > diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c > index 272901514b0c1..1abd75e90cd9e 100644 > --- a/fs/btrfs/volumes.c > +++ b/fs/btrfs/volumes.c > @@ -808,7 +808,7 @@ static noinline struct btrfs_device *device_list_add(const char *path, > }; > > mutex_lock(&fs_devices->device_list_mutex); > - device = btrfs_find_device(fs_devices, &args); > + device = btrfs_find_device(fs_devices, &args, false); > > /* > * If this disk has been pulled into an fs devices created by > @@ -2075,7 +2075,7 @@ int btrfs_rm_device(struct btrfs_fs_info *fs_info, > if (ret) > return ret; > > - device = btrfs_find_device(fs_info->fs_devices, args); > + device = btrfs_find_device(fs_info->fs_devices, args, false); > if (!device) { > if (args->missing) > ret = BTRFS_ERROR_DEV_MISSING_NOT_FOUND; > @@ -2381,7 +2381,7 @@ struct btrfs_device *btrfs_find_device_by_devspec( > > if (devid) { > args.devid = devid; > - device = btrfs_find_device(fs_info->fs_devices, &args); > + device = btrfs_find_device(fs_info->fs_devices, &args, false); > if (!device) > return ERR_PTR(-ENOENT); > return device; > @@ -2390,7 +2390,7 @@ struct btrfs_device *btrfs_find_device_by_devspec( > ret = btrfs_get_dev_args_from_path(fs_info, &args, device_path); > if (ret) > return ERR_PTR(ret); > - device = btrfs_find_device(fs_info->fs_devices, &args); > + device = btrfs_find_device(fs_info->fs_devices, &args, false); > btrfs_put_dev_args_from_path(&args); > if (!device) > return ERR_PTR(-ENOENT); > @@ -2551,7 +2551,7 @@ static int btrfs_finish_sprout(struct btrfs_trans_handle *trans) > BTRFS_FSID_SIZE); > args.uuid = dev_uuid; > args.fsid = fs_uuid; > - device = btrfs_find_device(fs_info->fs_devices, &args); > + device = btrfs_find_device(fs_info->fs_devices, &args, false); > BUG_ON(!device); /* Logic error */ > > if (device->fs_devices->seeding) { > @@ -6821,7 +6821,7 @@ static bool dev_args_match_device(const struct btrfs_dev_lookup_args *args, > * only devid is used. > */ > struct btrfs_device *btrfs_find_device(const struct btrfs_fs_devices *fs_devices, > - const struct btrfs_dev_lookup_args *args) > + const struct btrfs_dev_lookup_args *args, bool noseed) > { > struct btrfs_device *device; > struct btrfs_fs_devices *seed_devs; > @@ -6832,6 +6832,8 @@ struct btrfs_device *btrfs_find_device(const struct btrfs_fs_devices *fs_devices > return device; > } > } > + if (noseed) > + return NULL; > > list_for_each_entry(seed_devs, &fs_devices->seed_list, seed_list) { > if (!dev_args_match_fs_devices(args, seed_devs)) > @@ -7095,7 +7097,7 @@ static int read_one_chunk(struct btrfs_key *key, struct extent_buffer *leaf, > btrfs_stripe_dev_uuid_nr(chunk, i), > BTRFS_UUID_SIZE); > args.uuid = uuid; > - map->stripes[i].dev = btrfs_find_device(fs_info->fs_devices, &args); > + map->stripes[i].dev = btrfs_find_device(fs_info->fs_devices, &args, false); > if (!map->stripes[i].dev) { > map->stripes[i].dev = handle_missing_device(fs_info, > devid, uuid); > @@ -7226,7 +7228,7 @@ static int read_one_dev(struct extent_buffer *leaf, > return PTR_ERR(fs_devices); > } > > - device = btrfs_find_device(fs_info->fs_devices, &args); > + device = btrfs_find_device(fs_info->fs_devices, &args, false); > if (!device) { > if (!btrfs_test_opt(fs_info, DEGRADED)) { > btrfs_report_missing_device(fs_info, devid, > @@ -7884,7 +7886,7 @@ int btrfs_get_dev_stats(struct btrfs_fs_info *fs_info, > > mutex_lock(&fs_devices->device_list_mutex); > args.devid = stats->devid; > - dev = btrfs_find_device(fs_info->fs_devices, &args); > + dev = btrfs_find_device(fs_info->fs_devices, &args, false); > mutex_unlock(&fs_devices->device_list_mutex); > > if (!dev) { > @@ -8026,7 +8028,7 @@ static int verify_one_dev_extent(struct btrfs_fs_info *fs_info, > } > > /* Make sure no dev extent is beyond device boundary */ > - dev = btrfs_find_device(fs_info->fs_devices, &args); > + dev = btrfs_find_device(fs_info->fs_devices, &args, false); > if (!dev) { > btrfs_err(fs_info, "failed to find devid %llu", devid); > ret = -EUCLEAN; > diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h > index 5639961b3626f..4b6bcc777f752 100644 > --- a/fs/btrfs/volumes.h > +++ b/fs/btrfs/volumes.h > @@ -609,7 +609,10 @@ int btrfs_num_copies(struct btrfs_fs_info *fs_info, u64 logical, u64 len); > int btrfs_grow_device(struct btrfs_trans_handle *trans, > struct btrfs_device *device, u64 new_size); > struct btrfs_device *btrfs_find_device(const struct btrfs_fs_devices *fs_devices, > - const struct btrfs_dev_lookup_args *args); > + const struct btrfs_dev_lookup_args *args, > + bool noseed); > +struct btrfs_device *btrfs_find_device_noseed(const struct btrfs_fs_devices *fs_devices, > + const struct btrfs_dev_lookup_args *args); > int btrfs_shrink_device(struct btrfs_device *device, u64 new_size); > int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *path); > int btrfs_balance(struct btrfs_fs_info *fs_info, > diff --git a/include/uapi/linux/btrfs.h b/include/uapi/linux/btrfs.h > index 7ada84e4a3ed1..880b565479a12 100644 > --- a/include/uapi/linux/btrfs.h > +++ b/include/uapi/linux/btrfs.h > @@ -1078,6 +1078,8 @@ enum btrfs_err_code { > struct btrfs_ioctl_scrub_args) > #define BTRFS_IOC_DEV_INFO _IOWR(BTRFS_IOCTL_MAGIC, 30, \ > struct btrfs_ioctl_dev_info_args) > +#define BTRFS_IOC_DEV_INFO_NOSEED _IOR(BTRFS_IOCTL_MAGIC, 30, \ > + struct btrfs_ioctl_dev_info_args) > #define BTRFS_IOC_FS_INFO _IOR(BTRFS_IOCTL_MAGIC, 31, \ > struct btrfs_ioctl_fs_info_args) > #define BTRFS_IOC_BALANCE_V2 _IOWR(BTRFS_IOCTL_MAGIC, 32, \
diff --git a/fs/btrfs/dev-replace.c b/fs/btrfs/dev-replace.c index f43196a893ca3..49d3c587c2948 100644 --- a/fs/btrfs/dev-replace.c +++ b/fs/btrfs/dev-replace.c @@ -101,7 +101,7 @@ int btrfs_init_dev_replace(struct btrfs_fs_info *fs_info) * We don't have a replace item or it's corrupted. If there is * a replace target, fail the mount. */ - if (btrfs_find_device(fs_info->fs_devices, &args)) { + if (btrfs_find_device(fs_info->fs_devices, &args, false)) { btrfs_err(fs_info, "found replace target device without a valid replace item"); ret = -EUCLEAN; @@ -163,7 +163,7 @@ int btrfs_init_dev_replace(struct btrfs_fs_info *fs_info) * We don't have an active replace item but if there is a * replace target, fail the mount. */ - if (btrfs_find_device(fs_info->fs_devices, &args)) { + if (btrfs_find_device(fs_info->fs_devices, &args, false)) { btrfs_err(fs_info, "replace devid present without an active replace item"); ret = -EUCLEAN; @@ -174,9 +174,9 @@ int btrfs_init_dev_replace(struct btrfs_fs_info *fs_info) break; case BTRFS_IOCTL_DEV_REPLACE_STATE_STARTED: case BTRFS_IOCTL_DEV_REPLACE_STATE_SUSPENDED: - dev_replace->tgtdev = btrfs_find_device(fs_info->fs_devices, &args); + dev_replace->tgtdev = btrfs_find_device(fs_info->fs_devices, &args, false); args.devid = src_devid; - dev_replace->srcdev = btrfs_find_device(fs_info->fs_devices, &args); + dev_replace->srcdev = btrfs_find_device(fs_info->fs_devices, &args, false); /* * allow 'btrfs dev replace_cancel' if src/tgt device is diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c index fe0cc816b4eba..bdf1578839c99 100644 --- a/fs/btrfs/ioctl.c +++ b/fs/btrfs/ioctl.c @@ -2039,7 +2039,7 @@ static noinline int btrfs_ioctl_resize(struct file *file, } args.devid = devid; - device = btrfs_find_device(fs_info->fs_devices, &args); + device = btrfs_find_device(fs_info->fs_devices, &args, false); if (!device) { btrfs_info(fs_info, "resizer unable to find device %llu", devid); @@ -3721,7 +3721,7 @@ static long btrfs_ioctl_fs_info(struct btrfs_fs_info *fs_info, } static long btrfs_ioctl_dev_info(struct btrfs_fs_info *fs_info, - void __user *arg) + void __user *arg, bool noseed) { BTRFS_DEV_LOOKUP_ARGS(args); struct btrfs_ioctl_dev_info_args *di_args; @@ -3737,7 +3737,7 @@ static long btrfs_ioctl_dev_info(struct btrfs_fs_info *fs_info, args.uuid = di_args->uuid; rcu_read_lock(); - dev = btrfs_find_device(fs_info->fs_devices, &args); + dev = btrfs_find_device(fs_info->fs_devices, &args, noseed); if (!dev) { ret = -ENODEV; goto out; @@ -5468,7 +5468,7 @@ long btrfs_ioctl(struct file *file, unsigned int case BTRFS_IOC_FS_INFO: return btrfs_ioctl_fs_info(fs_info, argp); case BTRFS_IOC_DEV_INFO: - return btrfs_ioctl_dev_info(fs_info, argp); + return btrfs_ioctl_dev_info(fs_info, argp, false); case BTRFS_IOC_TREE_SEARCH: return btrfs_ioctl_tree_search(inode, argp); case BTRFS_IOC_TREE_SEARCH_V2: @@ -5570,6 +5570,8 @@ long btrfs_ioctl(struct file *file, unsigned int case BTRFS_IOC_ENCODED_WRITE_32: return btrfs_ioctl_encoded_write(file, argp, true); #endif + case BTRFS_IOC_DEV_INFO_NOSEED: + return btrfs_ioctl_dev_info(fs_info, argp, true); } return -ENOTTY; diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c index 3afe5fa50a631..4b734d76776ca 100644 --- a/fs/btrfs/scrub.c +++ b/fs/btrfs/scrub.c @@ -4143,7 +4143,7 @@ int btrfs_scrub_dev(struct btrfs_fs_info *fs_info, u64 devid, u64 start, goto out_free_ctx; mutex_lock(&fs_info->fs_devices->device_list_mutex); - dev = btrfs_find_device(fs_info->fs_devices, &args); + dev = btrfs_find_device(fs_info->fs_devices, &args, false); if (!dev || (test_bit(BTRFS_DEV_STATE_MISSING, &dev->dev_state) && !is_dev_replace)) { mutex_unlock(&fs_info->fs_devices->device_list_mutex); @@ -4321,7 +4321,7 @@ int btrfs_scrub_progress(struct btrfs_fs_info *fs_info, u64 devid, struct scrub_ctx *sctx = NULL; mutex_lock(&fs_info->fs_devices->device_list_mutex); - dev = btrfs_find_device(fs_info->fs_devices, &args); + dev = btrfs_find_device(fs_info->fs_devices, &args, false); if (dev) sctx = dev->scrub_ctx; if (sctx) diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c index 272901514b0c1..1abd75e90cd9e 100644 --- a/fs/btrfs/volumes.c +++ b/fs/btrfs/volumes.c @@ -808,7 +808,7 @@ static noinline struct btrfs_device *device_list_add(const char *path, }; mutex_lock(&fs_devices->device_list_mutex); - device = btrfs_find_device(fs_devices, &args); + device = btrfs_find_device(fs_devices, &args, false); /* * If this disk has been pulled into an fs devices created by @@ -2075,7 +2075,7 @@ int btrfs_rm_device(struct btrfs_fs_info *fs_info, if (ret) return ret; - device = btrfs_find_device(fs_info->fs_devices, args); + device = btrfs_find_device(fs_info->fs_devices, args, false); if (!device) { if (args->missing) ret = BTRFS_ERROR_DEV_MISSING_NOT_FOUND; @@ -2381,7 +2381,7 @@ struct btrfs_device *btrfs_find_device_by_devspec( if (devid) { args.devid = devid; - device = btrfs_find_device(fs_info->fs_devices, &args); + device = btrfs_find_device(fs_info->fs_devices, &args, false); if (!device) return ERR_PTR(-ENOENT); return device; @@ -2390,7 +2390,7 @@ struct btrfs_device *btrfs_find_device_by_devspec( ret = btrfs_get_dev_args_from_path(fs_info, &args, device_path); if (ret) return ERR_PTR(ret); - device = btrfs_find_device(fs_info->fs_devices, &args); + device = btrfs_find_device(fs_info->fs_devices, &args, false); btrfs_put_dev_args_from_path(&args); if (!device) return ERR_PTR(-ENOENT); @@ -2551,7 +2551,7 @@ static int btrfs_finish_sprout(struct btrfs_trans_handle *trans) BTRFS_FSID_SIZE); args.uuid = dev_uuid; args.fsid = fs_uuid; - device = btrfs_find_device(fs_info->fs_devices, &args); + device = btrfs_find_device(fs_info->fs_devices, &args, false); BUG_ON(!device); /* Logic error */ if (device->fs_devices->seeding) { @@ -6821,7 +6821,7 @@ static bool dev_args_match_device(const struct btrfs_dev_lookup_args *args, * only devid is used. */ struct btrfs_device *btrfs_find_device(const struct btrfs_fs_devices *fs_devices, - const struct btrfs_dev_lookup_args *args) + const struct btrfs_dev_lookup_args *args, bool noseed) { struct btrfs_device *device; struct btrfs_fs_devices *seed_devs; @@ -6832,6 +6832,8 @@ struct btrfs_device *btrfs_find_device(const struct btrfs_fs_devices *fs_devices return device; } } + if (noseed) + return NULL; list_for_each_entry(seed_devs, &fs_devices->seed_list, seed_list) { if (!dev_args_match_fs_devices(args, seed_devs)) @@ -7095,7 +7097,7 @@ static int read_one_chunk(struct btrfs_key *key, struct extent_buffer *leaf, btrfs_stripe_dev_uuid_nr(chunk, i), BTRFS_UUID_SIZE); args.uuid = uuid; - map->stripes[i].dev = btrfs_find_device(fs_info->fs_devices, &args); + map->stripes[i].dev = btrfs_find_device(fs_info->fs_devices, &args, false); if (!map->stripes[i].dev) { map->stripes[i].dev = handle_missing_device(fs_info, devid, uuid); @@ -7226,7 +7228,7 @@ static int read_one_dev(struct extent_buffer *leaf, return PTR_ERR(fs_devices); } - device = btrfs_find_device(fs_info->fs_devices, &args); + device = btrfs_find_device(fs_info->fs_devices, &args, false); if (!device) { if (!btrfs_test_opt(fs_info, DEGRADED)) { btrfs_report_missing_device(fs_info, devid, @@ -7884,7 +7886,7 @@ int btrfs_get_dev_stats(struct btrfs_fs_info *fs_info, mutex_lock(&fs_devices->device_list_mutex); args.devid = stats->devid; - dev = btrfs_find_device(fs_info->fs_devices, &args); + dev = btrfs_find_device(fs_info->fs_devices, &args, false); mutex_unlock(&fs_devices->device_list_mutex); if (!dev) { @@ -8026,7 +8028,7 @@ static int verify_one_dev_extent(struct btrfs_fs_info *fs_info, } /* Make sure no dev extent is beyond device boundary */ - dev = btrfs_find_device(fs_info->fs_devices, &args); + dev = btrfs_find_device(fs_info->fs_devices, &args, false); if (!dev) { btrfs_err(fs_info, "failed to find devid %llu", devid); ret = -EUCLEAN; diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h index 5639961b3626f..4b6bcc777f752 100644 --- a/fs/btrfs/volumes.h +++ b/fs/btrfs/volumes.h @@ -609,7 +609,10 @@ int btrfs_num_copies(struct btrfs_fs_info *fs_info, u64 logical, u64 len); int btrfs_grow_device(struct btrfs_trans_handle *trans, struct btrfs_device *device, u64 new_size); struct btrfs_device *btrfs_find_device(const struct btrfs_fs_devices *fs_devices, - const struct btrfs_dev_lookup_args *args); + const struct btrfs_dev_lookup_args *args, + bool noseed); +struct btrfs_device *btrfs_find_device_noseed(const struct btrfs_fs_devices *fs_devices, + const struct btrfs_dev_lookup_args *args); int btrfs_shrink_device(struct btrfs_device *device, u64 new_size); int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *path); int btrfs_balance(struct btrfs_fs_info *fs_info, diff --git a/include/uapi/linux/btrfs.h b/include/uapi/linux/btrfs.h index 7ada84e4a3ed1..880b565479a12 100644 --- a/include/uapi/linux/btrfs.h +++ b/include/uapi/linux/btrfs.h @@ -1078,6 +1078,8 @@ enum btrfs_err_code { struct btrfs_ioctl_scrub_args) #define BTRFS_IOC_DEV_INFO _IOWR(BTRFS_IOCTL_MAGIC, 30, \ struct btrfs_ioctl_dev_info_args) +#define BTRFS_IOC_DEV_INFO_NOSEED _IOR(BTRFS_IOCTL_MAGIC, 30, \ + struct btrfs_ioctl_dev_info_args) #define BTRFS_IOC_FS_INFO _IOR(BTRFS_IOCTL_MAGIC, 31, \ struct btrfs_ioctl_fs_info_args) #define BTRFS_IOC_BALANCE_V2 _IOWR(BTRFS_IOCTL_MAGIC, 32, \
testcase btrfs249 failed. [How to reproduce] mkfs.btrfs -f -d raid1 -m raid1 /dev/sdb /dev/sdc btrfstune -S 1 /dev/sdb wipefs -a /dev/sdb mount -o degraded /dev/sdc /mnt/scratch btrfs device add -f /dev/sdd /mnt/scratch btrfs filesystem usage /mnt/scratch [Root cause] mkfs.btrfs -f -d raid1 -m raid1 /dev/sdb /dev/sdc btrfstune -S 1 /dev/sdb wipefs -a /dev/sdb mount -o degraded /dev/sdc /mnt/scratch In the above commands, btrfstune command set the sdb and sdc to seeding device. After that you wipe the filesystem on sdb. After mount, you will find the status of sdb is missing. btrfs device add -f /dev/sdd /mnt/scratch: This command will invoke btrfs_setup_sprout to do the job. It put the devices on fs_devices->devices to seed_devices list. So only sdd is on the fs_devices->devices list. sdb(missing), sdc on the seed_devices list. But when we look into the btrfs_find_devices function, it find devices both in devices list and seed_devices list. btrfs filesystem usage /mnt/scratch This command use ioctl to get device info. The assertion is triggered because it finds the number of devices is inconsistent. [My fix solution] 1. Add noseed argument to btrfs_find_device. It force the function only look into devices list. 2. Add a new ioctl request(BTRFS_IOC_DEV_INFO_NOSEED) in case some application may depend the original ioctl behavior on BTRFS_IOC_DEV_INFO 3. Modify load_device_info and load_chunk_and_device_info in btrfs-prog for appropriate ioctl call. After the change, btrfs249 passed. Signed-off-by: Flint.Wang <hmsjwzb@zoho.com> --- fs/btrfs/dev-replace.c | 8 ++++---- fs/btrfs/ioctl.c | 10 ++++++---- fs/btrfs/scrub.c | 4 ++-- fs/btrfs/volumes.c | 22 ++++++++++++---------- fs/btrfs/volumes.h | 5 ++++- include/uapi/linux/btrfs.h | 2 ++ 6 files changed, 30 insertions(+), 21 deletions(-)