Re: [libvirt] [PATCH 4/9] util: cgroups do not implicitly add task to new machine cgroup
On Fri, 26 Feb 2016 13:26:36 +0000
"Daniel P. Berrange" <berrange(a)redhat.com> wrote:
On Fri, Feb 26, 2016 at 02:17:35PM +0100, Henning Schild wrote:
> On Fri, 26 Feb 2016 13:00:04 +0000
> "Daniel P. Berrange" <berrange(a)redhat.com> wrote:
>
> > On Fri, Feb 26, 2016 at 01:43:05PM +0100, Henning Schild wrote:
> > IIUC, the original problem you wanted to address was that vCPU
> > pids could run the wrong CPU for a short time. ie The original
> > code did
> >
> > 1. Libvirtd forks child pid
> > ... this pid runs on whatever pCPUs that libvirt is
> > permitted to use... 2. Libvirtd creates root cgroup for VM
> > ... this pid runs on whatever pCPUs the root cgroup
> > inherited... 3. Child pid execs QEMU
> > ... QEMU pid runs on whatever pCPUs the root cgroup
> > inherited... 4. QEMU creates vCPU pids
> > ... vCPU pids run on whatever pCPUs the root cgroup
> > inherited... 4. Libvirtd moves emulator PIDs and vCPU PIDs
> > ... emulator PIDs are running on assigned pCPUs for
> > emulator... ... vCPU PIDs are running on assigned pCPUs for
> > vCPUs....
> >
> > With the important change in patch 5 this now looks like
> >
> > 1. Libvirtd forks child pid
> > ... this pid runs on whatever pCPUs that libvirt is
> > permitted to use... 2. Libvirtd creates root cgroup for VM
>
> > ... this pid runs on whatever pCPUs the root cgroup
> > inherited...
>
> I am trying to come up with a solution that eliminates the above
> line from the whole bringup. I.e never allow a pid belonging to the
> VM outside the pinnings of libvirtd and the VM configuration.
That's imposible because you can't stop systemd placing the pid leader
> But until step 4 it should probably be
> "... this pid *just sits* on whatever pCPUs the root cgroup
> inherited..."
> If we are sure that it does not "run" before 4. patch 5 does the
> trick already
Yes the pid *runs* - it has to run in order to do the setup tasks
before exec'ing QEMU. Indeed even invoking 'execve()' syscall
requires that it run.
I am saying it should not run while in the parent cgroup. so
between steps 2 and 3. If we can not stop the pid from getting into the
parent cgroup we have to rely on it not causing disturbance by
"running". Otherwise the whole series is not a solution to the
disturbance problem, it is just a mitigation.
That beeing said i think it is still good enough and we should stop
that discussion here. I will send a v2 series.
> > 3. Libvirtd moves pid into emulator group
> > ... this pid runs on assigned pCPUs for emulator...
from now on it can run all it wants, because it is in the corrent cpuset
> > 4. Child pid execs QEMU
> > ... QEMU pid runs on assigned pCPUs for emulator...
> > 5. QEMU creates vCPU pids
> > ... vCPU pids are running on assigned pCPUS for emulator...
> > 6. Libvirtd moves vCPU PIDs
> > ... emulator PIDs are running on assigned pCPUs for
> > emulator... ... vCPU PIDs are running on assigned pCPUs for
> > vCPUs....
> >
> > Which is good, because vCPU pids don't ever run on un-restricted
> > pCPUs.
> >
> >
> > Your patch 4 here is attempting to change step 2 only so that it
> > looks like
> >
> >
> > 1. Libvirtd forks child pid
> > ... this pid runs on whatever pCPUs that libvirt is
> > permitted to use... 2. Libvirtd creates root cgroup for VM
> > ... this pid runs on whatever pCPUs that libvirt is
> > permitted to use... or depending on what controller system added
> > ... this pid runs on whatever pCPUs the root cgroup
> > inherited... 3. Libvirtd adds pid into emulator group
> > ... this pid runs on assigned pCPUs for emulator...
> > 4. Child pid execs QEMU
> > ... QEMU pid runs on assigned pCPUs for emulator...
> > 5. QEMU creates vCPU pids
> > ... vCPU pids are running on assigned pCPUS for emulator...
> > 6. Libvirtd moves vCPU PIDs
> > ... emulator PIDs are running on assigned pCPUs for
> > emulator... ... vCPU PIDs are running on assigned pCPUs for
> > vCPUs....
> >
> > At the time we exec QEMU in step 4 the situation is exactly the
> > same as before. The vCPU pids are still created in the right place
> > straight away.
> >
> > So this patch 4 doesn't achieve anything useful
> >
> > Regards,
> > Daniel
>
Regards,
Daniel