1. Patchew
  2. linux
  3. View series

[PATCH v2 -next] mm/hugetlb_cgroup: register lockdep key for cftype

Xiu Jianfeng posted 1 patch 1 year, 6 months ago
There is a newer version of this series
mm/hugetlb_cgroup.c | 2 ++
1 file changed, 2 insertions(+)
[PATCH v2 -next] mm/hugetlb_cgroup: register lockdep key for cftype
Posted by Xiu Jianfeng 1 year, 6 months ago 
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(+)

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);
 	}
 }
 
-- 
2.34.1
Re: [PATCH v2 -next] mm/hugetlb_cgroup: register lockdep key for cftype
Posted by SeongJae Park 1 year, 6 months ago 
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
Re: [PATCH v2 -next] mm/hugetlb_cgroup: register lockdep key for cftype
Posted by xiujianfeng 1 year, 6 months ago 

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
Re: [PATCH v2 -next] mm/hugetlb_cgroup: register lockdep key for cftype
Posted by Marek Szyprowski 1 year, 6 months ago 
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
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.