Re: [libvirt] [PATCH 4/4] LXC: add cpuset cgroup support for lxc

On 2013/03/15 17:24, Osier Yang wrote: > On 2013年03月01日 14:52, Gao feng wrote: >> This patch adds cpuset cgroup support for LXC. >> also set cpuset cgroup before setting cpu >> affinity and numa policy. > > Any special reason to move lxcSetupCgroup before the CPU affinity > and NUMA setttings? > Through it has no functional difference which step is done first. But consider sched_setaffinity will be affected by cpuset cgroup,

I think its better to set cpuset cgroup before we set CPU affinity. I will add this reason into changelog. >> >> Signed-off-by: Gao feng&lt;gaofeng(a)cn.fujitsu.com&gt; >> --- >> src/lxc/lxc_cgroup.c | 57 +++++++++++++++++++++++++++++++++++++++++++++++- >> src/lxc/lxc_cgroup.h | 2 +- >> src/lxc/lxc_controller.c | 6 ++--- >> 3 files changed, 60 insertions(+), 5 deletions(-) >> >> diff --git a/src/lxc/lxc_cgroup.c b/src/lxc/lxc_cgroup.c >> index a075335..f94b914 100644 >> --- a/src/lxc/lxc_cgroup.c >> +++ b/src/lxc/lxc_cgroup.c >> @@ -68,6 +68,58 @@ cleanup: >> } >> >> >> +static int virLXCCgroupSetupCpusetTune(virDomainDefPtr def, >> + virCgroupPtr cgroup, >> + virBitmapPtr nodemask) >> +{ >> + int rc = 0; >> + char *mask = NULL; >> + >> + if (def->placement_mode != VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO&& >> + def->cpumask) { >> + mask = virBitmapFormat(def->cpumask); >> + if (!mask) { >> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", >> + _("failed to convert cpumask")); >> + return -1; >> + } >> + >> + rc = virCgroupSetCpusetCpus(cgroup, mask); >> + VIR_FREE(mask); >> + if (rc< 0) { >> + virReportSystemError(-rc, "%s", >> + _("Unable to set cpuset.cpus")); >> + } >> + } >> + >> + if ((def->numatune.memory.nodemask || >> + (def->numatune.memory.placement_mode == >> + VIR_DOMAIN_NUMATUNE_MEM_PLACEMENT_MODE_AUTO))&& >> + def->numatune.memory.mode == VIR_DOMAIN_NUMATUNE_MEM_STRICT) { >> + if (def->numatune.memory.placement_mode == >> + VIR_DOMAIN_NUMATUNE_MEM_PLACEMENT_MODE_AUTO) >> + mask = virBitmapFormat(nodemask); >> + else >> + mask = virBitmapFormat(def->numatune.memory.nodemask); >> + >> + if (!mask) { >> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", >> + _("failed to convert memory nodemask")); >> + return -1; >> + } >> + >> + rc = virCgroupSetCpusetMems(cgroup, mask); >> + VIR_FREE(mask); >> + if (rc< 0) { >> + virReportSystemError(-rc, "%s", >> + _("Unable to set cpuset.mems")); >> + } >> + } >> + >> + return rc; >> +} >> + >> + >> static int virLXCCgroupSetupBlkioTune(virDomainDefPtr def, >> virCgroupPtr cgroup) >> { >> @@ -472,7 +524,7 @@ cleanup: >> } >> >> >> -int virLXCCgroupSetup(virDomainDefPtr def) >> +int virLXCCgroupSetup(virDomainDefPtr def, virBitmapPtr nodemask) >> { >> virCgroupPtr driver = NULL; >> virCgroupPtr cgroup = NULL; >> @@ -497,6 +549,9 @@ int virLXCCgroupSetup(virDomainDefPtr def) >> if (virLXCCgroupSetupCpuTune(def, cgroup)< 0) >> goto cleanup; >> >> + if (virLXCCgroupSetupCpusetTune(def, cgroup, nodemask)< 0) >> + goto cleanup; >> + >> if (virLXCCgroupSetupBlkioTune(def, cgroup)< 0) >> goto cleanup; >> >> diff --git a/src/lxc/lxc_cgroup.h b/src/lxc/lxc_cgroup.h >> index fff554b..29f21d6 100644 >> --- a/src/lxc/lxc_cgroup.h >> +++ b/src/lxc/lxc_cgroup.h >> @@ -26,7 +26,7 @@ >> # include "lxc_fuse.h" >> # include "virusb.h" >> >> -int virLXCCgroupSetup(virDomainDefPtr def); >> +int virLXCCgroupSetup(virDomainDefPtr def, virBitmapPtr nodemask); >> int virLXCCgroupGetMeminfo(virLXCMeminfoPtr meminfo); >> >> int >> diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c >> index 3db0a88..75e2fe4 100644 >> --- a/src/lxc/lxc_controller.c >> +++ b/src/lxc/lxc_controller.c >> @@ -505,15 +505,15 @@ static int virLXCControllerSetupResourceLimits(virLXCControllerPtr ctrl) >> if (ret< 0) >> goto cleanup; >> >> - ret = virLXCControllerSetupCpuAffinity(ctrl); >> + ret = virLXCCgroupSetup(ctrl->def, nodemask); >> if (ret< 0) >> goto cleanup; >> >> - ret = virSetupNumaMemoryPolicy(ctrl->def->numatune, nodemask); >> + ret = virLXCControllerSetupCpuAffinity(ctrl); >> if (ret< 0) >> goto cleanup; >> >> - ret = virLXCCgroupSetup(ctrl->def); >> + ret = virSetupNumaMemoryPolicy(ctrl->def->numatune, nodemask); >> if (ret< 0) >> goto cleanup; >> > > Looks good& ACK if there is a reasonble response on the question, but > this needs to be rebased for comments in 1/4. > Yes,of course, thanks for your comments again.