All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Flint.Wang" <hmsjwzb@zoho.com>
To: unlisted-recipients:; (no To-header on input)
Cc: anand.jain@oracle.com, nborisov@suse.com, strongbox8@zoho.com,
	hmsjwzb@zoho.com, Chris Mason <clm@fb.com>,
	Josef Bacik <josef@toxicpanda.com>,
	David Sterba <dsterba@suse.com>,
	linux-btrfs@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: [PATCH] btrfs: Fix btrfs_find_device for btrfs/249
Date: Wed, 10 Aug 2022 15:20:21 +0800	[thread overview]
Message-ID: <20220810072021.4539-1-hmsjwzb@zoho.com> (raw)

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(-)

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, \
-- 
2.37.0


             reply	other threads:[~2022-08-10  7:21 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-08-10  7:20 Flint.Wang [this message]
2022-08-10  7:26 ` [PATCH] btrfs: Fix btrfs_find_device for btrfs/249 Qu Wenruo
2022-08-10  7:47 ` Qu Wenruo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220810072021.4539-1-hmsjwzb@zoho.com \
    --to=hmsjwzb@zoho.com \
    --cc=anand.jain@oracle.com \
    --cc=clm@fb.com \
    --cc=dsterba@suse.com \
    --cc=josef@toxicpanda.com \
    --cc=linux-btrfs@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nborisov@suse.com \
    --cc=strongbox8@zoho.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.