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]

 



Hi,

On 2024/6/18 20:55, Marek Szyprowski wrote:
> 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.

Yes, the root cause is that the cft->lockdep_key has not been registered
proactively when the CONFIG_DEBUG_LOCK_ALLOC is enabled.

Would you please try the following patch?

https://lore.kernel.org/all/20240618071922.2127289-1-xiujianfeng@xxxxxxxxxx/

> 
> 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




[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