| Message ID | 20240618071922.2127289-1-xiujianfeng@huawei.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v2,-next] mm/hugetlb_cgroup: register lockdep key for cftype | expand |
On 18.06.2024 09:19, Xiu Jianfeng wrote: > When CONFIG_DEBUG_LOCK_ALLOC is enabled, the following commands can > trigger a bug, > > mount -t cgroup2 none /sys/fs/cgroup > cd /sys/fs/cgroup > echo "+hugetlb" > cgroup.subtree_control > > The log is as below: > > BUG: key ffff8880046d88d8 has not been registered! > ------------[ cut here ]------------ > DEBUG_LOCKS_WARN_ON(1) > WARNING: CPU: 3 PID: 226 at kernel/locking/lockdep.c:4945 lockdep_init_map_type+0x185/0x220 > Modules linked in: > CPU: 3 PID: 226 Comm: bash Not tainted 6.10.0-rc4-next-20240617-g76db4c64526c #544 > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.15.0-1 04/01/2014 > RIP: 0010:lockdep_init_map_type+0x185/0x220 > Code: 00 85 c0 0f 84 6c ff ff ff 8b 3d 6a d1 85 01 85 ff 0f 85 5e ff ff ff 48 c7 c6 21 99 4a 82 48 c7 c7 60 29 49 82 e8 3b 2e f5 > RSP: 0018:ffffc9000083fc30 EFLAGS: 00000282 > RAX: 0000000000000000 RBX: ffffffff828dd820 RCX: 0000000000000027 > RDX: ffff88803cd9cac8 RSI: 0000000000000001 RDI: ffff88803cd9cac0 > RBP: ffff88800674fbb0 R08: ffffffff828ce248 R09: 00000000ffffefff > R10: ffffffff8285e260 R11: ffffffff828b8eb8 R12: ffff8880046d88d8 > R13: 0000000000000000 R14: 0000000000000000 R15: ffff8880067281c0 > FS: 00007f68601ea740(0000) GS:ffff88803cd80000(0000) knlGS:0000000000000000 > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > CR2: 00005614f3ebc740 CR3: 000000000773a000 CR4: 00000000000006f0 > Call Trace: > <TASK> > ? __warn+0x77/0xd0 > ? lockdep_init_map_type+0x185/0x220 > ? report_bug+0x189/0x1a0 > ? handle_bug+0x3c/0x70 > ? exc_invalid_op+0x18/0x70 > ? asm_exc_invalid_op+0x1a/0x20 > ? lockdep_init_map_type+0x185/0x220 > __kernfs_create_file+0x79/0x100 > cgroup_addrm_files+0x163/0x380 > ? find_held_lock+0x2b/0x80 > ? find_held_lock+0x2b/0x80 > ? find_held_lock+0x2b/0x80 > css_populate_dir+0x73/0x180 > cgroup_apply_control_enable+0x12f/0x3a0 > cgroup_subtree_control_write+0x30b/0x440 > kernfs_fop_write_iter+0x13a/0x1f0 > vfs_write+0x341/0x450 > ksys_write+0x64/0xe0 > do_syscall_64+0x4b/0x110 > entry_SYSCALL_64_after_hwframe+0x76/0x7e > RIP: 0033:0x7f68602d9833 > Code: 8b 15 61 26 0e 00 f7 d8 64 89 02 48 c7 c0 ff ff ff ff eb b7 0f 1f 00 64 8b 04 25 18 00 00 00 85 c0 75 14 b8 01 00 00 00 08 > RSP: 002b:00007fff9bbdf8e8 EFLAGS: 00000246 ORIG_RAX: 0000000000000001 > RAX: ffffffffffffffda RBX: 0000000000000009 RCX: 00007f68602d9833 > RDX: 0000000000000009 RSI: 00005614f3ebc740 RDI: 0000000000000001 > RBP: 00005614f3ebc740 R08: 000000000000000a R09: 0000000000000008 > R10: 00005614f3db6ba0 R11: 0000000000000246 R12: 0000000000000009 > R13: 00007f68603bd6a0 R14: 0000000000000009 R15: 00007f68603b8880 > > For lockdep, there is a sanity check in lockdep_init_map_type(), the > lock-class key must either have been allocated statically or must > have been registered as a dynamic key. However the commit e18df2889ff9 > ("mm/hugetlb_cgroup: prepare cftypes based on template") has changed > the cftypes from static allocated objects to dynamic allocated objects, > so the cft->lockdep_key must be registered proactively. > > Fixes: e18df2889ff9 ("mm/hugetlb_cgroup: prepare cftypes based on template") > Reported-by: kernel test robot <oliver.sang@intel.com> > Closes: https://lore.kernel.org/oe-lkp/202406181046.8d8b2492-oliver.sang@intel.com > Signed-off-by: Xiu Jianfeng <xiujianfeng@huawei.com> Tested-by: Marek Szyprowski <m.szyprowski@samsung.com> > --- > v2: add bug log to commit message > --- > mm/hugetlb_cgroup.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/mm/hugetlb_cgroup.c b/mm/hugetlb_cgroup.c > index 2b899c4ae968..4ff238ba1250 100644 > --- a/mm/hugetlb_cgroup.c > +++ b/mm/hugetlb_cgroup.c > @@ -836,6 +836,8 @@ hugetlb_cgroup_cfttypes_init(struct hstate *h, struct cftype *cft, > cft->file_offset = MEMFILE_OFFSET0(offset) + > MEMFILE_FIELD_SIZE(offset) * idx; > } > + > + lockdep_register_key(&cft->lockdep_key); > } > } > Best regards
Hi Xiu, On Tue, 18 Jun 2024 07:19:22 +0000 Xiu Jianfeng <xiujianfeng@huawei.com> wrote: > When CONFIG_DEBUG_LOCK_ALLOC is enabled, the following commands can > trigger a bug, > > mount -t cgroup2 none /sys/fs/cgroup > cd /sys/fs/cgroup > echo "+hugetlb" > cgroup.subtree_control [...] > diff --git a/mm/hugetlb_cgroup.c b/mm/hugetlb_cgroup.c > index 2b899c4ae968..4ff238ba1250 100644 > --- a/mm/hugetlb_cgroup.c > +++ b/mm/hugetlb_cgroup.c > @@ -836,6 +836,8 @@ hugetlb_cgroup_cfttypes_init(struct hstate *h, struct cftype *cft, > cft->file_offset = MEMFILE_OFFSET0(offset) + > MEMFILE_FIELD_SIZE(offset) * idx; > } > + > + lockdep_register_key(&cft->lockdep_key); > } > } I found the latest mm-unstable tree fails build as below, and 'git bisect' points this patch. linux/mm/hugetlb_cgroup.c: In function ‘hugetlb_cgroup_cfttypes_init’: linux/mm/hugetlb_cgroup.c:840:42: error: ‘struct cftype’ has no member named ‘lockdep_key’ 840 | lockdep_register_key(&cft->lockdep_key); | ^~ Maybe we should take care of CONFIG_DEBUG_LOCK_ALLOC undefined case, like below? diff --git a/mm/hugetlb_cgroup.c b/mm/hugetlb_cgroup.c index a45065698419..9747c2e64e95 100644 --- a/mm/hugetlb_cgroup.c +++ b/mm/hugetlb_cgroup.c @@ -837,7 +837,9 @@ hugetlb_cgroup_cfttypes_init(struct hstate *h, struct cftype *cft, MEMFILE_FIELD_SIZE(offset) * idx; } +#ifdef CONFIG_DEBUG_LOCK_ALLOC lockdep_register_key(&cft->lockdep_key); +#endif } } [...] Thanks, SJ
On 2024/6/19 7:36, SeongJae Park wrote: > Hi Xiu, > > > On Tue, 18 Jun 2024 07:19:22 +0000 Xiu Jianfeng <xiujianfeng@huawei.com> wrote: > >> When CONFIG_DEBUG_LOCK_ALLOC is enabled, the following commands can >> trigger a bug, >> >> mount -t cgroup2 none /sys/fs/cgroup >> cd /sys/fs/cgroup >> echo "+hugetlb" > cgroup.subtree_control > > [...] >> diff --git a/mm/hugetlb_cgroup.c b/mm/hugetlb_cgroup.c >> index 2b899c4ae968..4ff238ba1250 100644 >> --- a/mm/hugetlb_cgroup.c >> +++ b/mm/hugetlb_cgroup.c >> @@ -836,6 +836,8 @@ hugetlb_cgroup_cfttypes_init(struct hstate *h, struct cftype *cft, >> cft->file_offset = MEMFILE_OFFSET0(offset) + >> MEMFILE_FIELD_SIZE(offset) * idx; >> } >> + >> + lockdep_register_key(&cft->lockdep_key); >> } >> } > > I found the latest mm-unstable tree fails build as below, and 'git bisect' > points this patch. > > linux/mm/hugetlb_cgroup.c: In function ‘hugetlb_cgroup_cfttypes_init’: > linux/mm/hugetlb_cgroup.c:840:42: error: ‘struct cftype’ has no member named ‘lockdep_key’ > 840 | lockdep_register_key(&cft->lockdep_key); > | ^~ > > Maybe we should take care of CONFIG_DEBUG_LOCK_ALLOC undefined case, like > below? > > > diff --git a/mm/hugetlb_cgroup.c b/mm/hugetlb_cgroup.c > index a45065698419..9747c2e64e95 100644 > --- a/mm/hugetlb_cgroup.c > +++ b/mm/hugetlb_cgroup.c > @@ -837,7 +837,9 @@ hugetlb_cgroup_cfttypes_init(struct hstate *h, struct cftype *cft, > MEMFILE_FIELD_SIZE(offset) * idx; > } > > +#ifdef CONFIG_DEBUG_LOCK_ALLOC > lockdep_register_key(&cft->lockdep_key); > +#endif > } > } Hi SeongJae, Thanks for you review. I think it's better to remove the #ifdef CONFIG_DEBUG_LOCK_ALLOC from the struct cftype which guards the cft->lockdep_key, because when CONFIG_DEBUG_LOCK_ALLOC is not defined, struct lock_class_key is an empty definition which takes no space and can be unconditionally used within the structure, this may make the code more clean. To Andrew, Would you please drop the v2 and pick the v3? thanks. > > [...] > > > Thanks, > SJ
diff --git a/mm/hugetlb_cgroup.c b/mm/hugetlb_cgroup.c index 2b899c4ae968..4ff238ba1250 100644 --- a/mm/hugetlb_cgroup.c +++ b/mm/hugetlb_cgroup.c @@ -836,6 +836,8 @@ hugetlb_cgroup_cfttypes_init(struct hstate *h, struct cftype *cft, cft->file_offset = MEMFILE_OFFSET0(offset) + MEMFILE_FIELD_SIZE(offset) * idx; } + + lockdep_register_key(&cft->lockdep_key); } }
When CONFIG_DEBUG_LOCK_ALLOC is enabled, the following commands can trigger a bug, mount -t cgroup2 none /sys/fs/cgroup cd /sys/fs/cgroup echo "+hugetlb" > cgroup.subtree_control The log is as below: BUG: key ffff8880046d88d8 has not been registered! ------------[ cut here ]------------ DEBUG_LOCKS_WARN_ON(1) WARNING: CPU: 3 PID: 226 at kernel/locking/lockdep.c:4945 lockdep_init_map_type+0x185/0x220 Modules linked in: CPU: 3 PID: 226 Comm: bash Not tainted 6.10.0-rc4-next-20240617-g76db4c64526c #544 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.15.0-1 04/01/2014 RIP: 0010:lockdep_init_map_type+0x185/0x220 Code: 00 85 c0 0f 84 6c ff ff ff 8b 3d 6a d1 85 01 85 ff 0f 85 5e ff ff ff 48 c7 c6 21 99 4a 82 48 c7 c7 60 29 49 82 e8 3b 2e f5 RSP: 0018:ffffc9000083fc30 EFLAGS: 00000282 RAX: 0000000000000000 RBX: ffffffff828dd820 RCX: 0000000000000027 RDX: ffff88803cd9cac8 RSI: 0000000000000001 RDI: ffff88803cd9cac0 RBP: ffff88800674fbb0 R08: ffffffff828ce248 R09: 00000000ffffefff R10: ffffffff8285e260 R11: ffffffff828b8eb8 R12: ffff8880046d88d8 R13: 0000000000000000 R14: 0000000000000000 R15: ffff8880067281c0 FS: 00007f68601ea740(0000) GS:ffff88803cd80000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 00005614f3ebc740 CR3: 000000000773a000 CR4: 00000000000006f0 Call Trace: <TASK> ? __warn+0x77/0xd0 ? lockdep_init_map_type+0x185/0x220 ? report_bug+0x189/0x1a0 ? handle_bug+0x3c/0x70 ? exc_invalid_op+0x18/0x70 ? asm_exc_invalid_op+0x1a/0x20 ? lockdep_init_map_type+0x185/0x220 __kernfs_create_file+0x79/0x100 cgroup_addrm_files+0x163/0x380 ? find_held_lock+0x2b/0x80 ? find_held_lock+0x2b/0x80 ? find_held_lock+0x2b/0x80 css_populate_dir+0x73/0x180 cgroup_apply_control_enable+0x12f/0x3a0 cgroup_subtree_control_write+0x30b/0x440 kernfs_fop_write_iter+0x13a/0x1f0 vfs_write+0x341/0x450 ksys_write+0x64/0xe0 do_syscall_64+0x4b/0x110 entry_SYSCALL_64_after_hwframe+0x76/0x7e RIP: 0033:0x7f68602d9833 Code: 8b 15 61 26 0e 00 f7 d8 64 89 02 48 c7 c0 ff ff ff ff eb b7 0f 1f 00 64 8b 04 25 18 00 00 00 85 c0 75 14 b8 01 00 00 00 08 RSP: 002b:00007fff9bbdf8e8 EFLAGS: 00000246 ORIG_RAX: 0000000000000001 RAX: ffffffffffffffda RBX: 0000000000000009 RCX: 00007f68602d9833 RDX: 0000000000000009 RSI: 00005614f3ebc740 RDI: 0000000000000001 RBP: 00005614f3ebc740 R08: 000000000000000a R09: 0000000000000008 R10: 00005614f3db6ba0 R11: 0000000000000246 R12: 0000000000000009 R13: 00007f68603bd6a0 R14: 0000000000000009 R15: 00007f68603b8880 For lockdep, there is a sanity check in lockdep_init_map_type(), the lock-class key must either have been allocated statically or must have been registered as a dynamic key. However the commit e18df2889ff9 ("mm/hugetlb_cgroup: prepare cftypes based on template") has changed the cftypes from static allocated objects to dynamic allocated objects, so the cft->lockdep_key must be registered proactively. Fixes: e18df2889ff9 ("mm/hugetlb_cgroup: prepare cftypes based on template") Reported-by: kernel test robot <oliver.sang@intel.com> Closes: https://lore.kernel.org/oe-lkp/202406181046.8d8b2492-oliver.sang@intel.com Signed-off-by: Xiu Jianfeng <xiujianfeng@huawei.com> --- v2: add bug log to commit message --- mm/hugetlb_cgroup.c | 2 ++ 1 file changed, 2 insertions(+)