[lxc-devel] [lxcfs/master] Revert "cpu: improve cpu info virtualization"
brauner on Github
lxc-bot at linuxcontainers.org
Tue Jul 23 13:53:15 UTC 2019
A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 773 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20190723/4f86480f/attachment.bin>
-------------- next part --------------
From 39b7df424120fa9c3c5dc209b37990fed57f1efd Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Tue, 23 Jul 2019 15:50:49 +0200
Subject: [PATCH] Revert "cpu: improve cpu info virtualization"
This reverts commit db1b32f6ff056ce713c06b9d41bcc1eadd054abd.
We had a multitude of reports saying that they hit segfaults with
current lxcfs. After a bisect and various mentions in the error reports
of corrupt cpu information this is likely the culprit: revert for now.
Link: https://github.com/lxc/lxd/issues/6000
Link: https://lists.linuxcontainers.org/pipermail/lxc-users/2019-July/014932.html
Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
.gitignore | 1 -
Makefile.am | 1 -
bindings.c | 147 +++++++---------------------------------------------
macro.h | 6 ---
4 files changed, 20 insertions(+), 135 deletions(-)
diff --git a/.gitignore b/.gitignore
index 6e2130c..95b724c 100644
--- a/.gitignore
+++ b/.gitignore
@@ -35,4 +35,3 @@ lxcfs-*.tar.gz
.libs
*.lo
*.la
-.vscode
\ No newline at end of file
diff --git a/Makefile.am b/Makefile.am
index ce88ac9..2c331bb 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -8,7 +8,6 @@ AM_CFLAGS += $(FUSE_CFLAGS)
AM_CFLAGS += -DLIBDIR=\"$(LIBDIR)\"
AM_LDFLAGS = $(FUSE_LIBS) -pthread
#AM_CFLAGS += -DDEBUG
-#AM_CFLAGS += -DVERBOSE
AM_CFLAGS += -DRUNTIME_PATH=\"$(RUNTIME_PATH)\"
diff --git a/bindings.c b/bindings.c
index e911ff6..1b0d1ff 100644
--- a/bindings.c
+++ b/bindings.c
@@ -17,7 +17,6 @@
#include <libgen.h>
#include <pthread.h>
#include <sched.h>
-#include <stdarg.h>
#include <stdbool.h>
#include <stdint.h>
#include <stdio.h>
@@ -1437,22 +1436,19 @@ static int wait_for_pid(pid_t pid)
return 0;
}
+
/*
- * append the given formatted string to *src.
- * src: a pointer to a char* in which to append the formatted string.
+ * append pid to *src.
+ * src: a pointer to a char* in which ot append the pid.
* sz: the number of characters printed so far, minus trailing \0.
* asz: the allocated size so far
- * format: string format. See printf for details.
- * ...: varargs. See printf for details.
+ * pid: the pid to append
*/
-static void must_strcat(char **src, size_t *sz, size_t *asz, const char *format, ...)
+static void must_strcat_pid(char **src, size_t *sz, size_t *asz, pid_t pid)
{
- char tmp[BUF_RESERVE_SIZE];
- va_list args;
+ char tmp[30];
- va_start (args, format);
- int tmplen = vsnprintf(tmp, BUF_RESERVE_SIZE, format, args);
- va_end(args);
+ int tmplen = sprintf(tmp, "%d\n", (int)pid);
if (!*src || tmplen + *sz + 1 >= *asz) {
char *tmp;
@@ -1466,18 +1462,6 @@ static void must_strcat(char **src, size_t *sz, size_t *asz, const char *format,
*sz += tmplen;
}
-/*
- * append pid to *src.
- * src: a pointer to a char* in which ot append the pid.
- * sz: the number of characters printed so far, minus trailing \0.
- * asz: the allocated size so far
- * pid: the pid to append
- */
-static void must_strcat_pid(char **src, size_t *sz, size_t *asz, pid_t pid)
-{
- must_strcat(src, sz, asz, "%d\n", (int)pid);
-}
-
/*
* Given a open file * to /proc/pid/{u,g}id_map, and an id
* valid in the caller's namespace, return the id mapped into
@@ -3695,35 +3679,6 @@ int max_cpu_count(const char *cg)
return rv;
}
-/*
- * Return the exact number of visible CPUs based on CPU quotas.
- * If there is no quota set, zero is returned.
- */
-static double exact_cpu_count(const char *cg)
-{
- double rv;
- int nprocs;
- int64_t cfs_quota, cfs_period;
-
- if (!read_cpu_cfs_param(cg, "quota", &cfs_quota))
- return 0;
-
- if (!read_cpu_cfs_param(cg, "period", &cfs_period))
- return 0;
-
- if (cfs_quota <= 0 || cfs_period <= 0)
- return 0;
-
- rv = (double)cfs_quota / (double)cfs_period;
-
- nprocs = get_nprocs();
-
- if (rv > nprocs)
- rv = nprocs;
-
- return rv;
-}
-
/*
* Determine whether CPU views should be used or not.
*/
@@ -4075,7 +4030,7 @@ static int read_cpuacct_usage_all(char *cg, char *cpuset, struct cpuacct_usage *
{
int cpucount = get_nprocs_conf();
struct cpuacct_usage *cpu_usage;
- int rv = 0, i, j, ret;
+ int rv = 0, i, j, ret, read_pos = 0, read_cnt;
int cg_cpu;
uint64_t cg_user, cg_system;
int64_t ticks_per_sec;
@@ -4084,7 +4039,7 @@ static int read_cpuacct_usage_all(char *cg, char *cpuset, struct cpuacct_usage *
ticks_per_sec = sysconf(_SC_CLK_TCK);
if (ticks_per_sec < 0 && errno == EINVAL) {
- lxcfs_v(
+ lxcfs_debug(
"%s\n",
"read_cpuacct_usage_all failed to determine number of clock ticks "
"in a second");
@@ -4095,39 +4050,11 @@ static int read_cpuacct_usage_all(char *cg, char *cpuset, struct cpuacct_usage *
if (!cpu_usage)
return -ENOMEM;
- memset(cpu_usage, 0, sizeof(struct cpuacct_usage) * cpucount);
if (!cgfs_get_value("cpuacct", cg, "cpuacct.usage_all", &usage_str)) {
- // read cpuacct.usage_percpu instead
- lxcfs_v("failed to read cpuacct.usage_all. reading cpuacct.usage_percpu instead\n%s", "");
- if (!cgfs_get_value("cpuacct", cg, "cpuacct.usage_percpu", &usage_str)) {
- rv = -1;
- goto err;
- }
- lxcfs_v("usage_str: %s\n", usage_str);
-
- // convert cpuacct.usage_percpu into cpuacct.usage_all
- lxcfs_v("converting cpuacct.usage_percpu into cpuacct.usage_all\n%s", "");
-
- char *data = NULL;
- size_t sz = 0, asz = 0;
-
- must_strcat(&data, &sz, &asz, "cpu user system\n");
-
- int i = 0, read_pos = 0, read_cnt=0;
- while (sscanf(usage_str + read_pos, "%lu %n", &cg_user, &read_cnt) > 0) {
- lxcfs_debug("i: %d, cg_user: %lu, read_pos: %d, read_cnt: %d\n", i, cg_user, read_pos, read_cnt);
- must_strcat(&data, &sz, &asz, "%d %lu 0\n", i, cg_user);
- i++;
- read_pos += read_cnt;
- }
-
- free(usage_str);
- usage_str = data;
-
- lxcfs_v("usage_str: %s\n", usage_str);
+ rv = -1;
+ goto err;
}
- int read_pos = 0, read_cnt=0;
if (sscanf(usage_str, "cpu user system\n%n", &read_cnt) != 0) {
lxcfs_error("read_cpuacct_usage_all reading first line from "
"%s/cpuacct.usage_all failed.\n", cg);
@@ -4641,14 +4568,14 @@ static int cpuview_proc_stat(const char *cg, const char *cpuset, struct cpuacct_
threshold = total_sum / cpu_cnt * max_cpus;
for (curcpu = 0, i = -1; curcpu < nprocs; curcpu++) {
+ if (i == max_cpus)
+ break;
+
if (!stat_node->usage[curcpu].online)
continue;
i++;
- if (i == max_cpus)
- break;
-
if (diff[curcpu].user + diff[curcpu].system >= threshold)
continue;
@@ -4675,20 +4602,15 @@ static int cpuview_proc_stat(const char *cg, const char *cpuset, struct cpuacct_
if (system_surplus > 0)
lxcfs_debug("leftover system: %lu for %s\n", system_surplus, cg);
- unsigned long diff_user = 0;
- unsigned long diff_system = 0;
- unsigned long diff_idle = 0;
- unsigned long max_diff_idle = 0;
- unsigned long max_diff_idle_index = 0;
for (curcpu = 0, i = -1; curcpu < nprocs; curcpu++) {
+ if (i == max_cpus)
+ break;
+
if (!stat_node->usage[curcpu].online)
continue;
i++;
- if (i == max_cpus)
- break;
-
stat_node->view[curcpu].user += diff[curcpu].user;
stat_node->view[curcpu].system += diff[curcpu].system;
stat_node->view[curcpu].idle += diff[curcpu].idle;
@@ -4696,34 +4618,8 @@ static int cpuview_proc_stat(const char *cg, const char *cpuset, struct cpuacct_
user_sum += stat_node->view[curcpu].user;
system_sum += stat_node->view[curcpu].system;
idle_sum += stat_node->view[curcpu].idle;
-
- diff_user += diff[curcpu].user;
- diff_system += diff[curcpu].system;
- diff_idle += diff[curcpu].idle;
- if (diff[curcpu].idle > max_diff_idle) {
- max_diff_idle = diff[curcpu].idle;
- max_diff_idle_index = curcpu;
- }
-
- lxcfs_v("curcpu: %d, diff_user: %lu, diff_system: %lu, diff_idle: %lu\n", curcpu, diff[curcpu].user, diff[curcpu].system, diff[curcpu].idle);
- }
- lxcfs_v("total. diff_user: %lu, diff_system: %lu, diff_idle: %lu\n", diff_user, diff_system, diff_idle);
-
- // revise cpu usage view to support partial cpu case
- double exact_cpus = exact_cpu_count(cg);
- if (exact_cpus < (double)max_cpus){
- lxcfs_v("revising cpu usage view to match the exact cpu count [%f]\n", exact_cpus);
- unsigned long delta = (unsigned long)((double)(diff_user + diff_system + diff_idle) * (1 - exact_cpus / (double)max_cpus));
- lxcfs_v("delta: %lu\n", delta);
- lxcfs_v("idle_sum before: %lu\n", idle_sum);
- idle_sum = idle_sum > delta ? idle_sum - delta : 0;
- lxcfs_v("idle_sum after: %lu\n", idle_sum);
-
- curcpu = max_diff_idle_index;
- lxcfs_v("curcpu: %d, idle before: %lu\n", curcpu, stat_node->view[curcpu].idle);
- stat_node->view[curcpu].idle = stat_node->view[curcpu].idle > delta ? stat_node->view[curcpu].idle - delta : 0;
- lxcfs_v("curcpu: %d, idle after: %lu\n", curcpu, stat_node->view[curcpu].idle);
}
+
} else {
for (curcpu = 0; curcpu < nprocs; curcpu++) {
if (!stat_node->usage[curcpu].online)
@@ -4745,12 +4641,12 @@ static int cpuview_proc_stat(const char *cg, const char *cpuset, struct cpuacct_
user_sum,
system_sum,
idle_sum);
- lxcfs_v("cpu-all: %s\n", buf);
if (l < 0) {
perror("Error writing to cache");
rv = 0;
goto err;
+
}
if (l >= buf_size) {
lxcfs_error("%s\n", "Internal error: truncated write to cache.");
@@ -4777,7 +4673,6 @@ static int cpuview_proc_stat(const char *cg, const char *cpuset, struct cpuacct_
stat_node->view[curcpu].user,
stat_node->view[curcpu].system,
stat_node->view[curcpu].idle);
- lxcfs_v("cpu: %s\n", buf);
if (l < 0) {
perror("Error writing to cache");
@@ -4880,11 +4775,9 @@ static int proc_stat_read(char *buf, size_t size, off_t offset,
}
pid_t initpid = lookup_initpid_in_store(fc->pid);
- lxcfs_v("initpid: %d\n", initpid);
if (initpid <= 0)
initpid = fc->pid;
cg = get_pid_cgroup(initpid, "cpuset");
- lxcfs_v("cg: %s\n", cg);
if (!cg)
return read_file("/proc/stat", buf, size, d);
prune_init_slice(cg);
@@ -4899,7 +4792,7 @@ static int proc_stat_read(char *buf, size_t size, off_t offset,
* CPU usage. If not, values from the host's /proc/stat are used.
*/
if (read_cpuacct_usage_all(cg, cpuset, &cg_cpu_usage, &cg_cpu_usage_size) != 0) {
- lxcfs_v("%s\n", "proc_stat_read failed to read from cpuacct, "
+ lxcfs_debug("%s\n", "proc_stat_read failed to read from cpuacct, "
"falling back to the host's /proc/stat");
}
diff --git a/macro.h b/macro.h
index 7213b8f..1763dc2 100644
--- a/macro.h
+++ b/macro.h
@@ -15,10 +15,4 @@
#define lxcfs_debug(format, ...)
#endif /* DEBUG */
-#ifdef VERBOSE
-#define lxcfs_v(format, ...) lxcfs_error(format, __VA_ARGS__);
-#else
-#define lxcfs_v(format, ...)
-#endif /* VERBOSE */
-
#endif /* __LXCFS_MACRO_H */
More information about the lxc-devel
mailing list