Thanks for your comments, Daniel. Well, since you have a consensus about /proc/cpuinfo, I will only resubmit the patches related to timer. Julio Cesar Faracco Em ter., 25 de fev. de 2020 às 08:34, Daniel P. Berrangé <berrange@redhat.com> escreveu:
On Mon, Feb 24, 2020 at 11:24:28AM -0300, Julio Faracco wrote:
This commit tries to fix lots of issues related to LXC VCPUs. One of them is related to /proc/cpuinfo content. If only 1 VCPU is set, LXC containers will show all CPUs available for host. The second one is related to CPU share, if an user set only 1 VCPU, the container/process will use all available CPUs. (This is not the case when `cpuset` attribute is declared). So, this commit adds a virtual cpuinfo based on VCPU mapping and it automatically limits the CPU usage according VCPU count.
Example (now): LXC container - 8 CPUS with 2 VCPU: lxc-root# stress --cpu 8
On host machine, only CPU 0 and 1 have 100% usage.
Signed-off-by: Julio Faracco <jcfaracco@gmail.com> --- src/lxc/lxc_cgroup.c | 31 ++++++++++++++ src/lxc/lxc_container.c | 39 ++++++++++------- src/lxc/lxc_fuse.c | 95 ++++++++++++++++++++++++++++++++++++++--- 3 files changed, 145 insertions(+), 20 deletions(-)
diff --git a/src/lxc/lxc_cgroup.c b/src/lxc/lxc_cgroup.c index d29b65092a..912a252473 100644 --- a/src/lxc/lxc_cgroup.c +++ b/src/lxc/lxc_cgroup.c @@ -50,6 +50,34 @@ static int virLXCCgroupSetupCpuTune(virDomainDefPtr def, }
+static int virLXCCgroupSetupVcpuAuto(virDomainDefPtr def, + virCgroupPtr cgroup) +{ + size_t i; + int vcpumax; + virBuffer buffer = VIR_BUFFER_INITIALIZER; + virBufferPtr cpuset = &buffer; + + vcpumax = virDomainDefGetVcpusMax(def); + for (i = 0; i < vcpumax; i++) { + virDomainVcpuDefPtr vcpu = virDomainDefGetVcpu(def, i); + /* Cgroup is smart enough to convert numbers separated + * by comma into ranges. Example: "0,1,2,5," -> "0-2,5". + * Libvirt does not need to process it here. */ + if (vcpu) + virBufferAsprintf(cpuset, "%zu,", i); + }
If I have 4 containers, each with vcpu==2, then all 4 containers are going to be taskset onto host CPUs 0 & 1, and host CPUs 2,3,4,5,6,7 are going to be unused. This is not viable as a strategy.
+ if (virCgroupSetCpusetCpus(cgroup, + virBufferCurrentContent(cpuset)) < 0) { + virBufferFreeAndReset(cpuset); + return -1; + } + + virBufferFreeAndReset(cpuset); + return 0; +}
+static int +lxcProcReadCpuinfoParse(virDomainDefPtr def, char *base, + virBufferPtr new_cpuinfo) +{ + char *procline = NULL; + char *saveptr = base; + size_t cpu; + size_t nvcpu; + size_t curcpu = 0; + bool get_proc = false; + + nvcpu = virDomainDefGetVcpus(def); + while ((procline = strtok_r(NULL, "\n", &saveptr))) { + if (sscanf(procline, "processor\t: %zu", &cpu) == 1) {
This doesn't work because it is assuming the /proc/cpuinfo data format for x86 architecture. Every architecture puts different info in this file, and only some of them using "processor: NN" as a delimiter for new entries. There's many examples in tests/sysinfodata/ and in tests/virhostcpudata/ showing this problem
We had considered taking /proc/cpuinfo in the past, but decided against it. Since this file is non-standard data format varying across arches, well written apps won't actually look at /proc/cpuinfo at all. Instead they'll use /sys/devices/system/cpu instead. I don't think we want to make /proc/cpuinfo be inconsistent with data in sysfs.
+ virDomainVcpuDefPtr vcpu = virDomainDefGetVcpu(def, cpu); + /* VCPU is mapped */ + if (vcpu) { + if (curcpu == nvcpu) + break; + + if (curcpu > 0) + virBufferAddLit(new_cpuinfo, "\n"); + + virBufferAsprintf(new_cpuinfo, "processor\t: %zu\n", + curcpu); + curcpu++; + get_proc = true; + } else { + get_proc = false; + } + } else { + /* It is not a processor index */ + if (get_proc) + virBufferAsprintf(new_cpuinfo, "%s\n", procline); + } + } + + virBufferAddLit(new_cpuinfo, "\n"); + + return strlen(virBufferCurrentContent(new_cpuinfo)); +}
Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|