From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 9CDC8C27C55 for ; Tue, 11 Jun 2024 02:29:38 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 2BEB16B008C; Mon, 10 Jun 2024 22:29:38 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 21F556B0092; Mon, 10 Jun 2024 22:29:38 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 099B16B0093; Mon, 10 Jun 2024 22:29:38 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id DC8646B008C for ; Mon, 10 Jun 2024 22:29:37 -0400 (EDT) Received: from smtpin02.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 9456FA1F72 for ; Tue, 11 Jun 2024 02:29:37 +0000 (UTC) X-FDA: 82217026794.02.A610713 Received: from szxga02-in.huawei.com (szxga02-in.huawei.com [45.249.212.188]) by imf22.hostedemail.com (Postfix) with ESMTP id D6171C001B for ; Tue, 11 Jun 2024 02:29:34 +0000 (UTC) Authentication-Results: imf22.hostedemail.com; dkim=none; dmarc=pass (policy=quarantine) header.from=huawei.com; spf=pass (imf22.hostedemail.com: domain of xiujianfeng@huawei.com designates 45.249.212.188 as permitted sender) smtp.mailfrom=xiujianfeng@huawei.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1718072975; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=J0aattryK43GSsMSF51jpHf3DOYe98P2o/SBBAZSMlo=; b=jSSqL2Fkjb5wDLKst7GWOz79nC3cdLOtbbaCDFwqQqoODdqvm7xKJz8jqSelw3nU7XTzzF 5H/3ZNfIfMf5+dYU7GDm2RqDr/h7Xp7btSFDm9VyNYJieIhBcKeAXl2i/nPZsJAeHOKKyd gksvyZBNEJq97QVO7z4ICaYwqUaqaC4= ARC-Authentication-Results: i=1; imf22.hostedemail.com; dkim=none; dmarc=pass (policy=quarantine) header.from=huawei.com; spf=pass (imf22.hostedemail.com: domain of xiujianfeng@huawei.com designates 45.249.212.188 as permitted sender) smtp.mailfrom=xiujianfeng@huawei.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1718072975; a=rsa-sha256; cv=none; b=Ig+K79+PmJklWtItgv7NNYvoq+Rd5mtCrZ/C7Tkf8N7eZQ7eiUNq6tY+k6OqWlGFOBQ7pl LJvQdMhXX9DvTwd5Ws/RnwP/WEnPv5L0MgwuIxOozKPBcN7QKbUsNiNlFYOF/umRKoAy7u JArHxQwM5sF4v5AsXf71pZeKrDEDba0= Received: from mail.maildlp.com (unknown [172.19.163.48]) by szxga02-in.huawei.com (SkyGuard) with ESMTP id 4VysvP5Rx8zmZ06; Tue, 11 Jun 2024 10:24:49 +0800 (CST) Received: from dggpeml500023.china.huawei.com (unknown [7.185.36.114]) by mail.maildlp.com (Postfix) with ESMTPS id 89E6118007C; Tue, 11 Jun 2024 10:29:31 +0800 (CST) Received: from [10.67.110.112] (10.67.110.112) by dggpeml500023.china.huawei.com (7.185.36.114) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2507.39; Tue, 11 Jun 2024 10:29:29 +0800 Message-ID: Date: Tue, 11 Jun 2024 10:29:29 +0800 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.5.1 Subject: Re: [PATCH v2 -next 2/3] mm/hugetlb_cgroup: prepare cftypes based on template Content-Language: en-US From: xiujianfeng To: , , Oscar Salvador CC: , References: <20240605070133.1941677-1-xiujianfeng@huawei.com> <20240605070133.1941677-3-xiujianfeng@huawei.com> In-Reply-To: <20240605070133.1941677-3-xiujianfeng@huawei.com> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit X-Originating-IP: [10.67.110.112] X-ClientProxiedBy: dggems706-chm.china.huawei.com (10.3.19.183) To dggpeml500023.china.huawei.com (7.185.36.114) X-Rspamd-Server: rspam07 X-Rspamd-Queue-Id: D6171C001B X-Stat-Signature: xp37wxcrqwqfo4i7pepp793hjq1qwqp7 X-Rspam-User: X-HE-Tag: 1718072974-957589 X-HE-Meta: U2FsdGVkX1+KB8aRXS4dhURmuq+JqxrGqO11hbkDGqbaOrTlQikB8VlA1iBs5lJzhgUCMQUwVOw+qRRMayUb5PE9T0u0c7Gn0RQHlZu5BhRDwx0aM1glN3pxHpsTULTswpKHuze7wgG9fZgBl03ftYNyKRmF6FXg+Ofq7xxuo7YGR6E6TWlbeSMd9TYkEYY14ZuyhmNOTyXL7R76S0fVM5di4hNQ0Z9jduq4dl4kI6jfxgxrY2kAG9rUjrGIIYhvhH34gyrKr1YGbUm3vSAJa7CA6iLbRzl+JSeeHHRJKZ4IX8edW8laPwo9Lv93UaVyT8ObQqST2uUiwYntJahkyn0NsBk/x749hJ4J+QprFs1Hq/ALVCBeb+3j+VxNOxj9GOQbJaT/iZUqzIxrdgNICMI9Rx6ej/JT6CFwcdr6YTgxqgTdekb1B1NeMPJk3zD3ibqY/qeMSjDoLgeccSf20P39K8BLxxeINmwXNL0XEsbKx2MWKnkP0af5DIZf/R9xgRLCB8y9smX3TlH+Q0nMLneJAhbcuPkasuvtxHI0P4MBNkFPltIbuvoLn7PZDnBmoC6ZgQKH589dYHEPdgDXdAQe7PCGJ4f2iTcOEYHHIUIbDSz0A4pd9yic0dWvoFUaiLTEInwM5PMdIxPNvs20AGzEkkXSlfAQPjLM0dQFtVSScjnAnaNBXITZiBkC9TJQRWjcnRZ2abGyCAU9N/3UKyN+kPyvrdjWARWmktixFzInjMH7dV+Sg70PKa8tqY7JsajrATTaeAyZDUZH4BSWUlDWtqhoMcHJRuMJ2E8DjDYPnyhoYrufwdX0u7rT7W5pCxMhnXDflZEWNFaf00dWZg3T4oF3Elgg9I6DPPdrqli0XKyDONREBf3P0NqsMWOkIWCng20R3nMBBbVCnh4DWoyyiq/1K4xRhkOpNn6MSh/nY4fi/CbTDSCPJjTlH1feLsZ3v3pyrQ4k038ARE2 6v5FS6is YV1qVnyyXI5IEET8O6I4fQE15p7kTXQQIflQDpM29Z4DqN6IIObt+8BwnFQYE4v/O4+WwCpGqdfyCSGTLegGvOH0dwuStSsNDehoUftH9hYs2hKlgETFfLPrPBR16RUTZKdXkintJFT6KzQDMdkesmEhLqjDQqxtO/pNVoccORSmz5s0+K7TVwb+NvtfJFZztFkYxknBY6jgBs9E= X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On 2024/6/5 15:01, Xiu Jianfeng wrote: > Unlike other cgroup subsystems, the hugetlb cgroup does not provide > a static array of cftype that explicitly displays the properties, > handling functions, etc., of each file. Instead, it dynamically creates > the attribute of cftypes based on the hstate during the startup > procedure. This reduces the readability of the code. > > To fix this issue, introduce two templates of cftypes, and rebuild the > attributes according to the hstate to make it ready to be added to > cgroup framework. > > Signed-off-by: Xiu Jianfeng > --- > mm/hugetlb_cgroup.c | 155 ++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 155 insertions(+) > > diff --git a/mm/hugetlb_cgroup.c b/mm/hugetlb_cgroup.c > index 45f94a869776..56876b61027d 100644 > --- a/mm/hugetlb_cgroup.c > +++ b/mm/hugetlb_cgroup.c > @@ -27,7 +27,16 @@ > #define MEMFILE_IDX(val) (((val) >> 16) & 0xffff) > #define MEMFILE_ATTR(val) ((val) & 0xffff) > > +#define MEMFILE_OFFSET(t, m) (((offsetof(t, m) << 16) | sizeof_field(t, m))) > +#define MEMFILE_OFFSET0(val) (((val) >> 16) & 0xffff) > +#define MEMFILE_FIELD_SIZE(val) ((val) & 0xffff) > + > +#define DFL_TMPL_SIZE ARRAY_SIZE(hugetlb_dfl_tmpl) > +#define LEGACY_TMPL_SIZE ARRAY_SIZE(hugetlb_legacy_tmpl) > + > static struct hugetlb_cgroup *root_h_cgroup __read_mostly; > +static struct cftype *dfl_files; > +static struct cftype *legacy_files; > > static inline struct page_counter * > __hugetlb_cgroup_counter_from_cgroup(struct hugetlb_cgroup *h_cg, int idx, > @@ -702,12 +711,142 @@ static int hugetlb_events_local_show(struct seq_file *seq, void *v) > return __hugetlb_events_show(seq, true); > } > > +static struct cftype hugetlb_dfl_tmpl[] = { > + { > + .name = "max", > + .private = RES_LIMIT, > + .seq_show = hugetlb_cgroup_read_u64_max, > + .write = hugetlb_cgroup_write_dfl, > + .flags = CFTYPE_NOT_ON_ROOT, > + }, > + { > + .name = "rsvd.max", > + .private = RES_RSVD_LIMIT, > + .seq_show = hugetlb_cgroup_read_u64_max, > + .write = hugetlb_cgroup_write_dfl, > + .flags = CFTYPE_NOT_ON_ROOT, > + }, > + { > + .name = "current", > + .private = RES_USAGE, > + .seq_show = hugetlb_cgroup_read_u64_max, > + .flags = CFTYPE_NOT_ON_ROOT, > + }, > + { > + .name = "rsvd.current", > + .private = RES_RSVD_USAGE, > + .seq_show = hugetlb_cgroup_read_u64_max, > + .flags = CFTYPE_NOT_ON_ROOT, > + }, > + { > + .name = "events", > + .seq_show = hugetlb_events_show, > + .file_offset = MEMFILE_OFFSET(struct hugetlb_cgroup, events_file[0]), > + .flags = CFTYPE_NOT_ON_ROOT, > + }, > + { > + .name = "events.local", > + .seq_show = hugetlb_events_local_show, > + .file_offset = MEMFILE_OFFSET(struct hugetlb_cgroup, events_local_file[0]), > + .flags = CFTYPE_NOT_ON_ROOT, > + }, > + { > + .name = "numa_stat", > + .seq_show = hugetlb_cgroup_read_numa_stat, > + .flags = CFTYPE_NOT_ON_ROOT, > + }, > + /* don't need terminator here */ > +}; > + > +static struct cftype hugetlb_legacy_tmpl[] = { > + { > + .name = "limit_in_bytes", > + .private = RES_LIMIT, > + .read_u64 = hugetlb_cgroup_read_u64, > + .write = hugetlb_cgroup_write_legacy, > + }, > + { > + .name = "rsvd.limit_in_bytes", > + .private = RES_RSVD_LIMIT, > + .read_u64 = hugetlb_cgroup_read_u64, > + .write = hugetlb_cgroup_write_legacy, > + }, > + { > + .name = "usage_in_bytes", > + .private = RES_USAGE, > + .read_u64 = hugetlb_cgroup_read_u64, > + }, > + { > + .name = "rsvd.usage_in_bytes", > + .private = RES_RSVD_USAGE, > + .read_u64 = hugetlb_cgroup_read_u64, > + }, > + { > + .name = "max_usage_in_bytes", > + .private = RES_MAX_USAGE, > + .write = hugetlb_cgroup_reset, > + .read_u64 = hugetlb_cgroup_read_u64, > + }, > + { > + .name = "rsvd.max_usage_in_bytes", > + .private = RES_RSVD_MAX_USAGE, > + .write = hugetlb_cgroup_reset, > + .read_u64 = hugetlb_cgroup_read_u64, > + }, > + { > + .name = "failcnt", > + .private = RES_FAILCNT, > + .write = hugetlb_cgroup_reset, > + .read_u64 = hugetlb_cgroup_read_u64, > + }, > + { > + .name = "rsvd.failcnt", > + .private = RES_RSVD_FAILCNT, > + .write = hugetlb_cgroup_reset, > + .read_u64 = hugetlb_cgroup_read_u64, > + }, > + { > + .name = "numa_stat", > + .seq_show = hugetlb_cgroup_read_numa_stat, > + }, > + /* don't need terminator here */ > +}; > + > +static void __init > +hugetlb_cgroup_cfttypes_init(struct hstate *h, struct cftype *cft, > + struct cftype *tmpl, int tmpl_size) > +{ > + char buf[32]; > + int i, idx = hstate_index(h); > + > + /* format the size */ > + mem_fmt(buf, sizeof(buf), huge_page_size(h)); > + > + for (i = 0; i < tmpl_size; cft++, tmpl++, i++) { > + *cft = *tmpl; > + /* rebuild the name */ > + snprintf(cft->name, MAX_CFTYPE_NAME, "%s.%s", buf, tmpl->name); > + /* rebuild the private */ > + if (tmpl->private) > + cft->private = MEMFILE_PRIVATE(idx, tmpl->private); I found there is a problem here, the if (tmpl->private) statment should be dropped, and the cft->private should be reassigned unconditionally according to the idx of the hstate, because the .private in the templates only represents the second argument of the MEMFILE_PRIVATE(). But cft->file_offset is a bit different; it does not need to be assigned unless it is required, its usage is as follows: int cgroup_add_file(...) { ... if (cft->file_offset) { struct cgroup_file *cfile = (void *)css + cft->file_offset; ... } ... } I will fix it in the next version. > + /* rebuild the file_offset */ > + if (tmpl->file_offset) { > + unsigned int offset = tmpl->file_offset; > + cft->file_offset = MEMFILE_OFFSET0(offset) + > + MEMFILE_FIELD_SIZE(offset) * idx; > + } > + } > +} > + > static void __init __hugetlb_cgroup_file_dfl_init(int idx) > { > char buf[32]; > struct cftype *cft; > struct hstate *h = &hstates[idx]; > > + 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)); > > @@ -779,6 +918,9 @@ static void __init __hugetlb_cgroup_file_legacy_init(int idx) > struct cftype *cft; > struct hstate *h = &hstates[idx]; > > + 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)); > > @@ -856,10 +998,23 @@ static void __init __hugetlb_cgroup_file_init(int idx) > __hugetlb_cgroup_file_legacy_init(idx); > } > > +static void __init __hugetlb_cgroup_file_pre_init(void) > +{ > + int cft_count; > + > + cft_count = hugetlb_max_hstate * DFL_TMPL_SIZE + 1; /* add terminator */ > + dfl_files = kcalloc(cft_count, sizeof(struct cftype), GFP_KERNEL); > + BUG_ON(!dfl_files); > + cft_count = hugetlb_max_hstate * LEGACY_TMPL_SIZE + 1; /* add terminator */ > + legacy_files = kcalloc(cft_count, sizeof(struct cftype), GFP_KERNEL); > + BUG_ON(!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)); > }