[Devel] [PATCH vz10 02/14] mm: fix VZ build errors with CONFIG_MEMCG=n
Konstantin Khorenko
khorenko at virtuozzo.com
Fri Aug 21 17:03:29 MSK 2026
On 6/26/26 00:08, Eva Kurchatova wrote:
> Guard VZ-specific code that uses memory cgroup internals
> (memory_cgrp_id, struct mem_cgroup, root_mem_cgroup) with
> CONFIG_MEMCG.
>
> Without CONFIG_MEMCG:
> - oom_berserker() and oom_kill_memcg_member() dereference struct
> mem_cgroup fields that do not exist.
> - si_meminfo_ve(), fill_meminfo_ve(), and fill_vmstat_ve() use
> memory_cgrp_id and mem_cgroup_from_css() which are undeclared.
> - proc_oom_score() VE-specific path references memory_cgrp_id.
>
> Wrap affected functions and their call sites with CONFIG_MEMCG (or
> CONFIG_VE && CONFIG_MEMCG where appropriate) and provide stubs where
> needed. Also guard VZ-specific nid field accesses in memcontrol.c
> with CONFIG_MEMCG_V1 and move the numa_migrate cftypes entry inside
> the existing CONFIG_NUMA block.
>
> Signed-off-by: Eva Kurchatova <eva.kurchatova at virtuozzo.com>
>
> https://virtuozzo.atlassian.net/browse/VSTOR-134732
> Feature: fix kunit
> ---
> fs/proc/base.c | 4 ++++
> fs/proc/meminfo.c | 4 ++++
> mm/memcontrol.c | 9 +++++++--
> mm/oom_kill.c | 8 ++++++++
> mm/show_mem.c | 2 ++
> mm/vmstat.c | 7 +++++--
> 6 files changed, 30 insertions(+), 4 deletions(-)
>
> diff --git a/fs/proc/base.c b/fs/proc/base.c
> index 8174b2a8a5dc..db7d304f1905 100644
> --- a/fs/proc/base.c
> +++ b/fs/proc/base.c
> @@ -692,6 +692,7 @@ static int proc_oom_score(struct seq_file *m, struct pid_namespace *ns,
> unsigned long points = 0;
> long badness;
>
> +#ifdef CONFIG_MEMCG
> scoped_guard (rcu) {
> struct cgroup_subsys_state *css = task_css(task, memory_cgrp_id);
>
> @@ -702,6 +703,9 @@ static int proc_oom_score(struct seq_file *m, struct pid_namespace *ns,
> memcg = mem_cgroup_from_css(css);
> totalpages = mem_cgroup_get_max(memcg);
> }
> +#else
> + scoped_guard (rcu) {
> +#endif
> }
Empty block with scoped_guard ?
>
> badness = oom_badness(task, totalpages, NULL);
> diff --git a/fs/proc/meminfo.c b/fs/proc/meminfo.c
> index ac3c88e68728..4db69eed0e21 100644
> --- a/fs/proc/meminfo.c
> +++ b/fs/proc/meminfo.c
> @@ -73,6 +73,7 @@ static int meminfo_proc_show_mi(struct seq_file *m, struct meminfo *mi)
> return 0;
> }
>
> +#if defined(CONFIG_VE) && defined(CONFIG_MEMCG)
> static void fill_meminfo_ve(struct meminfo *mi, struct ve_struct *ve)
> {
> struct cgroup_subsys_state *css;
> @@ -84,6 +85,7 @@ static void fill_meminfo_ve(struct meminfo *mi, struct ve_struct *ve)
> css_put(css);
>
> }
> +#endif
>
> static int meminfo_proc_show_ve(struct seq_file *m, void *v,
> struct ve_struct *ve)
> @@ -104,11 +106,13 @@ static int meminfo_proc_show_ve(struct seq_file *m, void *v,
> mi.si = &i;
> mi.ve = ve;
>
> +#if defined(CONFIG_VE) && defined(CONFIG_MEMCG)
> if (!ve_is_super(ve) && ve->meminfo_val == VE_MEMINFO_DEFAULT) {
> fill_meminfo_ve(&mi, ve);
>
> return meminfo_proc_show_mi(m, &mi);
> }
> +#endif
>
> committed = vm_memory_committed();
>
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 6301319529ee..c27ee90e599c 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -714,8 +714,11 @@ percpu_stats_memcg(struct mem_cgroup *memcg, struct mem_cgroup_per_node **pn)
> } while (memcg->percpu_stats_disabled);
>
> if (pn) {
> +#ifdef CONFIG_MEMCG_V1
> unsigned int nid = (*pn)->nid;
> -
> +#else
> + unsigned int nid = lruvec_pgdat(&(*pn)->lruvec)->node_id;
> +#endif
Three objections to deriving the node id from the lruvec instead of keeping the field:
1) It reads a lazily initialized field. lruvec_pgdat(&(*pn)->lruvec) reads pn->lruvec.pgdat, but lruvec_init() memsets the
whole structure and never sets pgdat - it is filled in on the first lookup, by mem_cgroup_lruvec():
if (unlikely(lruvec->pgdat != pgdat))
lruvec->pgdat = pgdat;
For the only caller that passes a non-NULL pn today - __mod_memcg_lruvec_state(), which always gets its lruvec from
mem_cgroup_lruvec() or folio_lruvec() - the pointer is already set by then, so as it stands this will not oops. But it
makes a correctness-critical read depend on initialization performed elsewhere: add a second caller whose lruvec comes
from another source and you get NULL->node_id, with nothing in the compiler or in the code to warn about it.
2) It costs more on a hot path. __mod_memcg_lruvec_state() is one of the hottest paths in memcg accounting, and this turns
a single unsigned short field read into two pointer dereferences (pn->lruvec.pgdat, then ->node_id) for no gain.
3) It makes the two configurations behave differently by construction. With v1 the node id comes from a field, without v1
it is derived from the pgdat. In statistics accounting code that is the last thing you want: the configurations diverge by
design rather than by accident.
So I went the other way instead - declare nid in the CONFIG_MEMCG_V1=n branch of struct mem_cgroup_per_node as well:
#ifdef CONFIG_MEMCG_V1
...
RH_KABI_FILL_HOLE(unsigned short nid)
#else
+ /*
+ * Not a v1 field: percpu_stats_memcg() needs the node id to find
+ * the matching per-node structure of the parent memcg.
+ */
+ unsigned short nid;
CACHELINE_PADDING(_pad1_);
#endif
It goes before CACHELINE_PADDING(_pad1_) on purpose: nid is written once at allocation and only read afterwards, so it
belongs on the read-mostly side of the false-sharing boundary that the padding exists to maintain.
With this, both the reader in percpu_stats_memcg() and the writer in alloc_mem_cgroup_per_node_info() stay unconditional
and no .c file has to change. The v1 branch is untouched, so the layout of the shipped kernel - and its kABI checksum -
does not change either: RH_KABI_FILL_HOLE() expands to nothing under __GENKSYMS__, and genksyms only ever sees the
configuration it is run with, which has MEMCG_V1=y.
Note that MEMCG=y with MEMCG_V1=n is not an exotic combination, by the way: MEMCG_V1 is default n upstream, so any plain
modern config built from this tree hits it.
> *pn = memcg->nodeinfo[nid];
> }
> return memcg;
> @@ -4179,7 +4182,9 @@ static bool alloc_mem_cgroup_per_node_info(struct mem_cgroup *memcg, int node)
>
> lruvec_init(&pn->lruvec);
> pn->memcg = memcg;
> +#ifdef CONFIG_MEMCG_V1
> pn->nid = node;
> +#endif
>
> memcg->nodeinfo[node] = pn;
> return true;
> @@ -5258,12 +5263,12 @@ static struct cftype memory_files[] = {
> .name = "numa_stat",
> .seq_show = memory_numa_stat_show,
> },
> -#endif
> {
> .name = "numa_migrate",
> .flags = CFTYPE_NOT_ON_ROOT,
> .write = memcg_numa_migrate_write,
> },
> +#endif
> {
> .name = "oom.group",
> .flags = CFTYPE_NOT_ON_ROOT | CFTYPE_NS_DELEGATABLE,
> diff --git a/mm/oom_kill.c b/mm/oom_kill.c
> index 1fd5f99ce3b9..b26a15072d06 100644
> --- a/mm/oom_kill.c
> +++ b/mm/oom_kill.c
> @@ -1034,6 +1034,7 @@ static void __oom_kill_process(struct task_struct *victim, const char *message)
> * Kill provided task unless it's secured by setting
> * oom_score_adj to OOM_SCORE_ADJ_MIN.
> */
> +#ifdef CONFIG_MEMCG
> static int oom_kill_memcg_member(struct task_struct *task, void *message)
> {
> if (task->signal->oom_score_adj != OOM_SCORE_ADJ_MIN &&
> @@ -1043,10 +1044,12 @@ static int oom_kill_memcg_member(struct task_struct *task, void *message)
> }
> return 0;
> }
> +#endif
>
> /*
> * Kill more processes if oom happens too often in this context.
> */
> +#ifdef CONFIG_MEMCG
> static void oom_berserker(struct oom_control *oc)
> {
> static DEFINE_RATELIMIT_STATE(berserker_rs,
> @@ -1150,6 +1153,9 @@ static void oom_berserker(struct oom_control *oc)
>
> pr_err("OOM killer in rage %d: %d tasks killed\n", rage, killed);
> }
> +#else
> +static inline void oom_berserker(struct oom_control *oc) { }
> +#endif /* CONFIG_MEMCG */
>
> atomic_t global_oom = ATOMIC_INIT(0);
>
> @@ -1196,6 +1202,7 @@ static void oom_kill_process(struct oom_control *oc, const char *message)
> /*
> * If necessary, kill all tasks in the selected memory cgroup.
> */
> +#ifdef CONFIG_MEMCG
> if (oom_group) {
> memcg_memory_event(oom_group, MEMCG_OOM_GROUP_KILL);
> mem_cgroup_print_oom_group(oom_group);
> @@ -1203,6 +1210,7 @@ static void oom_kill_process(struct oom_control *oc, const char *message)
> (void *)message);
> mem_cgroup_put(oom_group);
> }
> +#endif
> oom_berserker(oc);
> }
>
> diff --git a/mm/show_mem.c b/mm/show_mem.c
> index 3ab11c945bf4..4c879177a531 100644
> --- a/mm/show_mem.c
> +++ b/mm/show_mem.c
> @@ -91,6 +91,7 @@ void si_meminfo(struct sysinfo *val)
>
> EXPORT_SYMBOL(si_meminfo);
>
> +#if defined(CONFIG_VE) && defined(CONFIG_MEMCG)
> void si_meminfo_ve(struct sysinfo *si, struct ve_struct *ve)
> {
> unsigned long memtotal, memused, swaptotal, swapused;
> @@ -138,6 +139,7 @@ void si_meminfo_ve(struct sysinfo *si, struct ve_struct *ve)
> /* bufferram, totalhigh and freehigh left 0 */
> }
> EXPORT_SYMBOL(si_meminfo_ve);
> +#endif
>
> #ifdef CONFIG_NUMA
> void si_meminfo_node(struct sysinfo *val, int nid)
> diff --git a/mm/vmstat.c b/mm/vmstat.c
> index 47cb6bf4ecec..de4f8b4e51b6 100644
> --- a/mm/vmstat.c
> +++ b/mm/vmstat.c
> @@ -1855,6 +1855,7 @@ static const struct seq_operations zoneinfo_op = {
> (IS_ENABLED(CONFIG_VM_EVENT_COUNTERS) ? \
> NR_VM_EVENT_ITEMS : 0))
>
> +#if defined(CONFIG_VE) && defined(CONFIG_MEMCG)
> static void fill_vmstat_ve(unsigned long *stat, struct ve_struct *ve)
> {
> struct cgroup_subsys_state *css;
> @@ -1863,10 +1864,10 @@ static void fill_vmstat_ve(unsigned long *stat, struct ve_struct *ve)
> mem_cgroup_fill_vmstat(mem_cgroup_from_css(css), stat);
> css_put(css);
> }
> +#endif
>
> static void *vmstat_start(struct seq_file *m, loff_t *pos)
> {
> - struct ve_struct *ve;
> unsigned long *v;
> int i;
>
> @@ -1880,12 +1881,14 @@ static void *vmstat_start(struct seq_file *m, loff_t *pos)
> if (!v)
> return ERR_PTR(-ENOMEM);
>
> - ve = get_exec_env();
> +#if defined(CONFIG_VE) && defined(CONFIG_MEMCG)
> + struct ve_struct *ve = get_exec_env();
> if (!ve_is_super(ve)) {
> memset(v, 0, NR_VMSTAT_ITEMS * sizeof(unsigned long));
> fill_vmstat_ve(v, ve);
> return (unsigned long *)m->private + *pos;
> }
> +#endif
>
> for (i = 0; i < NR_VM_ZONE_STAT_ITEMS; i++)
> v[i] = global_zone_page_state(i);
More information about the Devel
mailing list