Messages in this thread |  | | Date | Thu, 7 Mar 2024 11:41:52 +0800 | | Subject | Re: [PATCH] erofs: fix lockdep false positives on initializing erofs_pseudo_mnt | | From | Jingbo Xu <> |
| |
Hi Baokun,
Thanks for catching this!
On 3/7/24 10:52 AM, Gao Xiang wrote: > Hi Baokun, > > On 2024/3/7 10:44, Baokun Li wrote: >> Lockdep reported the following issue when mounting erofs with a >> domain_id: >> >> ============================================ >> WARNING: possible recursive locking detected >> 6.8.0-rc7-xfstests #521 Not tainted >> -------------------------------------------- >> mount/396 is trying to acquire lock: >> ffff907a8aaaa0e0 (&type->s_umount_key#50/1){+.+.}-{3:3}, >> at: alloc_super+0xe3/0x3d0 >> >> but task is already holding lock: >> ffff907a8aaa90e0 (&type->s_umount_key#50/1){+.+.}-{3:3}, >> at: alloc_super+0xe3/0x3d0 >> >> other info that might help us debug this: >> Possible unsafe locking scenario: >> >> CPU0 >> ---- >> lock(&type->s_umount_key#50/1); >> lock(&type->s_umount_key#50/1); >> >> *** DEADLOCK *** >> >> May be due to missing lock nesting notation >> >> 2 locks held by mount/396: >> #0: ffff907a8aaa90e0 (&type->s_umount_key#50/1){+.+.}-{3:3}, >> at: alloc_super+0xe3/0x3d0 >> #1: ffffffffc00e6f28 (erofs_domain_list_lock){+.+.}-{3:3}, >> at: erofs_fscache_register_fs+0x3d/0x270 [erofs] >> >> stack backtrace: >> CPU: 1 PID: 396 Comm: mount Not tainted 6.8.0-rc7-xfstests #521 >> Call Trace: >> <TASK> >> dump_stack_lvl+0x64/0xb0 >> validate_chain+0x5c4/0xa00 >> __lock_acquire+0x6a9/0xd50 >> lock_acquire+0xcd/0x2b0 >> down_write_nested+0x45/0xd0 >> alloc_super+0xe3/0x3d0 >> sget_fc+0x62/0x2f0 >> vfs_get_super+0x21/0x90 >> vfs_get_tree+0x2c/0xf0 >> fc_mount+0x12/0x40 >> vfs_kern_mount.part.0+0x75/0x90 >> kern_mount+0x24/0x40 >> erofs_fscache_register_fs+0x1ef/0x270 [erofs] >> erofs_fc_fill_super+0x213/0x380 [erofs] >> >> This is because the file_system_type of both erofs and the pseudo-mount >> point of domain_id is erofs_fs_type, so two successive calls to >> alloc_super() are considered to be using the same lock and trigger the >> warning above. >> >> Therefore add a nodev file_system_type named erofs_anon_fs_type to >> silence this complaint. In addition, to reduce code coupling, refactor >> out the erofs_anon_init_fs_context() and erofs_kill_pseudo_sb() functions >> and move the erofs_pseudo_mnt related code to fscache.c. >> >> Signed-off-by: Baokun Li <libaokun1@huawei.com> > > IMHO, in the beginning, I'd like to avoid introducing another fs type > for erofs to share (meta)data between filesystems since it will cause > churn, could we use some alternative way to resolve this?
Yeah as Gao Xiang said, this is initially intended to avoid introducing anothoer file_system_type, say erofs_anon_fs_type.
What we need is actually a method of allocating anonymous inode as a sentinel identifying each blob. There is indeed a global mount, i.e. anon_inode_mnt, for allocating anonymous inode/file specifically. At the time the share domain feature is introduced, there's only one anonymous inode, i.e. anon_inode_inode, and all the allocated anonymous files are bound to this single anon_inode_inode. Thus we decided to implement a erofs internal pseudo mount for this usage.
But I noticed that we can now allocate unique anonymous inodes from anon_inode_mnt since commit e7e832c ("fs: add LSM-supporting anon-inode interface"), though the new interface is initially for LSM usage.
-- Thanks, Jingbo
|  |