Tweak squashfs for low-memory systems #2383

Merged
merged 1 commit into from Sep 15, 2016

Conversation

Projects
None yet
4 participants
Owner

stgraber commented Sep 15, 2016

On low-memory systems (below 2GB of RAM), limit unsquashfs to a single
CPU and to 10% of the RAM for its fragment size.

Closes #2382

Signed-off-by: Stéphane Graber stgraber@ubuntu.com

Tweak squashfs for low-memory systems
On low-memory systems (below 2GB of RAM), limit unsquashfs to a single
CPU and to 10% of the RAM for its fragment size.

Closes #2382

Signed-off-by: Stéphane Graber <stgraber@ubuntu.com>

@tych0 tych0 merged commit bf69978 into lxc:master Sep 15, 2016

4 checks passed

Branch target Branch target is correct
Details
Signed-off-by All commits signed-off
Details
Testsuite Testsuite passed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

Nitpicking and late to the party...

@@ -99,6 +99,13 @@ func unpack(file string, path string) error {
} else if strings.HasPrefix(extension, ".squashfs") {
command = "unsquashfs"
args = append(args, "-f", "-d", path, "-n")
+
+ mem, err := deviceTotalMemory()
+ mem = mem / 1024 / 1024 / 10
@sean-jc

sean-jc Sep 15, 2016

Contributor

This is non-intuitive for me; even with the commit message it took me a bit to work through the logic. I think it's especially confusing for people used to power-of-two math, the 10 seems so out of place. Any objection to changing the code to something like this? This assumes the comparison against 2.5gb instead of 2gb was unintentional.

if err == nil && mem <= (1<<31) {
    mem = mem / 1024 / 1024 / 10
    ...
}
@stgraber

stgraber Sep 15, 2016

Owner

The comparison against 2.5GB was absolutely intentional.

@stgraber

stgraber Sep 15, 2016

Owner

The reason is simply that unsquashfs defaults to 256MB chunks, so we only want to set the argument if the resulting computed chunk size is smaller than that and we don't want that chunk to be bigger than 10% of the system's memory.

@stgraber

stgraber Sep 15, 2016

Owner

The "mem" variable is intended as the chunk of memory in MB that we're willing to let unsquashfs use.

@sean-jc

sean-jc Sep 15, 2016

Contributor

I understand the intent and logic, but for someone coming along later I think the code will be quite confusing, especially since the commit message doesn't match the code. 2gb vs 2.5gb is nitpicking, but my initial reaction when looking at the code was "that's not 2gb". I can see someone (like me) changing the code to actually compare against 2gb, thinking they're doing the right thing.

Just want to confirm this works on my low mem env:

Creating barkier-maxton
Starting barkier-maxton
error: Error calling 'lxd forkstart barkier-maxton /var/lib/lxd/containers /var/log/lxd/barkier-maxton/lxc.conf': err='exit status 1'
  lxc 20160915190402.013 ERROR lxc_conf - conf.c:lxc_mount_auto_mounts:781 - No such file or directory - error mounting /sys/fs/cgroup
  lxc 20160915190402.013 ERROR lxc_conf - conf.c:lxc_setup:3758 - failed to setup the automatic mounts for 'barkier-maxton'
  lxc 20160915190402.013 ERROR lxc_start - start.c:do_start:834 - failed to setup the container
  lxc 20160915190402.014 ERROR lxc_sync - sync.c:__sync_wait:57 - An error occurred in another process (expected sequence number 3)
  lxc 20160915190402.015 ERROR lxc_start - start.c:__lxc_start:1354 - failed to spawn 'barkier-maxton'

Try `lxc info --show-log barkier-maxton` for more info
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment