Page MenuHomeFreeBSD

Virtualization of basic variables and locks for jail+vps.
Needs ReviewPublic

Authored by bz on May 25 2018, 11:05 AM.

Details

Reviewers
jamie
zec
kib
jhb
Summary
Virtualization of basic variables and locks for jail+vps.

Tried to exclude as many functional changes as possible.

The reason we are virtualising the locks is because eventually
we might want to suspend/resume or even migrate the jails based
on what the VPS work has shown to be doable in the past.

Sponsored by:	iXsystems, Inc.

Diff Detail

Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 17978
Build 17732: arc lint + arc unit

Event Timeline

This comment was removed by bz.

I have no idea about the global plan and most of the implementation details, so I am replying from the common sense PoV. I am completely not sure about the split of the allproc_lock and proctree_lock into per-VPS locks. For often need to iterate over all processes in the tree, e.g. to make a decision about the global memory subsystem state. In this case, the global process list and the global allproc_lock are right, but per-VPS lists and locks are not. Of course, unless you also partition the physical memory (but I doubt that it makes sense).

In other words, there still must be a global lock, and there probably should be per-container locks. But this requires initial architectural discussion and agreement on the major points, which would allow to make such decisions later without much reconsiderations.

In D15570#328824, @kib wrote:

I have no idea about the global plan and most of the implementation details, so I am replying from the common sense PoV. I am completely not sure about the split of the allproc_lock and proctree_lock into per-VPS locks. For often need to iterate over all processes in the tree, e.g. to make a decision about the global memory subsystem state. In this case, the global process list and the global allproc_lock are right, but per-VPS lists and locks are not. Of course, unless you also partition the physical memory (but I doubt that it makes sense).

In other words, there still must be a global lock, and there probably should be per-container locks. But this requires initial architectural discussion and agreement on the major points, which would allow to make such decisions later without much reconsiderations.

Can you give me a good example where we currently do this? The iterations over all processes won't really work anymore given there's no single all-processes list anymore.
I'd like to have a look at a good example (as you mention the memory subsystem maybe there) so I can give you a more informed description for discussion.

In D15570#335888, @bz wrote:
In D15570#328824, @kib wrote:

I have no idea about the global plan and most of the implementation details, so I am replying from the common sense PoV. I am completely not sure about the split of the allproc_lock and proctree_lock into per-VPS locks. For often need to iterate over all processes in the tree, e.g. to make a decision about the global memory subsystem state. In this case, the global process list and the global allproc_lock are right, but per-VPS lists and locks are not. Of course, unless you also partition the physical memory (but I doubt that it makes sense).

In other words, there still must be a global lock, and there probably should be per-container locks. But this requires initial architectural discussion and agreement on the major points, which would allow to make such decisions later without much reconsiderations.

Can you give me a good example where we currently do this? The iterations over all processes won't really work anymore given there's no single all-processes list anymore.
I'd like to have a look at a good example (as you mention the memory subsystem maybe there) so I can give you a more informed description for discussion.

grep for FOREACH_PROC_IN_SYSTEM() for the start. In fact these places are already visible in the patch because they require allproc_lock.

But the more important question still stands. Some of the uses of the global lists should go, but some, esp. for VM, probably must stay. As I noted, this requires preliminary architectural discussion.

In D15570#335980, @kib wrote:
In D15570#335888, @bz wrote:
In D15570#328824, @kib wrote:

grep for FOREACH_PROC_IN_SYSTEM() for the start. In fact these places are already visible in the patch because they require allproc_lock.

Yeah I got that bit. I was more wondering about ..

But the more important question still stands. Some of the uses of the global lists should go, but some, esp. for VM, probably must stay. As I noted, this requires preliminary architectural discussion.

.. exactly one of these esp for VM, where it must stay. Can you just point out one specific example.

In the network stack we have VNET_FOREACH() as an outside iterator over all virtual (network stack) instances and then can iterate over all per-virtual-instance lists. I am wondering if the same could be done here..

In D15570#335994, @bz wrote:
In D15570#335980, @kib wrote:
In D15570#335888, @bz wrote:
In D15570#328824, @kib wrote:

grep for FOREACH_PROC_IN_SYSTEM() for the start. In fact these places are already visible in the patch because they require allproc_lock.

Yeah I got that bit. I was more wondering about ..

But the more important question still stands. Some of the uses of the global lists should go, but some, esp. for VM, probably must stay. As I noted, this requires preliminary architectural discussion.

.. exactly one of these esp for VM, where it must stay. Can you just point out one specific example.

In the network stack we have VNET_FOREACH() as an outside iterator over all virtual (network stack) instances and then can iterate over all per-virtual-instance lists. I am wondering if the same could be done here..

I am almost completely sure that whatever the virtualization code ends up, FOREACH_PROC_IN_SYSTEM() in ini_main.c and imgact_elf.c must be global. Same for sys/ddb.

For sys/vm, the answer is highly depends on the architectural choises. If the VM is not partitioned, then OOM selectors and swapout daemon need to operate on the global list as well. Since partitioning VM is very hard option, I doubt that this is implemented.

In D15570#336006, @kib wrote:

I am almost completely sure that whatever the virtualization code ends up, FOREACH_PROC_IN_SYSTEM() in ini_main.c and imgact_elf.c must be global. Same for sys/ddb.

For sys/vm, the answer is highly depends on the architectural choises. If the VM is not partitioned, then OOM selectors and swapout daemon need to operate on the global list as well. Since partitioning VM is very hard option, I doubt that this is implemented.

See D15906 for a suggested way of changes; these are done similar to vnet; not all the operations are global, but you were right indeed, there's quite a few of them and I fear that going back to VPS I'll find a lot of these cases unhandled there as well.

I am leaving this review to be the mostly whitespace change which with one vps instance should make no difference change and the majority of the functional parts are in the two child changes.

  • Merge branch 'p0' into p1
  • Maintanance on new code; add virtualisation where needed.