All of lore.kernel.org
 help / color / mirror / Atom feed
From: Anand Jain <anand.jain@oracle.com>
To: Josef Bacik <josef@toxicpanda.com>, linux-btrfs@vger.kernel.org
Cc: dsterba@suse.com
Subject: Re: [PATCH 01/11] btrfs: initialize sysfs devid and device link for seed device
Date: Wed, 2 Sep 2020 00:16:09 +0800	[thread overview]
Message-ID: <c6f502de-1634-f79f-3da4-9febb4d0df27@oracle.com> (raw)
In-Reply-To: <1df53e43-2ec8-8e67-8a79-ec90782e9e3a@toxicpanda.com>



On 1/9/20 12:21 am, Josef Bacik wrote:
> On 8/30/20 10:40 AM, 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>
>> ---
> 
> Ok again, this will not work.
> 

  This comment was answered here [1] before.
    [1]  https://patchwork.kernel.org/patch/11729425/


  Again let me verify with the boilerplate code [2], which dumps
  btrfs_fs_devices and btrfs_device structures along with its
  memory address.
    [2] https://github.com/asj/bbp.git

> Mount a seed fs, you get fs_info->fs_devices pointed at the seed device, 
> and fs_info->fs_devices->devices_kobj is what is initialized.

Right.

$ mount /dev/sda /btrfs
mount: /btrfs: WARNING: device write-protected, mounted read-only.

Note our fs_devices is at 000000005c8322d4, which is paired with sda.

$ cat /proc/fs/btrfs/devlist | egrep "fsid|addr|device:|kobj"
[fsid: 938a1ff2-f493-4a9a-9ed3-3d2ace2f0fb2]
	fs_devs_addr:		000000005c8322d4 <-- fs_devices
	fsid_kobj_state:	1
	fsid_kobj_insysfs:	1
	kobj_state:		1
	kobj_insysfs:		1
		dev_addr:	0000000017aba761
		device:		/dev/sda
		devid_kobj:	1


> Now you sprout.


The relevant code snippets are as below.

-----------------------------
2344 static int btrfs_prepare_sprout(struct btrfs_fs_info *fs_info)

2361         seed_devices = alloc_fs_devices(NULL, NULL);

2371         old_devices = clone_fs_devices(fs_devices);

2377         list_add(&old_devices->fs_list, &fs_uuids);

2379         memcpy(seed_devices, fs_devices, sizeof(*seed_devices));

2386         list_splice_init_rcu(&fs_devices->devices, 
&seed_devices->devices,
2387                               synchronize_rcu);

2396         list_add_tail(&seed_devices->seed_list, 
&fs_devices->seed_list);

2398         generate_random_uuid(fs_devices->fsid);
-----------------------------


> This does clone_fs_devices(fs_info->fs_devices), which doesn't copy over 
> fs_fdevices->devices_kobj.

  Right. Line 2371.

>  Now we take this clone device, and set 
> fs_info->fsdevices to the cloned fs_devices,

  Hm. No we just add it to the fs_uuids. So that user doesn't have
  to scan again to mount the seed device at a different mount point.
  Line 2377.

> and we add the original 
> fs_devices, which had the sysfs objects init'ed already, to 
> fs_devices->seed_list.

  No. Original fs_devices still remains with fs_info->fs_devices.
  Which moves _devices_ _only_  to seed_devices::devices
  Line 2386.

  Lets verify.

  $ btrfs dev add -f /dev/sdb /btrfs

  Our fs_devices was at 000000005c8322d4.

  And after device add, our fs_devices 000000005c8322d4 is pairing with
  the RW device sdb.

  And the original sda is moved under seed_devices.

  Ignore clone_fs_devices its of no use to our conversation here.
  Which goes away after btrfs dev scan --forget as its not mounted
  anymore.

  As shown below.

$ cat /proc/fs/btrfs/devlist | egrep "fsid|addr|device:|kobj"
[fsid: 938a1ff2-f493-4a9a-9ed3-3d2ace2f0fb2]
	fs_devs_addr:		00000000f0c691d1 <-- clone_fs_devices
	fsid_kobj_state:	0
	fsid_kobj_insysfs:	0
	kobj_state:		null
	kobj_insysfs:		null
		dev_addr:	000000006e24ab09
		device:		/dev/sda
		devid_kobj:	null
[fsid: 91294469-9c0e-45c7-8dc7-af3134ba8cf4]
	fs_devs_addr:		000000005c8322d4 <-- fs_devices
	fsid_kobj_state:	1
	fsid_kobj_insysfs:	1
	kobj_state:		1
	kobj_insysfs:		1
		dev_addr:	0000000039f25fa1
		device:		/dev/sdb
		devid_kobj:	1
	[[seed_fsid: 938a1ff2-f493-4a9a-9ed3-3d2ace2f0fb2]]
		sprout_fsid:		91294469-9c0e-45c7-8dc7-af3134ba8cf4
		fs_devs_addr:		000000005e19db25 <-- seed_devices
		fsid_kobj_state:	1
		fsid_kobj_insysfs:	1
		kobj_state:		1
		kobj_insysfs:		1
			dev_addr:	0000000017aba761
			device:		/dev/sda
			devid_kobj:	1


Clone_fs_devices is freed after forget

$ btrfs dev scan --forget

$ cat /proc/fs/btrfs/devlist | egrep "fsid|addr|device:|kobj"
[fsid: 91294469-9c0e-45c7-8dc7-af3134ba8cf4]
	fs_devs_addr:		000000005c8322d4  <-- fs_devices
	fsid_kobj_state:	1
	fsid_kobj_insysfs:	1
	kobj_state:		1
	kobj_insysfs:		1
		dev_addr:	0000000039f25fa1
		device:		/dev/sdb
		devid_kobj:		1
	[[seed_fsid: 938a1ff2-f493-4a9a-9ed3-3d2ace2f0fb2]]
		sprout_fsid:		91294469-9c0e-45c7-8dc7-af3134ba8cf4
		fs_devs_addr:		000000005e19db25 <-- seed_devcies
		fsid_kobj_state:	1
		fsid_kobj_insysfs:	1
		kobj_state:		1
		kobj_insysfs:		1
			dev_addr:	0000000017aba761
			device:		/dev/sda
			devid_kobj:		1



HTH

Anand

  reply	other threads:[~2020-09-01 16:16 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   ` [PATCH 01/11] btrfs: initialize sysfs devid and device link for seed device Anand Jain
2020-08-31  9:07     ` Nikolay Borisov
2020-08-31 12:00       ` Anand Jain
2020-08-31 16:21     ` Josef Bacik
2020-09-01 16:16       ` Anand Jain [this message]
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=c6f502de-1634-f79f-3da4-9febb4d0df27@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.