linux-btrfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5 0/4] btrfs: match device by dev_t
@ 2022-01-12  5:05 Anand Jain
  2022-01-12  5:05 ` [PATCH v5 1/4] btrfs: harden identification of the stale device Anand Jain
                   ` (5 more replies)
  0 siblings, 6 replies; 7+ messages in thread
From: Anand Jain @ 2022-01-12  5:05 UTC (permalink / raw)
  To: linux-btrfs; +Cc: josef, dsterba, nborisov, l, kreijack

v5: Patch 1/4 device_matched() to return bool; absorb errors in it to false.
     Move if (!device->name) into device_matched().
    Patch 2/4 fix the commit title and add a comment.
    Patch 3/4 add more comment about btrfs_device::devt.
    
v4: Return 1 for device matched in device_matched()
    Use scnprintf() instead of sprintf() in device_matched()
    Change log updated drop commit id
    Use str[0] instead of strlen()
    Change logic from !lookup_bdev() to lookup_bdev == 0

v3: Added patch 3/4 saves and uses dev_t in btrfs_device and
    thus patch 4/4 removes the function device_matched().
    fstests btrfs passed with no new regressions.

v2: Fix 
     sparse: warning: incorrect type in argument 1 (different address spaces)
     For using device->name->str

    Fix Josef suggestion to pass dev_t instead of device-path in the
     patch 2/2.

--- original cover letter -----
Patch 1 is the actual bug fix and should go to stable 5.4 as well.
On 5.4 patch1 conflicts (outside of the changes in the patch),
so not yet marked for the stable.

Patch 2 simplifies calling lookup_bdev() in the device_matched()
by moving the same to the parent function two levels up.

Patch 2 is not merged with 1 because to keep the patch 1 changes local
to a function so that it can be easily backported to 5.4 and 5.10.

We should save the dev_t in struct btrfs_device with that may be
we could clean up a few more things, including fixing the below sparse
warning.

  sparse: sparse: incorrect type in argument 1 (different address spaces)

For using without rcu:

  error = lookup_bdev(device->name->str, &dev_old); 


Anand Jain (4):
  btrfs: harden identification of the stale device
  btrfs: redeclare btrfs_free_stale_devices arg1 to dev_t
  btrfs: add device major-minor info in the struct btrfs_device
  btrfs: use dev_t to match device in device_matched

 fs/btrfs/dev-replace.c |  3 ++
 fs/btrfs/super.c       |  8 ++++-
 fs/btrfs/volumes.c     | 67 +++++++++++++++++-------------------------
 fs/btrfs/volumes.h     |  7 ++++-
 4 files changed, 43 insertions(+), 42 deletions(-)

-- 
2.33.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v5 1/4] btrfs: harden identification of the stale device
  2022-01-12  5:05 [PATCH v5 0/4] btrfs: match device by dev_t Anand Jain
@ 2022-01-12  5:05 ` Anand Jain
  2022-01-12  5:06 ` [PATCH v5 2/4] btrfs: redeclare btrfs_free_stale_devices arg1 to dev_t Anand Jain
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Anand Jain @ 2022-01-12  5:05 UTC (permalink / raw)
  To: linux-btrfs; +Cc: josef, dsterba, nborisov, l, kreijack

Identifying and removing the stale device from the fs_uuids list is done
by the function btrfs_free_stale_devices().
btrfs_free_stale_devices() in turn depends on the function
device_path_matched() to check if the device repeats in more than one
btrfs_device structure.

The matching of the device happens by its path, the device path. However,
when dm mapper is in use, the dm device paths are nothing but a link to
the actual block device, which leads to the device_path_matched() failing
to match.

Fix this by matching the dev_t as provided by lookup_bdev() instead of
plain strcmp() the device paths.

Reported-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
v5: device_matched() now returns bool. On error return false.
    Add comments. Move the if(!device->name) to device_matched().
v4: Return 1 for device matched in device_matched()
    Use scnprintf() instead of sprintf() in device_matched()
v3: -
v2: Fix 
     sparse: warning: incorrect type in argument 1 (different address spaces)
     For using device->name->str

    Fix Josef suggestion to pass dev_t instead of device-path in the
     patch 2/2.
 fs/btrfs/volumes.c | 46 +++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 39 insertions(+), 7 deletions(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 4b244acd0cfa..775d0cba2b9b 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -535,15 +535,49 @@ btrfs_get_bdev_and_sb(const char *device_path, fmode_t flags, void *holder,
 	return ret;
 }
 
-static bool device_path_matched(const char *path, struct btrfs_device *device)
+/*
+ * Check if the device in the 'path' matches with the device in the given
+ * struct btrfs_device '*device'.
+ * Returns:
+ *   true  If it is the same device.
+ *   false If it is not the same device or on error.
+ */
+static bool device_matched(struct btrfs_device *device, const char *path)
 {
-	int found;
+	char *device_name;
+	dev_t dev_old;
+	dev_t dev_new;
+	int ret;
+
+	/*
+	 * If we are looking for a device with the matching dev_t, then skip
+	 * device without a name (a missing device).
+	 */
+	if (!device->name)
+		return false;
+
+	device_name = kzalloc(BTRFS_PATH_NAME_MAX, GFP_KERNEL);
+	if (!device_name)
+		return false;
 
 	rcu_read_lock();
-	found = strcmp(rcu_str_deref(device->name), path);
+	scnprintf(device_name, BTRFS_PATH_NAME_MAX, "%s",
+		  rcu_str_deref(device->name));
 	rcu_read_unlock();
 
-	return found == 0;
+	ret = lookup_bdev(device_name, &dev_old);
+	kfree(device_name);
+	if (ret)
+		return false;
+
+	ret = lookup_bdev(path, &dev_new);
+	if (ret)
+		return false;
+
+	if (dev_old == dev_new)
+		return true;
+
+	return false;
 }
 
 /*
@@ -576,9 +610,7 @@ static int btrfs_free_stale_devices(const char *path,
 					 &fs_devices->devices, dev_list) {
 			if (skip_device && skip_device == device)
 				continue;
-			if (path && !device->name)
-				continue;
-			if (path && !device_path_matched(path, device))
+			if (path && device_matched(device, path) == false)
 				continue;
 			if (fs_devices->opened) {
 				/* for an already deleted device return 0 */
-- 
2.33.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v5 2/4] btrfs: redeclare btrfs_free_stale_devices arg1 to dev_t
  2022-01-12  5:05 [PATCH v5 0/4] btrfs: match device by dev_t Anand Jain
  2022-01-12  5:05 ` [PATCH v5 1/4] btrfs: harden identification of the stale device Anand Jain
@ 2022-01-12  5:06 ` Anand Jain
  2022-01-12  5:06 ` [PATCH v5 3/4] btrfs: add device major-minor info in the struct btrfs_device Anand Jain
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Anand Jain @ 2022-01-12  5:06 UTC (permalink / raw)
  To: linux-btrfs; +Cc: josef, dsterba, nborisov, l, kreijack

After the commit (btrfs: harden identification of the stale device), we
don't have to match the device path anymore. Instead, we match the dev_t.
So pass in the dev_t instead of the device-path, in the call chain
btrfs_forget_devices()->btrfs_free_stale_devices().

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
v5: Fix commit title; add a comment
v4: Change log updated drop commit id
    Use name[0] instead of strlen()
    Change logic from !lookup_bdev() to lookup_bdev == 0
v3: -
 fs/btrfs/super.c   |  8 +++++++-
 fs/btrfs/volumes.c | 49 +++++++++++++++++++++++++---------------------
 fs/btrfs/volumes.h |  2 +-
 3 files changed, 35 insertions(+), 24 deletions(-)

diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 1ff03fb6c64a..6eca93096ca5 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -2386,6 +2386,7 @@ static long btrfs_control_ioctl(struct file *file, unsigned int cmd,
 {
 	struct btrfs_ioctl_vol_args *vol;
 	struct btrfs_device *device = NULL;
+	dev_t devt = 0;
 	int ret = -ENOTTY;
 
 	if (!capable(CAP_SYS_ADMIN))
@@ -2405,7 +2406,12 @@ static long btrfs_control_ioctl(struct file *file, unsigned int cmd,
 		mutex_unlock(&uuid_mutex);
 		break;
 	case BTRFS_IOC_FORGET_DEV:
-		ret = btrfs_forget_devices(vol->name);
+		if (vol->name[0] != '\0') {
+			ret = lookup_bdev(vol->name, &devt);
+			if (ret)
+				break;
+		}
+		ret = btrfs_forget_devices(devt);
 		break;
 	case BTRFS_IOC_DEVICES_READY:
 		mutex_lock(&uuid_mutex);
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 775d0cba2b9b..8cd273a9b325 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -542,11 +542,10 @@ btrfs_get_bdev_and_sb(const char *device_path, fmode_t flags, void *holder,
  *   true  If it is the same device.
  *   false If it is not the same device or on error.
  */
-static bool device_matched(struct btrfs_device *device, const char *path)
+static bool device_matched(struct btrfs_device *device, dev_t dev_new)
 {
 	char *device_name;
 	dev_t dev_old;
-	dev_t dev_new;
 	int ret;
 
 	/*
@@ -570,10 +569,6 @@ static bool device_matched(struct btrfs_device *device, const char *path)
 	if (ret)
 		return false;
 
-	ret = lookup_bdev(path, &dev_new);
-	if (ret)
-		return false;
-
 	if (dev_old == dev_new)
 		return true;
 
@@ -583,16 +578,16 @@ static bool device_matched(struct btrfs_device *device, const char *path)
 /*
  *  Search and remove all stale (devices which are not mounted) devices.
  *  When both inputs are NULL, it will search and release all stale devices.
- *  path:	Optional. When provided will it release all unmounted devices
- *		matching this path only.
+ *  devt:	Optional. When provided will it release all unmounted devices
+ *		matching this devt only.
  *  skip_dev:	Optional. Will skip this device when searching for the stale
  *		devices.
- *  Return:	0 for success or if @path is NULL.
- * 		-EBUSY if @path is a mounted device.
- * 		-ENOENT if @path does not match any device in the list.
+ *  Return:	0 for success or if @devt is 0.
+ *		-EBUSY if @devt is a mounted device.
+ *		-ENOENT if @devt does not match any device in the list.
  */
-static int btrfs_free_stale_devices(const char *path,
-				     struct btrfs_device *skip_device)
+static int btrfs_free_stale_devices(dev_t devt,
+				    struct btrfs_device *skip_device)
 {
 	struct btrfs_fs_devices *fs_devices, *tmp_fs_devices;
 	struct btrfs_device *device, *tmp_device;
@@ -600,7 +595,7 @@ static int btrfs_free_stale_devices(const char *path,
 
 	lockdep_assert_held(&uuid_mutex);
 
-	if (path)
+	if (devt)
 		ret = -ENOENT;
 
 	list_for_each_entry_safe(fs_devices, tmp_fs_devices, &fs_uuids, fs_list) {
@@ -610,11 +605,11 @@ static int btrfs_free_stale_devices(const char *path,
 					 &fs_devices->devices, dev_list) {
 			if (skip_device && skip_device == device)
 				continue;
-			if (path && device_matched(device, path) == false)
+			if (devt && device_matched(device, devt) == false)
 				continue;
 			if (fs_devices->opened) {
 				/* for an already deleted device return 0 */
-				if (path && ret != 0)
+				if (devt && ret != 0)
 					ret = -EBUSY;
 				break;
 			}
@@ -1373,12 +1368,12 @@ static struct btrfs_super_block *btrfs_read_disk_super(struct block_device *bdev
 	return disk_super;
 }
 
-int btrfs_forget_devices(const char *path)
+int btrfs_forget_devices(dev_t devt)
 {
 	int ret;
 
 	mutex_lock(&uuid_mutex);
-	ret = btrfs_free_stale_devices(strlen(path) ? path : NULL, NULL);
+	ret = btrfs_free_stale_devices(devt, NULL);
 	mutex_unlock(&uuid_mutex);
 
 	return ret;
@@ -1427,9 +1422,15 @@ struct btrfs_device *btrfs_scan_one_device(const char *path, fmode_t flags,
 	}
 
 	device = device_list_add(path, disk_super, &new_device_added);
-	if (!IS_ERR(device)) {
-		if (new_device_added)
-			btrfs_free_stale_devices(path, device);
+	if (!IS_ERR(device) && new_device_added) {
+		dev_t devt;
+
+		/*
+		 * It is ok to ignore if we fail to free the stale device (if
+		 * any). As there is nothing much that can be done about it.
+		 */
+		if (lookup_bdev(path, &devt) == 0)
+			btrfs_free_stale_devices(devt, device);
 	}
 
 	btrfs_release_disk_super(disk_super);
@@ -2660,6 +2661,7 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path
 	int ret = 0;
 	bool seeding_dev = false;
 	bool locked = false;
+	dev_t devt;
 
 	if (sb_rdonly(sb) && !fs_devices->seeding)
 		return -EROFS;
@@ -2851,10 +2853,13 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path
 	 * Now that we have written a new super block to this device, check all
 	 * other fs_devices list if device_path alienates any other scanned
 	 * device.
+	 * Skip forget_deivces if lookup_bdev() fails as there is nothing much
+	 * that can do about it.
 	 * We can ignore the return value as it typically returns -EINVAL and
 	 * only succeeds if the device was an alien.
 	 */
-	btrfs_forget_devices(device_path);
+	if (lookup_bdev(device_path, &devt) == 0)
+		btrfs_forget_devices(devt);
 
 	/* Update ctime/mtime for blkid or udev */
 	update_dev_time(device_path);
diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h
index 98bbb293a3f9..76215de8ce34 100644
--- a/fs/btrfs/volumes.h
+++ b/fs/btrfs/volumes.h
@@ -529,7 +529,7 @@ int btrfs_open_devices(struct btrfs_fs_devices *fs_devices,
 		       fmode_t flags, void *holder);
 struct btrfs_device *btrfs_scan_one_device(const char *path,
 					   fmode_t flags, void *holder);
-int btrfs_forget_devices(const char *path);
+int btrfs_forget_devices(dev_t devt);
 void btrfs_close_devices(struct btrfs_fs_devices *fs_devices);
 void btrfs_free_extra_devids(struct btrfs_fs_devices *fs_devices);
 void btrfs_assign_next_active_device(struct btrfs_device *device,
-- 
2.33.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v5 3/4] btrfs: add device major-minor info in the struct btrfs_device
  2022-01-12  5:05 [PATCH v5 0/4] btrfs: match device by dev_t Anand Jain
  2022-01-12  5:05 ` [PATCH v5 1/4] btrfs: harden identification of the stale device Anand Jain
  2022-01-12  5:06 ` [PATCH v5 2/4] btrfs: redeclare btrfs_free_stale_devices arg1 to dev_t Anand Jain
@ 2022-01-12  5:06 ` Anand Jain
  2022-01-12  5:06 ` [PATCH v5 4/4] btrfs: use dev_t to match device in device_matched Anand Jain
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Anand Jain @ 2022-01-12  5:06 UTC (permalink / raw)
  To: linux-btrfs; +Cc: josef, dsterba, nborisov, l, kreijack

Internally it is common to use the major-minor number to identify a device
and, at a few locations in btrfs, we use the major-minor number to match
the device.

So when we identify a new btrfs device through device add or device
replace or device-scan/ready save the device's major-minor (dev_t) in the
struct btrfs_device so that we don't have to call lookup_bdev() again.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
v5: Add more comment for the member btrfs_device::devt.
v4: Add comment; And, use the logic lookup_bdev() == 0;
v3: -

 fs/btrfs/dev-replace.c |  3 +++
 fs/btrfs/volumes.c     | 40 +++++++++++++++-------------------------
 fs/btrfs/volumes.h     |  5 +++++
 3 files changed, 23 insertions(+), 25 deletions(-)

diff --git a/fs/btrfs/dev-replace.c b/fs/btrfs/dev-replace.c
index 62b9651ea662..289d6cc1f5db 100644
--- a/fs/btrfs/dev-replace.c
+++ b/fs/btrfs/dev-replace.c
@@ -302,6 +302,9 @@ static int btrfs_init_dev_replace_tgtdev(struct btrfs_fs_info *fs_info,
 		goto error;
 	}
 	rcu_assign_pointer(device->name, name);
+	ret = lookup_bdev(device_path, &device->devt);
+	if (ret)
+		goto error;
 
 	set_bit(BTRFS_DEV_STATE_WRITEABLE, &device->dev_state);
 	device->generation = 0;
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 8cd273a9b325..5044b8f9cfff 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -818,11 +818,17 @@ static noinline struct btrfs_device *device_list_add(const char *path,
 	struct rcu_string *name;
 	u64 found_transid = btrfs_super_generation(disk_super);
 	u64 devid = btrfs_stack_device_id(&disk_super->dev_item);
+	dev_t path_devt;
+	int error;
 	bool has_metadata_uuid = (btrfs_super_incompat_flags(disk_super) &
 		BTRFS_FEATURE_INCOMPAT_METADATA_UUID);
 	bool fsid_change_in_progress = (btrfs_super_flags(disk_super) &
 					BTRFS_SUPER_FLAG_CHANGING_FSID_V2);
 
+	error = lookup_bdev(path, &path_devt);
+	if (error)
+		return ERR_PTR(error);
+
 	if (fsid_change_in_progress) {
 		if (!has_metadata_uuid)
 			fs_devices = find_fsid_inprogress(disk_super);
@@ -905,6 +911,7 @@ static noinline struct btrfs_device *device_list_add(const char *path,
 			return ERR_PTR(-ENOMEM);
 		}
 		rcu_assign_pointer(device->name, name);
+		device->devt = path_devt;
 
 		list_add_rcu(&device->dev_list, &fs_devices->devices);
 		fs_devices->num_devices++;
@@ -967,16 +974,7 @@ static noinline struct btrfs_device *device_list_add(const char *path,
 		 * make sure it's the same device if the device is mounted
 		 */
 		if (device->bdev) {
-			int error;
-			dev_t path_dev;
-
-			error = lookup_bdev(path, &path_dev);
-			if (error) {
-				mutex_unlock(&fs_devices->device_list_mutex);
-				return ERR_PTR(error);
-			}
-
-			if (device->bdev->bd_dev != path_dev) {
+			if (device->devt != path_devt) {
 				mutex_unlock(&fs_devices->device_list_mutex);
 				/*
 				 * device->fs_info may not be reliable here, so
@@ -1009,6 +1007,7 @@ static noinline struct btrfs_device *device_list_add(const char *path,
 			fs_devices->missing_devices--;
 			clear_bit(BTRFS_DEV_STATE_MISSING, &device->dev_state);
 		}
+		device->devt = path_devt;
 	}
 
 	/*
@@ -1422,16 +1421,8 @@ struct btrfs_device *btrfs_scan_one_device(const char *path, fmode_t flags,
 	}
 
 	device = device_list_add(path, disk_super, &new_device_added);
-	if (!IS_ERR(device) && new_device_added) {
-		dev_t devt;
-
-		/*
-		 * It is ok to ignore if we fail to free the stale device (if
-		 * any). As there is nothing much that can be done about it.
-		 */
-		if (lookup_bdev(path, &devt) == 0)
-			btrfs_free_stale_devices(devt, device);
-	}
+	if (!IS_ERR(device) && new_device_added)
+		btrfs_free_stale_devices(device->devt, device);
 
 	btrfs_release_disk_super(disk_super);
 
@@ -2661,7 +2652,6 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path
 	int ret = 0;
 	bool seeding_dev = false;
 	bool locked = false;
-	dev_t devt;
 
 	if (sb_rdonly(sb) && !fs_devices->seeding)
 		return -EROFS;
@@ -2711,6 +2701,9 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path
 
 	device->fs_info = fs_info;
 	device->bdev = bdev;
+	ret = lookup_bdev(device_path, &device->devt);
+	if (ret)
+		goto error_free_device;
 
 	ret = btrfs_get_dev_zone_info(device, false);
 	if (ret)
@@ -2853,13 +2846,10 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path
 	 * Now that we have written a new super block to this device, check all
 	 * other fs_devices list if device_path alienates any other scanned
 	 * device.
-	 * Skip forget_deivces if lookup_bdev() fails as there is nothing much
-	 * that can do about it.
 	 * We can ignore the return value as it typically returns -EINVAL and
 	 * only succeeds if the device was an alien.
 	 */
-	if (lookup_bdev(device_path, &devt) == 0)
-		btrfs_forget_devices(devt);
+	btrfs_forget_devices(device->devt);
 
 	/* Update ctime/mtime for blkid or udev */
 	update_dev_time(device_path);
diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h
index 76215de8ce34..d0c6dfcfc0f6 100644
--- a/fs/btrfs/volumes.h
+++ b/fs/btrfs/volumes.h
@@ -73,6 +73,11 @@ struct btrfs_device {
 	/* the mode sent to blkdev_get */
 	fmode_t mode;
 
+	/*
+	 * Device's major-minor number. Must set even if the device is not
+	 * opened (bdev == NULL), unless the device is missing.
+	 */
+	dev_t devt;
 	unsigned long dev_state;
 	blk_status_t last_flush_error;
 
-- 
2.33.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v5 4/4] btrfs: use dev_t to match device in device_matched
  2022-01-12  5:05 [PATCH v5 0/4] btrfs: match device by dev_t Anand Jain
                   ` (2 preceding siblings ...)
  2022-01-12  5:06 ` [PATCH v5 3/4] btrfs: add device major-minor info in the struct btrfs_device Anand Jain
@ 2022-01-12  5:06 ` Anand Jain
  2022-01-18  2:02 ` [PATCH v5 0/4] btrfs: match device by dev_t Anand Jain
  2022-01-18 15:31 ` David Sterba
  5 siblings, 0 replies; 7+ messages in thread
From: Anand Jain @ 2022-01-12  5:06 UTC (permalink / raw)
  To: linux-btrfs; +Cc: josef, dsterba, nborisov, l, kreijack

Commit 6531891b2bcb ("btrfs: add device major-minor info in the struct
btrfs_device") saved the device major-minor number in the struct
btrfs_device upon discovering it.

So no need to lookup_bdev() again just match, which means
device_matched() can go away.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
v5: -
v4: -
v3: -

 fs/btrfs/volumes.c | 42 +-----------------------------------------
 1 file changed, 1 insertion(+), 41 deletions(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 5044b8f9cfff..87d53b3a3377 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -535,46 +535,6 @@ btrfs_get_bdev_and_sb(const char *device_path, fmode_t flags, void *holder,
 	return ret;
 }
 
-/*
- * Check if the device in the 'path' matches with the device in the given
- * struct btrfs_device '*device'.
- * Returns:
- *   true  If it is the same device.
- *   false If it is not the same device or on error.
- */
-static bool device_matched(struct btrfs_device *device, dev_t dev_new)
-{
-	char *device_name;
-	dev_t dev_old;
-	int ret;
-
-	/*
-	 * If we are looking for a device with the matching dev_t, then skip
-	 * device without a name (a missing device).
-	 */
-	if (!device->name)
-		return false;
-
-	device_name = kzalloc(BTRFS_PATH_NAME_MAX, GFP_KERNEL);
-	if (!device_name)
-		return false;
-
-	rcu_read_lock();
-	scnprintf(device_name, BTRFS_PATH_NAME_MAX, "%s",
-		  rcu_str_deref(device->name));
-	rcu_read_unlock();
-
-	ret = lookup_bdev(device_name, &dev_old);
-	kfree(device_name);
-	if (ret)
-		return false;
-
-	if (dev_old == dev_new)
-		return true;
-
-	return false;
-}
-
 /*
  *  Search and remove all stale (devices which are not mounted) devices.
  *  When both inputs are NULL, it will search and release all stale devices.
@@ -605,7 +565,7 @@ static int btrfs_free_stale_devices(dev_t devt,
 					 &fs_devices->devices, dev_list) {
 			if (skip_device && skip_device == device)
 				continue;
-			if (devt && device_matched(device, devt) == false)
+			if (devt && devt != device->devt)
 				continue;
 			if (fs_devices->opened) {
 				/* for an already deleted device return 0 */
-- 
2.33.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v5 0/4] btrfs: match device by dev_t
  2022-01-12  5:05 [PATCH v5 0/4] btrfs: match device by dev_t Anand Jain
                   ` (3 preceding siblings ...)
  2022-01-12  5:06 ` [PATCH v5 4/4] btrfs: use dev_t to match device in device_matched Anand Jain
@ 2022-01-18  2:02 ` Anand Jain
  2022-01-18 15:31 ` David Sterba
  5 siblings, 0 replies; 7+ messages in thread
From: Anand Jain @ 2022-01-18  2:02 UTC (permalink / raw)
  To: dsterba; +Cc: josef, nborisov, l, kreijack, linux-btrfs


David, Gentle ping about this patch set.
Thanks, Anand


On 12/01/2022 13:05, Anand Jain wrote:
> v5: Patch 1/4 device_matched() to return bool; absorb errors in it to false.
>       Move if (!device->name) into device_matched().
>      Patch 2/4 fix the commit title and add a comment.
>      Patch 3/4 add more comment about btrfs_device::devt.
>      
> v4: Return 1 for device matched in device_matched()
>      Use scnprintf() instead of sprintf() in device_matched()
>      Change log updated drop commit id
>      Use str[0] instead of strlen()
>      Change logic from !lookup_bdev() to lookup_bdev == 0
> 
> v3: Added patch 3/4 saves and uses dev_t in btrfs_device and
>      thus patch 4/4 removes the function device_matched().
>      fstests btrfs passed with no new regressions.
> 
> v2: Fix
>       sparse: warning: incorrect type in argument 1 (different address spaces)
>       For using device->name->str
> 
>      Fix Josef suggestion to pass dev_t instead of device-path in the
>       patch 2/2.
> 
> --- original cover letter -----
> Patch 1 is the actual bug fix and should go to stable 5.4 as well.
> On 5.4 patch1 conflicts (outside of the changes in the patch),
> so not yet marked for the stable.
> 
> Patch 2 simplifies calling lookup_bdev() in the device_matched()
> by moving the same to the parent function two levels up.
> 
> Patch 2 is not merged with 1 because to keep the patch 1 changes local
> to a function so that it can be easily backported to 5.4 and 5.10.
> 
> We should save the dev_t in struct btrfs_device with that may be
> we could clean up a few more things, including fixing the below sparse
> warning.
> 
>    sparse: sparse: incorrect type in argument 1 (different address spaces)
> 
> For using without rcu:
> 
>    error = lookup_bdev(device->name->str, &dev_old);
> 
> 
> Anand Jain (4):
>    btrfs: harden identification of the stale device
>    btrfs: redeclare btrfs_free_stale_devices arg1 to dev_t
>    btrfs: add device major-minor info in the struct btrfs_device
>    btrfs: use dev_t to match device in device_matched
> 
>   fs/btrfs/dev-replace.c |  3 ++
>   fs/btrfs/super.c       |  8 ++++-
>   fs/btrfs/volumes.c     | 67 +++++++++++++++++-------------------------
>   fs/btrfs/volumes.h     |  7 ++++-
>   4 files changed, 43 insertions(+), 42 deletions(-)
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v5 0/4] btrfs: match device by dev_t
  2022-01-12  5:05 [PATCH v5 0/4] btrfs: match device by dev_t Anand Jain
                   ` (4 preceding siblings ...)
  2022-01-18  2:02 ` [PATCH v5 0/4] btrfs: match device by dev_t Anand Jain
@ 2022-01-18 15:31 ` David Sterba
  5 siblings, 0 replies; 7+ messages in thread
From: David Sterba @ 2022-01-18 15:31 UTC (permalink / raw)
  To: Anand Jain; +Cc: linux-btrfs, josef, dsterba, nborisov, l, kreijack

On Wed, Jan 12, 2022 at 01:05:58PM +0800, Anand Jain wrote:
> v5: Patch 1/4 device_matched() to return bool; absorb errors in it to false.
>      Move if (!device->name) into device_matched().
>     Patch 2/4 fix the commit title and add a comment.
>     Patch 3/4 add more comment about btrfs_device::devt.
>     
> v4: Return 1 for device matched in device_matched()
>     Use scnprintf() instead of sprintf() in device_matched()
>     Change log updated drop commit id
>     Use str[0] instead of strlen()
>     Change logic from !lookup_bdev() to lookup_bdev == 0
> 
> v3: Added patch 3/4 saves and uses dev_t in btrfs_device and
>     thus patch 4/4 removes the function device_matched().
>     fstests btrfs passed with no new regressions.
> 
> v2: Fix 
>      sparse: warning: incorrect type in argument 1 (different address spaces)
>      For using device->name->str
> 
>     Fix Josef suggestion to pass dev_t instead of device-path in the
>      patch 2/2.
> 
> --- original cover letter -----
> Patch 1 is the actual bug fix and should go to stable 5.4 as well.
> On 5.4 patch1 conflicts (outside of the changes in the patch),
> so not yet marked for the stable.
> 
> Patch 2 simplifies calling lookup_bdev() in the device_matched()
> by moving the same to the parent function two levels up.
> 
> Patch 2 is not merged with 1 because to keep the patch 1 changes local
> to a function so that it can be easily backported to 5.4 and 5.10.
> 
> We should save the dev_t in struct btrfs_device with that may be
> we could clean up a few more things, including fixing the below sparse
> warning.
> 
>   sparse: sparse: incorrect type in argument 1 (different address spaces)
> 
> For using without rcu:
> 
>   error = lookup_bdev(device->name->str, &dev_old); 
> 
> 
> Anand Jain (4):
>   btrfs: harden identification of the stale device
>   btrfs: redeclare btrfs_free_stale_devices arg1 to dev_t
>   btrfs: add device major-minor info in the struct btrfs_device
>   btrfs: use dev_t to match device in device_matched

V5 looks good to me, thanks. If there are still fixups needed, I'm not
sure Nick has seen v5, I'll fold them to the patches in misc-next.

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2022-01-18 15:31 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-12  5:05 [PATCH v5 0/4] btrfs: match device by dev_t Anand Jain
2022-01-12  5:05 ` [PATCH v5 1/4] btrfs: harden identification of the stale device Anand Jain
2022-01-12  5:06 ` [PATCH v5 2/4] btrfs: redeclare btrfs_free_stale_devices arg1 to dev_t Anand Jain
2022-01-12  5:06 ` [PATCH v5 3/4] btrfs: add device major-minor info in the struct btrfs_device Anand Jain
2022-01-12  5:06 ` [PATCH v5 4/4] btrfs: use dev_t to match device in device_matched Anand Jain
2022-01-18  2:02 ` [PATCH v5 0/4] btrfs: match device by dev_t Anand Jain
2022-01-18 15:31 ` David Sterba

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).