runtime: Enable file based backend by ganeshmaharaj · Pull Request #1657 · kata-containers/runtime

ganeshmaharaj · 2019-05-10T05:04:09Z

A file based memory backend mapped to the host, fot eg: '/dev/shm' will
be used by virtio-fs for performance reasons. This change is a generic
implementation of that for kata. This will be enabled default for
virtio-fs negating the need to enable hugepages in that scenario. This
option can be used without virtio-fs by setting 'file_mem_backend' to
the location in the configuration file. Default value is an empty
string.

Fixes: #1656
Signed-off-by: Ganesh Maharaj Mahalingam ganesh.mahalingam@intel.com

ganeshmaharaj · 2019-05-10T05:05:21Z

/cc @stefanha @egernst @mcastelino @sboeuf

sboeuf

Looks good thanks @ganeshmaharaj

sboeuf · 2019-05-10T06:16:43Z

/test

virtcontainers/qemu.go

jodh-intel · 2019-05-10T07:56:50Z

cli/config/configuration-qemu.toml.in

 # result in memory pre allocation
 #enable_hugepages = true

+# Enable file based guest memory support. The default is an empty string which


This is making the qemu config file deviate further from the firecracker one. Is this change qemu specific? If so, please could you mention that in the commit to make it clear.

Related: discussions on breaking up the config file into fragments - #1648

Good point. Let me see if this at all will work with firecracker and then comment back here.

@jodh-intel verified with @mcastelino that this feature will not work on firecracker. I have since removed the config from the firecracker hypervisor invocation and cleaned up the code. Will do a follow-on document update regarding this and hugepages.

Thanks @ganeshmaharaj !

stefanha · 2019-05-10T09:29:21Z

virtcontainers/qemu.go

 	return incoming
 }

+func (q *qemu) setupFileBackedMem(knobs *govmmQemu.Knobs, memory *govmmQemu.Memory) {


There are now 3 users:

VM templating, see setupTemplate()

virtio-fs

Explicit file_mem_backend in configuration.toml

Additionally, all other vhost-user devices also require this setting if hugepages is disabled. Perhaps it should be enabled automatically for all vhost-user devices, not just virtio-fs?

When VM templating is enabled together with virtio-fs the MemoryPath is ignored in favor of FileBackedMemRootDir. Did you check that VM templating still works in this case?

@ganeshmaharaj We also have a gap around memory hotplug. Our memory hotplug model should also switch over to file_mem_backend for all hotplugged dimms also when this is flipped. All the vhost-user backends may or may not tolerate this, but we should ensure we do not end up with a mix of file backed and no file backed DIMMs.

@ganeshmaharaj just to be clear, when you flip this flag every DIMM should be file backed. (This also applies to huge pages). So this will be a good time to fix hotplug handling overall. It can be in a separate PR as this feature is experimental in this release. But before we move it out of experimental we need to make the change as kubernetes will break as it relies heavily on memory hotplug support today.

/cc @dagrh

@mcastelino It is a common use case for vm templating to mix file backed and non-file backed DIMMs. When we unmix them, vm templating's initial memory path should be excluded since it is always file backed.

@bergwolf fair enough. However vhost-user backends will break if we do not make them all file based. So when vhost-user is used, we should default all DIMMs to file based. When it is not in use we should allow the mixing.

However it may just be easier to go one way or the other. (If the overhead is marginal)

@mcastelino Once vm template's initial memory path is excluded, I'm all for it to never mix the two.

Have fixed the choices on multiple scenarios. VM Templating gets the high priority off all. If VM templating is not enabled, the user's filebacking location will be used and fallback is /dev/shm. @stefanha I was not able to get VM templating to work with virtio-fs. That is now next in the list of things to tackle once this patch lands. @mcastelino @bergwolf so will memory hotplug and removing the mixing of memory backends.

@bergwolf @ganeshmaharaj in summary if VM templating is enabled then virtio-fs cannot be used. Actually any vhost-user implementation cannot be used.

If I understand right this is due to the fact that the file backing the templated RAM is setup with share=off. Is that a correct summary. So this is independent of hotplug right?

Can we add this as a known limitation to the documentation?

@ganeshmaharaj @mcastelino I think it is a problem due to kata/govmm implementation. vm template is using its own dimm, and virtio-fs is using its own dimm. Their share property is independent to each other. I don't see why we cannot make them work together.

renzhengeek · 2019-05-17T11:54:40Z

Tested here, it'd solved my problem, so looking for this been merged ASAP :)

ganeshmaharaj · 2019-05-22T05:58:02Z

/test

nitkon · 2019-05-22T11:42:53Z

/test-power

A file based memory backend mapped to the host, fot eg: '/dev/shm' will be used by virtio-fs for performance reasons. This change is a generic implementation of that for kata. This will be enabled default for virtio-fs negating the need to enable hugepages in that scenario. This option can be used without virtio-fs by setting 'file_mem_backend' to the location in the configuration file. Default value is an empty string. Fixes: kata-containers#1656 Signed-off-by: Ganesh Maharaj Mahalingam <ganesh.mahalingam@intel.com>

ganeshmaharaj · 2019-05-24T04:04:28Z

/test

codecov · 2019-05-24T04:55:42Z

Codecov Report

Merging #1657 into master will decrease coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #1657      +/-   ##
==========================================
- Coverage   54.57%   54.56%   -0.01%     
==========================================
  Files         106      106              
  Lines       12765    12792      +27     
==========================================
+ Hits         6966     6980      +14     
- Misses       4966     4975       +9     
- Partials      833      837       +4

raravena80 · 2019-05-24T16:17:09Z

@ganeshmaharaj any updates? Seems it's approved but a test failing.

ganeshmaharaj · 2019-05-24T17:31:52Z

@raravena80 i believe the fedora failure is a known one that folks have been debugging now to figure out the rootcause. There are multiple bugs/PRs towards this. kata-containers/tests#1449, kata-containers/tests#1531

ganeshmaharaj · 2019-05-28T17:31:31Z

i believe this patch is ready to be merged into the tree if there are no objections.

cc @kata-containers/runtime

ganeshmaharaj added the wip label May 10, 2019

sboeuf approved these changes May 10, 2019

View reviewed changes

jodh-intel reviewed May 10, 2019

View reviewed changes

stefanha reviewed May 10, 2019

View reviewed changes

mcastelino mentioned this pull request May 15, 2019

Backport 1.7 #1685

Merged

ganeshmaharaj force-pushed the filebackend branch 2 times, most recently from 9ef20dd to 42a17ff Compare May 21, 2019 15:58

ganeshmaharaj force-pushed the filebackend branch 2 times, most recently from 281a4aa to 9a7fd4d Compare May 24, 2019 03:36

ganeshmaharaj force-pushed the filebackend branch from 9a7fd4d to a41894d Compare May 24, 2019 03:47

ganeshmaharaj removed the wip label May 24, 2019

jodh-intel approved these changes May 24, 2019

View reviewed changes

devimc approved these changes May 28, 2019

View reviewed changes

devimc merged commit 919615f into kata-containers:master May 28, 2019

ganeshmaharaj deleted the filebackend branch May 28, 2019 19:29

Conversation

ganeshmaharaj commented May 10, 2019

Uh oh!

ganeshmaharaj commented May 10, 2019

Uh oh!

sboeuf left a comment

Choose a reason for hiding this comment

Uh oh!

sboeuf commented May 10, 2019

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

renzhengeek commented May 17, 2019

Uh oh!

ganeshmaharaj commented May 22, 2019

Uh oh!

nitkon commented May 22, 2019

Uh oh!

ganeshmaharaj commented May 24, 2019

Uh oh!

codecov bot commented May 24, 2019

Codecov Report

Uh oh!

raravena80 commented May 24, 2019

Uh oh!

ganeshmaharaj commented May 24, 2019

Uh oh!

ganeshmaharaj commented May 28, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

13 participants