All of lore.kernel.org
 help / color / mirror / Atom feed
From: Anand Jain <anand.jain@oracle.com>
To: linux-btrfs@vger.kernel.org
Cc: dsterba@suse.com, josef@toxicpanda.com
Subject: [PATCH 01/11] btrfs: initialize sysfs devid and device link for seed device
Date: Sun, 30 Aug 2020 22:40:56 +0800	[thread overview]
Message-ID: <2db650ec206db1cb3e68590951b59e222fb10116.1598792561.git.anand.jain@oracle.com> (raw)
In-Reply-To: <cover.1598792561.git.anand.jain@oracle.com>

The following test case leads to null kobject-being-freed error.

 mount seed /mnt
 add sprout to /mnt
 umount /mnt
 mount sprout to /mnt
 delete seed

 kobject: '(null)' (00000000dd2b87e4): is not initialized, yet kobject_put() is being called.
 WARNING: CPU: 1 PID: 15784 at lib/kobject.c:736 kobject_put+0x80/0x350
 RIP: 0010:kobject_put+0x80/0x350
 ::
 Call Trace:
 btrfs_sysfs_remove_devices_dir+0x6e/0x160 [btrfs]
 btrfs_rm_device.cold+0xa8/0x298 [btrfs]
 btrfs_ioctl+0x206c/0x22a0 [btrfs]
 ksys_ioctl+0xe2/0x140
 __x64_sys_ioctl+0x1e/0x29
 do_syscall_64+0x96/0x150
 entry_SYSCALL_64_after_hwframe+0x44/0xa9
 RIP: 0033:0x7f4047c6288b
 ::

This is because, at the end of the seed device-delete, we try to remove
the seed's devid sysfs entry. But for the seed devices under the sprout
fs, we don't initialize the devid kobject yet. So this patch initializes
the seed device devid kobject and the device link in the sysfs. This takes
care of the Warning.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 fs/btrfs/sysfs.c | 146 ++++++++++++++++++++++++++++++-----------------
 1 file changed, 93 insertions(+), 53 deletions(-)

diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c
index 190e59152be5..9b5e58091fae 100644
--- a/fs/btrfs/sysfs.c
+++ b/fs/btrfs/sysfs.c
@@ -1149,45 +1149,48 @@ int btrfs_sysfs_add_space_info_type(struct btrfs_fs_info *fs_info,
 	return 0;
 }
 
-/* when one_device is NULL, it removes all device links */
-
-int btrfs_sysfs_remove_devices_dir(struct btrfs_fs_devices *fs_devices,
-		struct btrfs_device *one_device)
+static void btrfs_sysfs_remove_device(struct btrfs_device *device)
 {
 	struct hd_struct *disk;
 	struct kobject *disk_kobj;
+	struct kobject *devices_kobj;
 
-	if (!fs_devices->devices_kobj)
-		return -EINVAL;
+	/*
+	 * Seed fs_devices devices_kobj aren't used, fetch kobject from the
+	 * fs_info::fs_devices.
+	 */
+	devices_kobj = device->fs_info->fs_devices->devices_kobj;
+	ASSERT(devices_kobj);
 
-	if (one_device) {
-		if (one_device->bdev) {
-			disk = one_device->bdev->bd_part;
-			disk_kobj = &part_to_dev(disk)->kobj;
-			sysfs_remove_link(fs_devices->devices_kobj,
-					  disk_kobj->name);
-		}
+	if (device->bdev) {
+		disk = device->bdev->bd_part;
+		disk_kobj = &part_to_dev(disk)->kobj;
+		sysfs_remove_link(devices_kobj, disk_kobj->name);
+	}
 
-		kobject_del(&one_device->devid_kobj);
-		kobject_put(&one_device->devid_kobj);
+	kobject_del(&device->devid_kobj);
+	kobject_put(&device->devid_kobj);
 
-		wait_for_completion(&one_device->kobj_unregister);
+	wait_for_completion(&device->kobj_unregister);
+}
 
+/* when 2nd argument device is NULL, it removes all devices link */
+int btrfs_sysfs_remove_devices_dir(struct btrfs_fs_devices *fs_devices,
+				   struct btrfs_device *device)
+{
+	struct btrfs_fs_devices *seed_fs_devices;
+
+	if (device) {
+		btrfs_sysfs_remove_device(device);
 		return 0;
 	}
 
-	list_for_each_entry(one_device, &fs_devices->devices, dev_list) {
-
-		if (one_device->bdev) {
-			disk = one_device->bdev->bd_part;
-			disk_kobj = &part_to_dev(disk)->kobj;
-			sysfs_remove_link(fs_devices->devices_kobj,
-					  disk_kobj->name);
-		}
-		kobject_del(&one_device->devid_kobj);
-		kobject_put(&one_device->devid_kobj);
+	list_for_each_entry(device, &fs_devices->devices, dev_list)
+		btrfs_sysfs_remove_device(device);
 
-		wait_for_completion(&one_device->kobj_unregister);
+	list_for_each_entry(seed_fs_devices, &fs_devices->seed_list, seed_list) {
+		list_for_each_entry(device, &seed_fs_devices->devices, dev_list)
+			btrfs_sysfs_remove_device(device);
 	}
 
 	return 0;
@@ -1271,44 +1274,81 @@ static struct kobj_type devid_ktype = {
 	.release	= btrfs_release_devid_kobj,
 };
 
+static int btrfs_sysfs_add_device(struct btrfs_device *device)
+{
+	int ret;
+	struct kobject *devices_kobj;
+        struct kobject *devinfo_kobj;
+
+	/*
+	 * make sure we use the fs_info::fs_devices to fetch the kobjects
+	 * even for the seed fs_devices
+	 */
+	devices_kobj = device->fs_devices->fs_info->fs_devices->devices_kobj;
+	devinfo_kobj = device->fs_devices->fs_info->fs_devices->devinfo_kobj;
+	ASSERT(devices_kobj);
+	ASSERT(devinfo_kobj);
+
+	if (device->bdev) {
+		struct hd_struct *disk;
+		struct kobject *disk_kobj;
+
+		disk = device->bdev->bd_part;
+		disk_kobj = &part_to_dev(disk)->kobj;
+
+		ret = sysfs_create_link(devices_kobj, disk_kobj,
+					disk_kobj->name);
+		if (ret) {
+			btrfs_warn(device->fs_info,
+			   "sysfs create device link failed %d devid %llu",
+				   ret, device->devid);
+			return ret;
+		}
+	}
+
+	init_completion(&device->kobj_unregister);
+	ret = kobject_init_and_add(&device->devid_kobj, &devid_ktype,
+				   devinfo_kobj, "%llu", device->devid);
+	if (ret) {
+		kobject_put(&device->devid_kobj);
+		btrfs_warn(device->fs_info,
+			   "sysfs devinfo init failed %d devid %llu",
+			   ret, device->devid);
+	}
+
+	return ret;
+}
+
 int btrfs_sysfs_add_devices_dir(struct btrfs_fs_devices *fs_devices,
-				struct btrfs_device *one_device)
+				struct btrfs_device *device)
 {
-	int error = 0;
-	struct btrfs_device *dev;
+	int ret = 0;
 	unsigned int nofs_flag;
+	struct btrfs_fs_devices *seed_fs_devices;
 
 	nofs_flag = memalloc_nofs_save();
-	list_for_each_entry(dev, &fs_devices->devices, dev_list) {
 
-		if (one_device && one_device != dev)
-			continue;
+	if (device)
+		return btrfs_sysfs_add_device(device);
 
-		if (dev->bdev) {
-			struct hd_struct *disk;
-			struct kobject *disk_kobj;
-
-			disk = dev->bdev->bd_part;
-			disk_kobj = &part_to_dev(disk)->kobj;
-
-			error = sysfs_create_link(fs_devices->devices_kobj,
-						  disk_kobj, disk_kobj->name);
-			if (error)
-				break;
-		}
+	list_for_each_entry(device, &fs_devices->devices, dev_list) {
+		ret = btrfs_sysfs_add_device(device);
+		if (ret)
+			goto out;
+	}
 
-		init_completion(&dev->kobj_unregister);
-		error = kobject_init_and_add(&dev->devid_kobj, &devid_ktype,
-					     fs_devices->devinfo_kobj, "%llu",
-					     dev->devid);
-		if (error) {
-			kobject_put(&dev->devid_kobj);
-			break;
+	list_for_each_entry(seed_fs_devices, &fs_devices->seed_list, seed_list) {
+		list_for_each_entry(device, &seed_fs_devices->devices, dev_list) {
+			ret = btrfs_sysfs_add_device(device);
+			if (ret)
+				goto out;
 		}
 	}
+
+out:
 	memalloc_nofs_restore(nofs_flag);
 
-	return error;
+	return ret;
 }
 
 void btrfs_kobject_uevent(struct block_device *bdev, enum kobject_action action)
-- 
2.25.1


  reply	other threads:[~2020-08-30 14:42 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-30 14:40 Anand Jain
2020-08-30 14:40 ` Anand Jain
2020-08-31  1:38   ` [PATCH 0/11] btrfs: seed fix null ptr, use only main device_list_mutex, and cleanups Anand Jain
2020-08-30 14:40   ` Anand Jain [this message]
2020-08-31  9:07     ` [PATCH 01/11] btrfs: initialize sysfs devid and device link for seed device Nikolay Borisov
2020-08-31 12:00       ` Anand Jain
2020-08-31 16:21     ` Josef Bacik
2020-09-01 16:16       ` Anand Jain
2020-08-30 14:40   ` [PATCH 02/11] btrfs: refactor btrfs_sysfs_add_devices_dir Anand Jain
2020-08-30 14:40   ` [PATCH 03/11] btrfs: refactor btrfs_sysfs_remove_devices_dir Anand Jain
2020-08-31  8:58     ` Nikolay Borisov
2020-08-31  9:12       ` Anand Jain
2020-08-30 14:40   ` [PATCH 04/11] btrfs: reada: use sprout device_list_mutex Anand Jain
2020-08-31  8:54     ` Nikolay Borisov
2020-08-31 16:08     ` Josef Bacik
2020-09-01  9:02       ` Anand Jain
2020-08-30 14:41   ` [PATCH 05/11] btrfs: btrfs_init_devices_late: " Anand Jain
2020-08-31  8:37     ` Nikolay Borisov
2020-09-01  8:54       ` Anand Jain
2020-08-30 14:41   ` [PATCH 06/11] btrfs: open code list_head pointer in btrfs_init_dev_replace_tgtdev Anand Jain
2020-08-31  8:38     ` Nikolay Borisov
2020-08-30 14:41   ` [PATCH 07/11] btrfs: cleanup btrfs_remove_chunk Anand Jain
2020-08-31  8:43     ` Nikolay Borisov
2020-08-30 14:41   ` [PATCH 08/11] btrfs: cleanup btrfs_assign_next_active_device() Anand Jain
2020-08-31  8:44     ` Nikolay Borisov
2020-08-30 14:41   ` [PATCH 09/11] btrfs: cleanup unnecessary goto in open_seed_device Anand Jain
2020-08-31  8:44     ` Nikolay Borisov
2020-08-30 14:41   ` [PATCH 10/11] btrfs: btrfs_dev_replace_update_device_in_mapping_tree drop file global declare Anand Jain
2020-08-31  8:46     ` Nikolay Borisov
2020-08-30 14:41   ` [PATCH 11/11] btrfs: fix replace of seed device Anand Jain
  -- strict thread matches above, loose matches on Subject: below --
2020-08-21 13:15 [PATCH RFC] btrfs/163: replace sprout instead of seed Anand Jain
2020-08-30 14:41 ` [PATCH] fstests: " Anand Jain
2020-08-21 13:15 ` [PATCH 1/2] btrfs: initialize sysfs devid and device link for seed device Anand Jain
2020-08-21 13:15   ` [PATCH RFC 2/2] btrfs: fix replace of " Anand Jain
2020-08-21 14:38     ` Josef Bacik
2020-08-23 15:05       ` Anand Jain
2020-08-21 14:36   ` [PATCH 1/2] btrfs: initialize sysfs devid and device link for " Josef Bacik
2020-08-23 13:05     ` Anand Jain
2020-08-29 11:44   ` Anand Jain

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=2db650ec206db1cb3e68590951b59e222fb10116.1598792561.git.anand.jain@oracle.com \
    --to=anand.jain@oracle.com \
    --cc=dsterba@suse.com \
    --cc=josef@toxicpanda.com \
    --cc=linux-btrfs@vger.kernel.org \
    /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.