Re: [PATCH v3 -next 3/3] mm/hugetlb_cgroup: switch to the new cftypes

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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





[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux