Dear All,
On 12.06.2024 11:24, Xiu Jianfeng wrote:
> The previous patch has already reconstructed the cftype attributes
> based on the templates and saved them in dfl_cftypes and legacy_cftypes.
> then remove the old procedure and switch to the new cftypes.
>
> Signed-off-by: Xiu Jianfeng <xiujianfeng@xxxxxxxxxx>
This patch landed in yesterday's linux-next (next-20240617) as commit
11308a02a56cc ("mm/hugetlb_cgroup: switch to the new cftypes"). In my
daily tests I found that it triggers the following lock dependency
checker warning on most of my ARM64 test machines:
BUG: key ffff000005c080d8 has not been registered!
------------[ cut here ]------------
DEBUG_LOCKS_WARN_ON(1)
WARNING: CPU: 1 PID: 1080 at kernel/locking/lockdep.c:4895
lockdep_init_map_type+0x1cc/0x284
Modules linked in: ipv6
CPU: 1 PID: 1080 Comm: cgmanager Not tainted 6.10.0-rc3+ #1011
Hardware name: linux,dummy-virt (DT)
pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
pc : lockdep_init_map_type+0x1cc/0x284
lr : lockdep_init_map_type+0x1cc/0x284
...
Call trace:
lockdep_init_map_type+0x1cc/0x284
__kernfs_create_file+0x7c/0x138
cgroup_addrm_files+0x170/0x360
css_populate_dir+0x70/0x174
cgroup_apply_control_enable+0x128/0x378
rebind_subsystems+0x384/0x504
cgroup_setup_root+0x224/0x420
cgroup1_get_tree+0x248/0x36c
vfs_get_tree+0x28/0xe8
path_mount+0x3e8/0xb78
__arm64_sys_mount+0x1f0/0x2dc
invoke_syscall+0x48/0x118
el0_svc_common.constprop.0+0x40/0xe8
do_el0_svc_compat+0x20/0x3c
el0_svc_compat+0x44/0xe0
el0t_32_sync_handler+0x98/0x148
el0t_32_sync+0x194/0x198
irq event stamp: 8225
hardirqs last enabled at (8225): [<ffff800080136134>]
console_unlock+0x164/0x190
hardirqs last disabled at (8224): [<ffff800080136120>]
console_unlock+0x150/0x190
softirqs last enabled at (8214): [<ffff8000800ae1f4>]
handle_softirqs+0x4dc/0x4f4
softirqs last disabled at (8207): [<ffff8000800105d4>]
__do_softirq+0x14/0x20
---[ end trace 0000000000000000 ]---
It looks that something is not properly intialized. Reverting $subject
on top of linux-next fixes this issue.
I've used defconfig with some debug options enabled and some drivers
compiled-in (if this matters):
make ARCH=arm64 defconfig
./scripts/config -e BLK_DEV_RAM --set-val BLK_DEV_RAM_COUNT 4 --set-val
BLK_DEV_RAM_SIZE 81920 --set-val CMA_SIZE_MBYTES 96 -e PROVE_LOCKING -e
DEBUG_ATOMIC_SLEEP -e STAGING -e I2C_GPIO -e PM_DEBUG -e
PM_ADVANCED_DEBUG -e USB_GADGET -e USB_ETH -e CONFIG_DEVFREQ_THERMAL -e
CONFIG_BRCMFMAC_PCIE -e CONFIG_NFC -d ARCH_SUNXI -d ARCH_ALPINE -d
DRM_NOUVEAU -d ARCH_BCM_IPROC -d ARCH_BERLIN -d ARCH_BRCMSTB -d
ARCH_LAYERSCAPE -d ARCH_LG1K -d ARCH_HISI -d ARCH_MEDIATEK -d ARCH_MVEBU
-d ARCH_SEATTLE -d ARCH_SYNQUACER -d ARCH_RENESAS -d ARCH_STRATIX10 -d
ARCH_TEGRA -d ARCH_SPRD -d ARCH_THUNDER -d ARCH_THUNDER2 -d
ARCH_UNIPHIER -d ARCH_XGENE -d ARCH_ZX -d ARCH_ZYNQMP -d HIBERNATION -d
CLK_SUNXI -d CONFIG_EFI -d CONFIG_TEE -e FW_CFG_SYSFS
> ---
> include/linux/hugetlb.h | 5 --
> mm/hugetlb_cgroup.c | 163 +++++-----------------------------------
> 2 files changed, 17 insertions(+), 151 deletions(-)
>
> diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
> index 279aca379b95..a951c0d06061 100644
> --- a/include/linux/hugetlb.h
> +++ b/include/linux/hugetlb.h
> @@ -686,11 +686,6 @@ struct hstate {
> unsigned int nr_huge_pages_node[MAX_NUMNODES];
> unsigned int free_huge_pages_node[MAX_NUMNODES];
> unsigned int surplus_huge_pages_node[MAX_NUMNODES];
> -#ifdef CONFIG_CGROUP_HUGETLB
> - /* cgroup control files */
> - struct cftype cgroup_files_dfl[8];
> - struct cftype cgroup_files_legacy[10];
> -#endif
> char name[HSTATE_NAME_LEN];
> };
>
> diff --git a/mm/hugetlb_cgroup.c b/mm/hugetlb_cgroup.c
> index 43aae8f88d5f..2b899c4ae968 100644
> --- a/mm/hugetlb_cgroup.c
> +++ b/mm/hugetlb_cgroup.c
> @@ -839,164 +839,26 @@ hugetlb_cgroup_cfttypes_init(struct hstate *h, struct cftype *cft,
> }
> }
>
> -static void __init __hugetlb_cgroup_file_dfl_init(int idx)
> +static void __init __hugetlb_cgroup_file_dfl_init(struct hstate *h)
> {
> - char buf[32];
> - struct cftype *cft;
> - struct hstate *h = &hstates[idx];
> + int idx = hstate_index(h);
>
> hugetlb_cgroup_cfttypes_init(h, dfl_files + idx * DFL_TMPL_SIZE,
> hugetlb_dfl_tmpl, DFL_TMPL_SIZE);
> -
> - /* format the size */
> - mem_fmt(buf, sizeof(buf), huge_page_size(h));
> -
> - /* Add the limit file */
> - cft = &h->cgroup_files_dfl[0];
> - snprintf(cft->name, MAX_CFTYPE_NAME, "%s.max", buf);
> - cft->private = MEMFILE_PRIVATE(idx, RES_LIMIT);
> - cft->seq_show = hugetlb_cgroup_read_u64_max;
> - cft->write = hugetlb_cgroup_write_dfl;
> - cft->flags = CFTYPE_NOT_ON_ROOT;
> -
> - /* Add the reservation limit file */
> - cft = &h->cgroup_files_dfl[1];
> - snprintf(cft->name, MAX_CFTYPE_NAME, "%s.rsvd.max", buf);
> - cft->private = MEMFILE_PRIVATE(idx, RES_RSVD_LIMIT);
> - cft->seq_show = hugetlb_cgroup_read_u64_max;
> - cft->write = hugetlb_cgroup_write_dfl;
> - cft->flags = CFTYPE_NOT_ON_ROOT;
> -
> - /* Add the current usage file */
> - cft = &h->cgroup_files_dfl[2];
> - snprintf(cft->name, MAX_CFTYPE_NAME, "%s.current", buf);
> - cft->private = MEMFILE_PRIVATE(idx, RES_USAGE);
> - cft->seq_show = hugetlb_cgroup_read_u64_max;
> - cft->flags = CFTYPE_NOT_ON_ROOT;
> -
> - /* Add the current reservation usage file */
> - cft = &h->cgroup_files_dfl[3];
> - snprintf(cft->name, MAX_CFTYPE_NAME, "%s.rsvd.current", buf);
> - cft->private = MEMFILE_PRIVATE(idx, RES_RSVD_USAGE);
> - cft->seq_show = hugetlb_cgroup_read_u64_max;
> - cft->flags = CFTYPE_NOT_ON_ROOT;
> -
> - /* Add the events file */
> - cft = &h->cgroup_files_dfl[4];
> - snprintf(cft->name, MAX_CFTYPE_NAME, "%s.events", buf);
> - cft->private = MEMFILE_PRIVATE(idx, 0);
> - cft->seq_show = hugetlb_events_show;
> - cft->file_offset = offsetof(struct hugetlb_cgroup, events_file[idx]);
> - cft->flags = CFTYPE_NOT_ON_ROOT;
> -
> - /* Add the events.local file */
> - cft = &h->cgroup_files_dfl[5];
> - snprintf(cft->name, MAX_CFTYPE_NAME, "%s.events.local", buf);
> - cft->private = MEMFILE_PRIVATE(idx, 0);
> - cft->seq_show = hugetlb_events_local_show;
> - cft->file_offset = offsetof(struct hugetlb_cgroup,
> - events_local_file[idx]);
> - cft->flags = CFTYPE_NOT_ON_ROOT;
> -
> - /* Add the numa stat file */
> - cft = &h->cgroup_files_dfl[6];
> - snprintf(cft->name, MAX_CFTYPE_NAME, "%s.numa_stat", buf);
> - cft->private = MEMFILE_PRIVATE(idx, 0);
> - cft->seq_show = hugetlb_cgroup_read_numa_stat;
> - cft->flags = CFTYPE_NOT_ON_ROOT;
> -
> - /* NULL terminate the last cft */
> - cft = &h->cgroup_files_dfl[7];
> - memset(cft, 0, sizeof(*cft));
> -
> - WARN_ON(cgroup_add_dfl_cftypes(&hugetlb_cgrp_subsys,
> - h->cgroup_files_dfl));
> }
>
> -static void __init __hugetlb_cgroup_file_legacy_init(int idx)
> +static void __init __hugetlb_cgroup_file_legacy_init(struct hstate *h)
> {
> - char buf[32];
> - struct cftype *cft;
> - struct hstate *h = &hstates[idx];
> + int idx = hstate_index(h);
>
> hugetlb_cgroup_cfttypes_init(h, legacy_files + idx * LEGACY_TMPL_SIZE,
> hugetlb_legacy_tmpl, LEGACY_TMPL_SIZE);
> -
> - /* format the size */
> - mem_fmt(buf, sizeof(buf), huge_page_size(h));
> -
> - /* Add the limit file */
> - cft = &h->cgroup_files_legacy[0];
> - snprintf(cft->name, MAX_CFTYPE_NAME, "%s.limit_in_bytes", buf);
> - cft->private = MEMFILE_PRIVATE(idx, RES_LIMIT);
> - cft->read_u64 = hugetlb_cgroup_read_u64;
> - cft->write = hugetlb_cgroup_write_legacy;
> -
> - /* Add the reservation limit file */
> - cft = &h->cgroup_files_legacy[1];
> - snprintf(cft->name, MAX_CFTYPE_NAME, "%s.rsvd.limit_in_bytes", buf);
> - cft->private = MEMFILE_PRIVATE(idx, RES_RSVD_LIMIT);
> - cft->read_u64 = hugetlb_cgroup_read_u64;
> - cft->write = hugetlb_cgroup_write_legacy;
> -
> - /* Add the usage file */
> - cft = &h->cgroup_files_legacy[2];
> - snprintf(cft->name, MAX_CFTYPE_NAME, "%s.usage_in_bytes", buf);
> - cft->private = MEMFILE_PRIVATE(idx, RES_USAGE);
> - cft->read_u64 = hugetlb_cgroup_read_u64;
> -
> - /* Add the reservation usage file */
> - cft = &h->cgroup_files_legacy[3];
> - snprintf(cft->name, MAX_CFTYPE_NAME, "%s.rsvd.usage_in_bytes", buf);
> - cft->private = MEMFILE_PRIVATE(idx, RES_RSVD_USAGE);
> - cft->read_u64 = hugetlb_cgroup_read_u64;
> -
> - /* Add the MAX usage file */
> - cft = &h->cgroup_files_legacy[4];
> - snprintf(cft->name, MAX_CFTYPE_NAME, "%s.max_usage_in_bytes", buf);
> - cft->private = MEMFILE_PRIVATE(idx, RES_MAX_USAGE);
> - cft->write = hugetlb_cgroup_reset;
> - cft->read_u64 = hugetlb_cgroup_read_u64;
> -
> - /* Add the MAX reservation usage file */
> - cft = &h->cgroup_files_legacy[5];
> - snprintf(cft->name, MAX_CFTYPE_NAME, "%s.rsvd.max_usage_in_bytes", buf);
> - cft->private = MEMFILE_PRIVATE(idx, RES_RSVD_MAX_USAGE);
> - cft->write = hugetlb_cgroup_reset;
> - cft->read_u64 = hugetlb_cgroup_read_u64;
> -
> - /* Add the failcntfile */
> - cft = &h->cgroup_files_legacy[6];
> - snprintf(cft->name, MAX_CFTYPE_NAME, "%s.failcnt", buf);
> - cft->private = MEMFILE_PRIVATE(idx, RES_FAILCNT);
> - cft->write = hugetlb_cgroup_reset;
> - cft->read_u64 = hugetlb_cgroup_read_u64;
> -
> - /* Add the reservation failcntfile */
> - cft = &h->cgroup_files_legacy[7];
> - snprintf(cft->name, MAX_CFTYPE_NAME, "%s.rsvd.failcnt", buf);
> - cft->private = MEMFILE_PRIVATE(idx, RES_RSVD_FAILCNT);
> - cft->write = hugetlb_cgroup_reset;
> - cft->read_u64 = hugetlb_cgroup_read_u64;
> -
> - /* Add the numa stat file */
> - cft = &h->cgroup_files_legacy[8];
> - snprintf(cft->name, MAX_CFTYPE_NAME, "%s.numa_stat", buf);
> - cft->private = MEMFILE_PRIVATE(idx, 0);
> - cft->seq_show = hugetlb_cgroup_read_numa_stat;
> -
> - /* NULL terminate the last cft */
> - cft = &h->cgroup_files_legacy[9];
> - memset(cft, 0, sizeof(*cft));
> -
> - WARN_ON(cgroup_add_legacy_cftypes(&hugetlb_cgrp_subsys,
> - h->cgroup_files_legacy));
> }
>
> -static void __init __hugetlb_cgroup_file_init(int idx)
> +static void __init __hugetlb_cgroup_file_init(struct hstate *h)
> {
> - __hugetlb_cgroup_file_dfl_init(idx);
> - __hugetlb_cgroup_file_legacy_init(idx);
> + __hugetlb_cgroup_file_dfl_init(h);
> + __hugetlb_cgroup_file_legacy_init(h);
> }
>
> static void __init __hugetlb_cgroup_file_pre_init(void)
> @@ -1011,13 +873,22 @@ static void __init __hugetlb_cgroup_file_pre_init(void)
> BUG_ON(!legacy_files);
> }
>
> +static void __init __hugetlb_cgroup_file_post_init(void)
> +{
> + WARN_ON(cgroup_add_dfl_cftypes(&hugetlb_cgrp_subsys,
> + dfl_files));
> + WARN_ON(cgroup_add_legacy_cftypes(&hugetlb_cgrp_subsys,
> + legacy_files));
> +}
> +
> void __init hugetlb_cgroup_file_init(void)
> {
> struct hstate *h;
>
> __hugetlb_cgroup_file_pre_init();
> for_each_hstate(h)
> - __hugetlb_cgroup_file_init(hstate_index(h));
> + __hugetlb_cgroup_file_init(h);
> + __hugetlb_cgroup_file_post_init();
> }
>
> /*
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
