From: Nikolay Borisov <nborisov@suse.com>
To: Anand Jain <anand.jain@oracle.com>, linux-btrfs@vger.kernel.org
Cc: dsterba@suse.com, josef@toxicpanda.com
Subject: Re: [PATCH 01/11] btrfs: initialize sysfs devid and device link for seed device
Date: Mon, 31 Aug 2020 12:07:55 +0300 [thread overview]
Message-ID: <e6175fbf-3439-f290-d3d6-7fb1320f91e6@suse.com> (raw)
In-Reply-To: <2db650ec206db1cb3e68590951b59e222fb10116.1598792561.git.anand.jain@oracle.com>
On 30.08.20 г. 17:40 ч., Anand Jain wrote:
> 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>
This patch is doing too many things at once. Split it so that:
1. You first add the new functions add_device/remove_device in 2
separate patches.
2. Switch btrfs_sysfs_add_devices_dir/btrfs_sysfs_remove_devices_dir to
ousing the newly added helpers, again in 2 separate patches
> ---
> 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)
>
next prev parent reply other threads:[~2020-08-31 9:08 UTC|newest]
Thread overview: 38+ messages / expand[flat|nested] mbox.gz Atom feed top
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-21 13:15 ` [PATCH RFC] btrfs/163: replace sprout instead of seed 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
2020-08-30 14:41 ` [PATCH] fstests: btrfs/163: replace sprout instead of seed Anand Jain
2020-08-30 14:40 ` Anand Jain
2020-08-30 14:40 ` [PATCH 01/11] btrfs: initialize sysfs devid and device link for seed device Anand Jain
2020-08-31 9:07 ` Nikolay Borisov [this message]
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
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=e6175fbf-3439-f290-d3d6-7fb1320f91e6@suse.com \
--to=nborisov@suse.com \
--cc=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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox