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 <[email protected]>
---
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);
+ /* 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));
}
--
2.34.1
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 <[email protected]>
> ---
> 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));
> }