Re: [PATCH 1/1] cgroup: make per-cgroup pressure stall tracking configurable
From: Suren Baghdasaryan <surenb@google.com>
Date: 2021-05-14 18:20:33
Also in:
cgroups, lkml
On Fri, May 14, 2021 at 10:52 AM Peter Zijlstra [off-list ref] wrote:
quoted hunk ↗ jump to hunk
On Fri, May 14, 2021 at 08:54:47AM -0700, Suren Baghdasaryan wrote:quoted
Correct, for this function CONFIG_CGROUPS=n and cgroup_disable=pressure are treated the same. True, from the code it's not very obvious. Do you have some refactoring in mind that would make it more explicit?Does this make sense?--- a/kernel/sched/psi.c +++ b/kernel/sched/psi.c@@ -744,24 +744,26 @@ static void psi_group_change(struct psi_ static struct psi_group *iterate_groups(struct task_struct *task, void **iter) { + if (cgroup_psi_enabled()) { #ifdef CONFIG_CGROUPS - struct cgroup *cgroup = NULL; + struct cgroup *cgroup = NULL; - if (!*iter) - cgroup = task->cgroups->dfl_cgrp; - else if (*iter == &psi_system) - return NULL; - else - cgroup = cgroup_parent(*iter); + if (!*iter) + cgroup = task->cgroups->dfl_cgrp; + else if (*iter == &psi_system) + return NULL; + else + cgroup = cgroup_parent(*iter); - if (cgroup && cgroup_parent(cgroup)) { - *iter = cgroup; - return cgroup_psi(cgroup); - } -#else - if (*iter) - return NULL; + if (cgroup && cgroup_parent(cgroup)) { + *iter = cgroup; + return cgroup_psi(cgroup); + } #endif + } else { + if (*iter) + return NULL; + } *iter = &psi_system; return &psi_system; }
Hmm. Looks like the case when cgroup_psi_enabled()==true and
CONFIG_CGROUPS=n would miss the "if (*iter) return NULL;" condition.
Effectively with CONFIG_CGROUPS=n this becomes:
if (cgroup_psi_enabled()) { <== assume this is true
#ifdef CONFIG_CGROUPS <== compiled out
#endif
} else {
if (*iter) <== this
statement will never execute
return NULL;
}
*iter = &psi_system;
return &psi_system;
-- To unsubscribe from this group and stop receiving emails from it, send an email to kernel-team+unsubscribe@android.com.