On 2024/6/19 7:36, SeongJae Park wrote: > Hi Xiu, > > > On Tue, 18 Jun 2024 07:19:22 +0000 Xiu Jianfeng <xiujianfeng@xxxxxxxxxx> 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
