Thread (5 messages) 5 messages, 2 authors, 2021-12-16

Re: [PATCH] btrfs: Create sysfs entries only for non-stale devices

From: Nikolay Borisov <hidden>
Date: 2021-12-15 14:47:42


On 15.12.21 г. 16:46, Nikolay Borisov wrote:
quoted hunk ↗ jump to hunk
Currently /sys/fs/btrfs/<uuid>/devinfo/<devid> entry is always created
for a device present in btrfs_fs_devices on the other hand
/sys/fs/btrfs/<uuid>/devices/<devname> sysfs link is created only when
the given btrfs_device::bdisk member is populated. This can lead to
cases where a filesystem consisting of 2 device, one of which is stale
ends up having 2 entries under /sys/fs/btrfs/<uuid>/devinfo/<devid>
but only one under /sys/fs/btrfs/<uuid>/devices/<devname>.

Another case that occurs is if a filesystem initially occupied 2
devices, then got unmounted, and a new filesystem is created, which
occupies a single device but with the same UUID as the one occupying 2
devices. In this case /sys/fs/btrfs/<uuid>/devices/<devname> will
correctly have 1 entry but /sys/fs/btrfs/<uuid>/devices/<devname> will
incorrectly has 2. This behavior is demonstrated by the following
script:

    UUID=292afefb-6e8c-4fb3-9d12-8c4ecb1f2374
    rm /tmp/d1
    rm /tmp/d2
    truncate -s 1G /tmp/d1
    truncate -s 1G /tmp/d2
    sudo losetup /dev/loop1 /tmp/d1
    sudo losetup /dev/loop2 /tmp/d2
    sudo mkfs.btrfs -U $UUID /dev/loop1 /dev/loop2
    sudo mount /dev/loop1 /mnt/btrfs1
    sudo umount /dev/loop1
    sudo losetup -d /dev/loop2
    sudo losetup -d /dev/loop1

    # create a new filesystem with only ONE loop-device; mount it
    rm /tmp/d1
    truncate -s 1G /tmp/d1
    sudo losetup /dev/loop1 /tmp/d1
    sudo mkfs.btrfs -U $UUID /dev/loop1
    sudo mount /dev/loop1 /mnt/btrfs1

Fix this by ensuring that device sysfs attributes are only added for
devices which are actually present at the time of mount.

Signed-off-by: Nikolay Borisov <redacted>
---
 fs/btrfs/sysfs.c | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)
diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c
index beb7f72d50b8..e2e110d7798a 100644
--- a/fs/btrfs/sysfs.c
+++ b/fs/btrfs/sysfs.c
@@ -12,6 +12,7 @@
 #include <crypto/hash.h>
 
 #include "ctree.h"
+#include "rcu-string.h"
Doh, that's unrelated and is a remain of some debug aid. If there are no
major feedback on the patch David could you remove it when merging.
quoted hunk ↗ jump to hunk
 #include "discard.h"
 #include "disk-io.h"
 #include "send.h"
@@ -1611,9 +1612,13 @@ int btrfs_sysfs_add_device(struct btrfs_device *device)
 {
 	int ret;
 	unsigned int nofs_flag;
+	struct kobject *disk_kobj;
 	struct kobject *devices_kobj;
 	struct kobject *devinfo_kobj;
 
+	if (!device->bdev)
+		return 0;
+
 	/*
 	 * Make sure we use the fs_info::fs_devices to fetch the kobjects even
 	 * for the seed fs_devices
@@ -1625,16 +1630,14 @@ int btrfs_sysfs_add_device(struct btrfs_device *device)
 
 	nofs_flag = memalloc_nofs_save();
 
-	if (device->bdev) {
-		struct kobject *disk_kobj = bdev_kobj(device->bdev);
+	disk_kobj = bdev_kobj(device->bdev);
 
-		ret = sysfs_create_link(devices_kobj, disk_kobj, disk_kobj->name);
-		if (ret) {
-			btrfs_warn(device->fs_info,
-				"creating sysfs device link for devid %llu failed: %d",
-				device->devid, ret);
-			goto out;
-		}
+	ret = sysfs_create_link(devices_kobj, disk_kobj, disk_kobj->name);
+	if (ret) {
+		btrfs_warn(device->fs_info,
+			   "creating sysfs device link for devid %llu failed: %d",
+			   device->devid, ret);
+		goto out;
 	}
 
 	init_completion(&device->kobj_unregister);
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help