From a5bc11f9eb70482daf559019f9b775c285defb63 Mon Sep 17 00:00:00 2001 From: aszlig Date: Mon, 14 Dec 2015 17:26:24 +0100 Subject: [PATCH 1/3] nixos/vm-tests: Remove msize mount option This seems to be the root cause of the random page allocation failures and @wizeman did a very good job on not only finding the root problem but also giving a detailed explanation of it in #10828. Here is an excerpt: The problem here is that the kernel is trying to allocate a contiguous section of 2^7=128 pages, which is 512 KB. This is way too much: kernel pages tend to get fragmented over time and kernel developers often go to great lengths to try allocating at most only 1 contiguous page at a time whenever they can. From the error message, it looks like the culprit is unionfs, but this is misleading: unionfs is the name of the userspace process that was running when the system ran out of memory, but it wasn't unionfs who was allocating the memory: it was the kernel; specifically it was the v9fs_dir_readdir_dotl() function, which is the code for handling the readdir() function in the 9p filesystem (the filesystem that is used to share a directory structure between a qemu host and its VM). If you look at the code, here's what it's doing at the moment it tries to allocate memory: buflen = fid->clnt->msize - P9_IOHDRSZ; rdir = v9fs_alloc_rdir_buf(file, buflen); If you look into v9fs_alloc_rdir_buf(), you will see that it will try to allocate a contiguous buffer of memory (using kzalloc(), which is a wrapper around kmalloc()) of size buflen + 8 bytes or so. So in reality, this code actually allocates a buffer of size proportional to fid->clnt->msize. What is this msize? If you follow the definition of the structures, you will see that it's the negotiated buffer transfer size between 9p client and 9p server. On the client side, it can be controlled with the msize mount option. What this all means is that, the reason for running out of memory is that the code (which we can't easily change) tries to allocate a contiguous buffer of size more or less equal to "negotiated 9p protocol buffer size", which seems to be way too big (in our NixOS tests, at least). After that initial finding, @lethalman tested the gnome3 gdm test without setting the msize parameter at all and it seems to have resolved the problem. The reason why I'm committing this without testing against all of the NixOS VM test is basically that I think we can only go better but not worse than the current state. Signed-off-by: aszlig --- nixos/modules/virtualisation/qemu-vm.nix | 6 +++--- pkgs/build-support/vm/default.nix | 4 ++-- pkgs/build-support/vm/windows/controller/default.nix | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/nixos/modules/virtualisation/qemu-vm.nix b/nixos/modules/virtualisation/qemu-vm.nix index b1f36cf32644..0cefd618075c 100644 --- a/nixos/modules/virtualisation/qemu-vm.nix +++ b/nixos/modules/virtualisation/qemu-vm.nix @@ -426,19 +426,19 @@ in ${if cfg.writableStore then "/nix/.ro-store" else "/nix/store"} = { device = "store"; fsType = "9p"; - options = "trans=virtio,version=9p2000.L,msize=1048576,cache=loose"; + options = "trans=virtio,version=9p2000.L,cache=loose"; neededForBoot = true; }; "/tmp/xchg" = { device = "xchg"; fsType = "9p"; - options = "trans=virtio,version=9p2000.L,msize=1048576,cache=loose"; + options = "trans=virtio,version=9p2000.L,cache=loose"; neededForBoot = true; }; "/tmp/shared" = { device = "shared"; fsType = "9p"; - options = "trans=virtio,version=9p2000.L,msize=1048576"; + options = "trans=virtio,version=9p2000.L"; neededForBoot = true; }; } // optionalAttrs cfg.writableStore diff --git a/pkgs/build-support/vm/default.nix b/pkgs/build-support/vm/default.nix index 250f2b0fbce7..ebcbf8c87b5e 100644 --- a/pkgs/build-support/vm/default.nix +++ b/pkgs/build-support/vm/default.nix @@ -118,7 +118,7 @@ rec { echo "mounting Nix store..." mkdir -p /fs/nix/store - mount -t 9p store /fs/nix/store -o trans=virtio,version=9p2000.L,msize=262144,cache=loose + mount -t 9p store /fs/nix/store -o trans=virtio,version=9p2000.L,cache=loose mkdir -p /fs/tmp /fs/run /fs/var mount -t tmpfs -o "mode=1777" none /fs/tmp @@ -127,7 +127,7 @@ rec { echo "mounting host's temporary directory..." mkdir -p /fs/tmp/xchg - mount -t 9p xchg /fs/tmp/xchg -o trans=virtio,version=9p2000.L,msize=262144,cache=loose + mount -t 9p xchg /fs/tmp/xchg -o trans=virtio,version=9p2000.L,cache=loose mkdir -p /fs/proc mount -t proc none /fs/proc diff --git a/pkgs/build-support/vm/windows/controller/default.nix b/pkgs/build-support/vm/windows/controller/default.nix index 0beaf401758a..1c8e6af83b86 100644 --- a/pkgs/build-support/vm/windows/controller/default.nix +++ b/pkgs/build-support/vm/windows/controller/default.nix @@ -48,11 +48,11 @@ let mount -t proc none /fs/proc mount -t 9p \ - -o trans=virtio,version=9p2000.L,msize=262144,cache=loose \ + -o trans=virtio,version=9p2000.L,cache=loose \ store /fs/nix/store mount -t 9p \ - -o trans=virtio,version=9p2000.L,msize=262144,cache=loose \ + -o trans=virtio,version=9p2000.L,cache=loose \ xchg /fs/xchg echo root:x:0:0::/root:/bin/false > /fs/etc/passwd From 6353f580f90c0fdd2b418fa853a78ec508bda2a5 Mon Sep 17 00:00:00 2001 From: aszlig Date: Mon, 14 Dec 2015 17:36:22 +0100 Subject: [PATCH 2/3] nixos/qemu-vm: Disable cache for $NIX_DISK_IMAGE As @domenkozar noted in #10828, cache=writeback seems to do more harm than good: https://github.com/NixOS/nixpkgs/issues/10828#issuecomment-164426821 He has tested it using the openstack NixOS tests and found that cache=none significantly improves startup performance. Signed-off-by: aszlig --- nixos/modules/virtualisation/qemu-vm.nix | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/nixos/modules/virtualisation/qemu-vm.nix b/nixos/modules/virtualisation/qemu-vm.nix index 0cefd618075c..e90e319a36ba 100644 --- a/nixos/modules/virtualisation/qemu-vm.nix +++ b/nixos/modules/virtualisation/qemu-vm.nix @@ -76,14 +76,14 @@ let -virtfs local,path=$TMPDIR/xchg,security_model=none,mount_tag=xchg \ -virtfs local,path=''${SHARED_DIR:-$TMPDIR/xchg},security_model=none,mount_tag=shared \ ${if cfg.useBootLoader then '' - -drive index=0,id=drive1,file=$NIX_DISK_IMAGE,if=${cfg.qemu.diskInterface},cache=writeback,werror=report \ + -drive index=0,id=drive1,file=$NIX_DISK_IMAGE,if=${cfg.qemu.diskInterface},cache=none,werror=report \ -drive index=1,id=drive2,file=$TMPDIR/disk.img,media=disk \ ${if cfg.useEFIBoot then '' -pflash $TMPDIR/bios.bin \ '' else '' ''} '' else '' - -drive index=0,id=drive1,file=$NIX_DISK_IMAGE,if=${cfg.qemu.diskInterface},cache=writeback,werror=report \ + -drive index=0,id=drive1,file=$NIX_DISK_IMAGE,if=${cfg.qemu.diskInterface},cache=none,werror=report \ -kernel ${config.system.build.toplevel}/kernel \ -initrd ${config.system.build.toplevel}/initrd \ -append "$(cat ${config.system.build.toplevel}/kernel-params) init=${config.system.build.toplevel}/init regInfo=${regInfo} ${kernelConsole} $QEMU_KERNEL_PARAMS" \ From 00934bb90865b6ccd3b226c7d495adbf4dabc9da Mon Sep 17 00:00:00 2001 From: aszlig Date: Mon, 14 Dec 2015 17:39:55 +0100 Subject: [PATCH 3/3] nixos/tests: Revert setting min_free_kbytes This reverts commit 02b568414d509b5d06dbd95bcc0868d487ed359e. With a5bc11f and 6353f58 in place, we really don't need this anymore. After running about 500 VM tests on my Hydra, it still didn't improve very much. Signed-off-by: aszlig --- nixos/modules/testing/test-instrumentation.nix | 4 ---- 1 file changed, 4 deletions(-) diff --git a/nixos/modules/testing/test-instrumentation.nix b/nixos/modules/testing/test-instrumentation.nix index c1dbfe305ddd..f37bbd0246da 100644 --- a/nixos/modules/testing/test-instrumentation.nix +++ b/nixos/modules/testing/test-instrumentation.nix @@ -88,10 +88,6 @@ let kernel = config.boot.kernelPackages.kernel; in boot.consoleLogLevel = 7; - # Make sure we don't hit page allocation failures if the VM's memory is - # heavily fragmented. - boot.kernel.sysctl."vm.min_free_kbytes" = 16384; - # Prevent tests from accessing the Internet. networking.defaultGateway = mkOverride 150 ""; networking.nameservers = mkOverride 150 [ ];