the introduction of nested cgroups in our LXC package exposed a bug in lxcfs swap limit handling - for the regular memory value, it already looked for the minimum limit of the hierarchy, but for the swap limit it didn't.
Signed-off-by: Fabian Grünbichler <[email protected]> --- @w.bumiller could you take a look before this gets forward upstream? I am not 100% sure about leaving the check via memswlimit_str in both methods (e.g. line 3124/3123), although it's irrelevant for us because swap accounting is always on in our kernel. debian/patches/fix-swap-with-nested-cgroups.patch | 143 ++++++++++++++++++++++ debian/patches/series | 1 + 2 files changed, 144 insertions(+) create mode 100644 debian/patches/fix-swap-with-nested-cgroups.patch diff --git a/debian/patches/fix-swap-with-nested-cgroups.patch b/debian/patches/fix-swap-with-nested-cgroups.patch new file mode 100644 index 0000000..aa14941 --- /dev/null +++ b/debian/patches/fix-swap-with-nested-cgroups.patch @@ -0,0 +1,143 @@ +diff --git a/bindings.c b/bindings.c +index 9fbabb6..d20e437 100644 +--- a/bindings.c ++++ b/bindings.c +@@ -3046,12 +3046,12 @@ static int read_file(const char *path, char *buf, size_t size, + * FUSE ops for /proc + */ + +-static unsigned long get_memlimit(const char *cgroup) ++static unsigned long get_memlimit(const char *cgroup, const char *file) + { + char *memlimit_str = NULL; + unsigned long memlimit = -1; + +- if (cgfs_get_value("memory", cgroup, "memory.limit_in_bytes", &memlimit_str)) ++ if (cgfs_get_value("memory", cgroup, file, &memlimit_str)) + memlimit = strtoul(memlimit_str, NULL, 10); + + free(memlimit_str); +@@ -3059,16 +3059,16 @@ static unsigned long get_memlimit(const char *cgroup) + return memlimit; + } + +-static unsigned long get_min_memlimit(const char *cgroup) ++static unsigned long get_min_memlimit(const char *cgroup, const char *file) + { + char *copy = strdupa(cgroup); + unsigned long memlimit = 0, retlimit; + +- retlimit = get_memlimit(copy); ++ retlimit = get_memlimit(copy, file); + + while (strcmp(copy, "/") != 0) { + copy = dirname(copy); +- memlimit = get_memlimit(copy); ++ memlimit = get_memlimit(copy, file); + if (memlimit != -1 && memlimit < retlimit) + retlimit = memlimit; + }; +@@ -3083,8 +3083,7 @@ static int proc_meminfo_read(char *buf, size_t size, off_t offset, + struct file_info *d = (struct file_info *)fi->fh; + char *cg; + char *memusage_str = NULL, *memstat_str = NULL, +- *memswlimit_str = NULL, *memswusage_str = NULL, +- *memswlimit_default_str = NULL, *memswusage_default_str = NULL; ++ *memswlimit_str = NULL, *memswusage_str = NULL; + unsigned long memlimit = 0, memusage = 0, memswlimit = 0, memswusage = 0, + cached = 0, hosttotal = 0, active_anon = 0, inactive_anon = 0, + active_file = 0, inactive_file = 0, unevictable = 0; +@@ -3113,7 +3112,7 @@ static int proc_meminfo_read(char *buf, size_t size, off_t offset, + return read_file("/proc/meminfo", buf, size, d); + prune_init_slice(cg); + +- memlimit = get_min_memlimit(cg); ++ memlimit = get_min_memlimit(cg, "memory.limit_in_bytes"); + if (!cgfs_get_value("memory", cg, "memory.usage_in_bytes", &memusage_str)) + goto err; + if (!cgfs_get_value("memory", cg, "memory.stat", &memstat_str)) +@@ -3124,20 +3123,9 @@ static int proc_meminfo_read(char *buf, size_t size, off_t offset, + if(cgfs_get_value("memory", cg, "memory.memsw.limit_in_bytes", &memswlimit_str) && + cgfs_get_value("memory", cg, "memory.memsw.usage_in_bytes", &memswusage_str)) + { +- /* If swapaccounting is turned on, then default value is assumed to be that of cgroup / */ +- if (!cgfs_get_value("memory", "/", "memory.memsw.limit_in_bytes", &memswlimit_default_str)) +- goto err; +- if (!cgfs_get_value("memory", "/", "memory.memsw.usage_in_bytes", &memswusage_default_str)) +- goto err; +- +- memswlimit = strtoul(memswlimit_str, NULL, 10); ++ memswlimit = get_min_memlimit(cg, "memory.memsw.limit_in_bytes"); + memswusage = strtoul(memswusage_str, NULL, 10); + +- if (!strcmp(memswlimit_str, memswlimit_default_str)) +- memswlimit = 0; +- if (!strcmp(memswusage_str, memswusage_default_str)) +- memswusage = 0; +- + memswlimit = memswlimit / 1024; + memswusage = memswusage / 1024; + } +@@ -3257,8 +3245,6 @@ err: + free(memswlimit_str); + free(memswusage_str); + free(memstat_str); +- free(memswlimit_default_str); +- free(memswusage_default_str); + return rv; + } + +@@ -3859,8 +3845,7 @@ static int proc_swaps_read(char *buf, size_t size, off_t offset, + struct fuse_context *fc = fuse_get_context(); + struct file_info *d = (struct file_info *)fi->fh; + char *cg = NULL; +- char *memswlimit_str = NULL, *memlimit_str = NULL, *memusage_str = NULL, *memswusage_str = NULL, +- *memswlimit_default_str = NULL, *memswusage_default_str = NULL; ++ char *memswlimit_str = NULL, *memlimit_str = NULL, *memusage_str = NULL, *memswusage_str = NULL; + unsigned long memswlimit = 0, memlimit = 0, memusage = 0, memswusage = 0, swap_total = 0, swap_free = 0; + ssize_t total_len = 0, rv = 0; + ssize_t l = 0; +@@ -3885,32 +3870,19 @@ static int proc_swaps_read(char *buf, size_t size, off_t offset, + return read_file("/proc/swaps", buf, size, d); + prune_init_slice(cg); + +- if (!cgfs_get_value("memory", cg, "memory.limit_in_bytes", &memlimit_str)) +- goto err; ++ memlimit = get_min_memlimit(cg, "memory.limit_in_bytes"); + + if (!cgfs_get_value("memory", cg, "memory.usage_in_bytes", &memusage_str)) + goto err; + +- memlimit = strtoul(memlimit_str, NULL, 10); + memusage = strtoul(memusage_str, NULL, 10); + + if (cgfs_get_value("memory", cg, "memory.memsw.usage_in_bytes", &memswusage_str) && + cgfs_get_value("memory", cg, "memory.memsw.limit_in_bytes", &memswlimit_str)) { + +- /* If swap accounting is turned on, then default value is assumed to be that of cgroup / */ +- if (!cgfs_get_value("memory", "/", "memory.memsw.limit_in_bytes", &memswlimit_default_str)) +- goto err; +- if (!cgfs_get_value("memory", "/", "memory.memsw.usage_in_bytes", &memswusage_default_str)) +- goto err; +- +- memswlimit = strtoul(memswlimit_str, NULL, 10); ++ memswlimit = get_min_memlimit(cg, "memory.memsw.limit_in_bytes"); + memswusage = strtoul(memswusage_str, NULL, 10); + +- if (!strcmp(memswlimit_str, memswlimit_default_str)) +- memswlimit = 0; +- if (!strcmp(memswusage_str, memswusage_default_str)) +- memswusage = 0; +- + swap_total = (memswlimit - memlimit) / 1024; + swap_free = (memswusage - memusage) / 1024; + } +@@ -3964,8 +3936,6 @@ err: + free(memlimit_str); + free(memusage_str); + free(memswusage_str); +- free(memswusage_default_str); +- free(memswlimit_default_str); + return rv; + } + diff --git a/debian/patches/series b/debian/patches/series index f0b6317..433cb3c 100644 --- a/debian/patches/series +++ b/debian/patches/series @@ -1,2 +1,3 @@ do-not-start-without-lxcfs.patch fix-offsets-for-memory.stat-parsing.patch +fix-swap-with-nested-cgroups.patch -- 2.1.4 _______________________________________________ pve-devel mailing list [email protected] http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
