[lxc-devel] [lxcfs/master] cpuview: fixes

brauner on Github lxc-bot at linuxcontainers.org
Wed Jun 10 08:43:42 UTC 2020


A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 365 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20200610/cfaf037a/attachment-0001.bin>
-------------- next part --------------
From 6a4dceb11b54d8d99a2489911baa8558fbb78e18 Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Wed, 10 Jun 2020 09:23:33 +0200
Subject: [PATCH 01/14] proc_cpuview: cleanup new_proc_stat_node()

Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
 src/proc_cpuview.c | 82 ++++++++++++++++++++--------------------------
 1 file changed, 35 insertions(+), 47 deletions(-)

diff --git a/src/proc_cpuview.c b/src/proc_cpuview.c
index c576bba..9b2b1ab 100644
--- a/src/proc_cpuview.c
+++ b/src/proc_cpuview.c
@@ -133,13 +133,22 @@ static bool expand_proc_stat_node(struct cg_proc_stat *node, int cpu_count)
 
 static void free_proc_stat_node(struct cg_proc_stat *node)
 {
-	pthread_mutex_destroy(&node->lock);
-	free_disarm(node->cg);
-	free_disarm(node->usage);
-	free_disarm(node->view);
-	free_disarm(node);
+	if (node) {
+		/*
+		 * We're abusing the usage pointer to indicate that
+		 * pthread_mutex_init() was successful. Don't judge me.
+		 */
+		if (node->usage)
+			pthread_mutex_destroy(&node->lock);
+		free_disarm(node->cg);
+		free_disarm(node->usage);
+		free_disarm(node->view);
+		free_disarm(node);
+	}
 }
 
+define_cleanup_function(struct cg_proc_stat *, free_proc_stat_node);
+
 static struct cg_proc_stat *add_proc_stat_node(struct cg_proc_stat *new_node)
 {
 	int hash = calc_hash(new_node->cg) % CPUVIEW_HASH_SIZE;
@@ -177,60 +186,39 @@ static struct cg_proc_stat *add_proc_stat_node(struct cg_proc_stat *new_node)
 	return rv;
 }
 
-static struct cg_proc_stat *new_proc_stat_node(struct cpuacct_usage *usage, int cpu_count, const char *cg)
+static struct cg_proc_stat *new_proc_stat_node(struct cpuacct_usage *usage,
+					       int cpu_count, const char *cg)
 {
-	struct cg_proc_stat *node;
-	int i;
+	call_cleaner(free_proc_stat_node) struct cg_proc_stat *node = NULL;
+	__do_free struct cpuacct_usage *new_usage = NULL;
 
-	node = malloc(sizeof(struct cg_proc_stat));
+	node = zalloc(sizeof(struct cg_proc_stat));
 	if (!node)
-		goto err;
-
-	node->cg = NULL;
-	node->usage = NULL;
-	node->view = NULL;
+		return NULL;
 
-	node->cg = malloc(strlen(cg) + 1);
+	node->cg = strdup(cg);
 	if (!node->cg)
-		goto err;
-
-	strcpy(node->cg, cg);
-
-	node->usage = malloc(sizeof(struct cpuacct_usage) * cpu_count);
-	if (!node->usage)
-		goto err;
+		return NULL;
 
-	memcpy(node->usage, usage, sizeof(struct cpuacct_usage) * cpu_count);
+	new_usage = memdup(usage, sizeof(struct cpuacct_usage) * cpu_count);
+	if (!new_usage)
+		return NULL;
 
-	node->view = malloc(sizeof(struct cpuacct_usage) * cpu_count);
+	node->view = zalloc(sizeof(struct cpuacct_usage) * cpu_count);
 	if (!node->view)
-		goto err;
+		return NULL;
 
 	node->cpu_count = cpu_count;
-	node->next = NULL;
-
-	if (pthread_mutex_init(&node->lock, NULL) != 0)
-		log_error(goto err, "Failed to initialize node lock");
-
-	for (i = 0; i < cpu_count; i++) {
-		node->view[i].user = 0;
-		node->view[i].system = 0;
-		node->view[i].idle = 0;
-	}
 
-	return node;
+	if (pthread_mutex_init(&node->lock, NULL))
+		return NULL;
+	/*
+	 * We're abusing the usage pointer to indicate that
+	 * pthread_mutex_init() was successful. Don't judge me.
+	 */
+	node->usage = move_ptr(new_usage);
 
-err:
-	if (node && node->cg)
-		free(node->cg);
-	if (node && node->usage)
-		free(node->usage);
-	if (node && node->view)
-		free(node->view);
-	if (node)
-		free(node);
-
-	return NULL;
+	return move_ptr(node);
 }
 
 static bool cgfs_param_exist(const char *controller, const char *cgroup,

From 164acda74caf904092ec8cc258952a30a09725c6 Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Wed, 10 Jun 2020 09:35:16 +0200
Subject: [PATCH 02/14] proc_cpuview: use more descriptive labels in
 add_proc_stat_node()

Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
 src/proc_cpuview.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/proc_cpuview.c b/src/proc_cpuview.c
index 9b2b1ab..589da40 100644
--- a/src/proc_cpuview.c
+++ b/src/proc_cpuview.c
@@ -159,7 +159,7 @@ static struct cg_proc_stat *add_proc_stat_node(struct cg_proc_stat *new_node)
 
 	if (!head->next) {
 		head->next = new_node;
-		goto out;
+		goto out_rwlock_unlock;
 	}
 
 	node = head->next;
@@ -169,7 +169,7 @@ static struct cg_proc_stat *add_proc_stat_node(struct cg_proc_stat *new_node)
 			/* The node is already present, return it */
 			free_proc_stat_node(new_node);
 			rv = node;
-			goto out;
+			goto out_rwlock_unlock;
 		}
 
 		if (node->next) {
@@ -178,10 +178,10 @@ static struct cg_proc_stat *add_proc_stat_node(struct cg_proc_stat *new_node)
 		}
 
 		node->next = new_node;
-		goto out;
+		goto out_rwlock_unlock;
 	}
 
-out:
+out_rwlock_unlock:
 	pthread_rwlock_unlock(&head->lock);
 	return rv;
 }

From 905769cd18db23355602f2d33209560bbde77945 Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Wed, 10 Jun 2020 09:35:37 +0200
Subject: [PATCH 03/14] proc_cpuview: reduce variable scope in
 cpuview_free_head()

Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
 src/proc_cpuview.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/src/proc_cpuview.c b/src/proc_cpuview.c
index 589da40..e325978 100644
--- a/src/proc_cpuview.c
+++ b/src/proc_cpuview.c
@@ -1123,16 +1123,15 @@ bool init_cpuview(void)
 
 static void cpuview_free_head(struct cg_proc_stat_head *head)
 {
-	struct cg_proc_stat *node, *tmp;
+	struct cg_proc_stat *node;
 
 	if (head->next) {
 		node = head->next;
 
 		for (;;) {
-			tmp = node;
+			struct cg_proc_stat *cur = node;
 			node = node->next;
-			free_proc_stat_node(tmp);
-
+			free_proc_stat_node(cur);
 			if (!node)
 				break;
 		}

From 0d129671b9b4ec04eb4aa9d4f23ba17fb30f4a7e Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Wed, 10 Jun 2020 09:48:55 +0200
Subject: [PATCH 04/14] proc_cpuview: cleanup add_proc_stat_node()

Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
 src/proc_cpuview.c | 30 ++++++++++++++++++------------
 1 file changed, 18 insertions(+), 12 deletions(-)

diff --git a/src/proc_cpuview.c b/src/proc_cpuview.c
index e325978..c3cabcb 100644
--- a/src/proc_cpuview.c
+++ b/src/proc_cpuview.c
@@ -151,39 +151,45 @@ define_cleanup_function(struct cg_proc_stat *, free_proc_stat_node);
 
 static struct cg_proc_stat *add_proc_stat_node(struct cg_proc_stat *new_node)
 {
-	int hash = calc_hash(new_node->cg) % CPUVIEW_HASH_SIZE;
+	call_cleaner(free_proc_stat_node) struct cg_proc_stat *new = new_node;
+	struct cg_proc_stat *rv = new_node;
+	int hash = calc_hash(new->cg) % CPUVIEW_HASH_SIZE;
 	struct cg_proc_stat_head *head = proc_stat_history[hash];
-	struct cg_proc_stat *node, *rv = new_node;
+	struct cg_proc_stat *cur;
 
 	pthread_rwlock_wrlock(&head->lock);
 
 	if (!head->next) {
-		head->next = new_node;
+		head->next = move_ptr(new);
 		goto out_rwlock_unlock;
 	}
 
-	node = head->next;
+	cur = head->next;
 
 	for (;;) {
-		if (strcmp(node->cg, new_node->cg) == 0) {
-			/* The node is already present, return it */
-			free_proc_stat_node(new_node);
-			rv = node;
+		/*
+		 * The node to be added is already present in the list, so
+		 * free the newly allocated one and return the one we found.
+		 */
+		if (strcmp(cur->cg, new->cg) == 0) {
+			rv = cur;
 			goto out_rwlock_unlock;
 		}
 
-		if (node->next) {
-			node = node->next;
+		/* Keep walking. */
+		if (cur->next) {
+			cur = cur->next;
 			continue;
 		}
 
-		node->next = new_node;
+		/* Add new node to end of list. */
+		cur->next = move_ptr(new);
 		goto out_rwlock_unlock;
 	}
 
 out_rwlock_unlock:
 	pthread_rwlock_unlock(&head->lock);
-	return rv;
+	return move_ptr(rv);
 }
 
 static struct cg_proc_stat *new_proc_stat_node(struct cpuacct_usage *usage,

From ce617d73c3a8bfb7f65eda9eb6e93dc85a7b0a84 Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Wed, 10 Jun 2020 09:50:18 +0200
Subject: [PATCH 05/14] proc_cpuview: use correct comment style

Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
 src/proc_cpuview.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/proc_cpuview.c b/src/proc_cpuview.c
index c3cabcb..cb0f99d 100644
--- a/src/proc_cpuview.c
+++ b/src/proc_cpuview.c
@@ -52,10 +52,10 @@
 /* Data for CPU view */
 struct cg_proc_stat {
 	char *cg;
-	struct cpuacct_usage *usage; // Real usage as read from the host's /proc/stat
-	struct cpuacct_usage *view; // Usage stats reported to the container
+	struct cpuacct_usage *usage; 	/* Real usage as read from the host's /proc/stat. */
+	struct cpuacct_usage *view; 	/* Usage stats reported to the container. */
 	int cpu_count;
-	pthread_mutex_t lock; // For node manipulation
+	pthread_mutex_t lock; 		/* For node manipulation. */
 	struct cg_proc_stat *next;
 };
 

From 82d74a959246162d628ffad5c5fe7cdb30a81dc9 Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Wed, 10 Jun 2020 09:52:53 +0200
Subject: [PATCH 06/14] proc_cpuview: clean up expand_proc_stat_node()

Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
 src/proc_cpuview.c | 24 ++++++++----------------
 1 file changed, 8 insertions(+), 16 deletions(-)

diff --git a/src/proc_cpuview.c b/src/proc_cpuview.c
index cb0f99d..ef71515 100644
--- a/src/proc_cpuview.c
+++ b/src/proc_cpuview.c
@@ -92,32 +92,24 @@ static bool expand_proc_stat_node(struct cg_proc_stat *node, int cpu_count)
 	__do_free struct cpuacct_usage *new_usage = NULL, *new_view = NULL;
 
 	/* Allocate new memory */
-	new_usage = malloc(sizeof(struct cpuacct_usage) * cpu_count);
+	new_usage = zalloc(sizeof(struct cpuacct_usage) * cpu_count);
 	if (!new_usage)
 		return false;
 
-	new_view = malloc(sizeof(struct cpuacct_usage) * cpu_count);
+	new_view = zalloc(sizeof(struct cpuacct_usage) * cpu_count);
 	if (!new_view)
 		return false;
 
 	/* Copy existing data & initialize new elements */
 	for (int i = 0; i < cpu_count; i++) {
 		if (i < node->cpu_count) {
-			new_usage[i].user = node->usage[i].user;
-			new_usage[i].system = node->usage[i].system;
-			new_usage[i].idle = node->usage[i].idle;
+			new_usage[i].user 	= node->usage[i].user;
+			new_usage[i].system 	= node->usage[i].system;
+			new_usage[i].idle 	= node->usage[i].idle;
 
-			new_view[i].user = node->view[i].user;
-			new_view[i].system = node->view[i].system;
-			new_view[i].idle = node->view[i].idle;
-		} else {
-			new_usage[i].user = 0;
-			new_usage[i].system = 0;
-			new_usage[i].idle = 0;
-
-			new_view[i].user = 0;
-			new_view[i].system = 0;
-			new_view[i].idle = 0;
+			new_view[i].user 	= node->view[i].user;
+			new_view[i].system 	= node->view[i].system;
+			new_view[i].idle 	= node->view[i].idle;
 		}
 	}
 

From 2d00d04c9e923814336002bd13edce7c3ff8a8ea Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Wed, 10 Jun 2020 09:56:31 +0200
Subject: [PATCH 07/14] proc_cpuview: clean up prune_proc_stat_list()

Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
 src/proc_cpuview.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/src/proc_cpuview.c b/src/proc_cpuview.c
index ef71515..dbb409d 100644
--- a/src/proc_cpuview.c
+++ b/src/proc_cpuview.c
@@ -219,8 +219,8 @@ static struct cg_proc_stat *new_proc_stat_node(struct cpuacct_usage *usage,
 	return move_ptr(node);
 }
 
-static bool cgfs_param_exist(const char *controller, const char *cgroup,
-			     const char *file)
+static bool cgroup_supports(const char *controller, const char *cgroup,
+			    const char *file)
 {
 	__do_free char *path = NULL;
 	int cfd;
@@ -230,7 +230,7 @@ static bool cgfs_param_exist(const char *controller, const char *cgroup,
 		return false;
 
 	path = must_make_path_relative(cgroup, file, NULL);
-	return (faccessat(cfd, path, F_OK, 0) == 0);
+	return faccessat(cfd, path, F_OK, 0) == 0;
 }
 
 static struct cg_proc_stat *prune_proc_stat_list(struct cg_proc_stat *node)
@@ -238,10 +238,8 @@ static struct cg_proc_stat *prune_proc_stat_list(struct cg_proc_stat *node)
 	struct cg_proc_stat *first = NULL;
 
 	for (struct cg_proc_stat *prev = NULL; node; ) {
-		if (!cgfs_param_exist("cpu", node->cg, "cpu.shares")) {
-			struct cg_proc_stat *tmp = node;
-
-			lxcfs_debug("Removing stat node for %s\n", node->cg);
+		if (!cgroup_supports("cpu", node->cg, "cpu.shares")) {
+			call_cleaner(free_proc_stat_node) struct cg_proc_stat *cur = node;
 
 			if (prev)
 				prev->next = node->next;
@@ -249,7 +247,7 @@ static struct cg_proc_stat *prune_proc_stat_list(struct cg_proc_stat *node)
 				first = node->next;
 
 			node = node->next;
-			free_proc_stat_node(tmp);
+			lxcfs_debug("Removing stat node for %s\n", cur->cg);
 		} else {
 			if (!first)
 				first = node;

From ce089f1057d891652b0595fcbed37781700b94ef Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Wed, 10 Jun 2020 09:59:01 +0200
Subject: [PATCH 08/14] proc_cpuview: clean up find_or_create_proc_stat_node()

Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
 src/proc_cpuview.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/src/proc_cpuview.c b/src/proc_cpuview.c
index dbb409d..03089c0 100644
--- a/src/proc_cpuview.c
+++ b/src/proc_cpuview.c
@@ -326,11 +326,13 @@ static struct cg_proc_stat *find_or_create_proc_stat_node(struct cpuacct_usage *
 
 	pthread_mutex_lock(&node->lock);
 
-	/* If additional CPUs on the host have been enabled, CPU usage counter
-	 * arrays have to be expanded */
+	/*
+	 * If additional CPUs on the host have been enabled, CPU usage counter
+	 * arrays have to be expanded.
+	 */
 	if (node->cpu_count < cpu_count) {
 		lxcfs_debug("Expanding stat node %d->%d for %s\n",
-				node->cpu_count, cpu_count, cg);
+			    node->cpu_count, cpu_count, cg);
 
 		if (!expand_proc_stat_node(node, cpu_count)) {
 			pthread_mutex_unlock(&node->lock);

From 8206874af4ae4df7b507d3a9afa37d98a7f884e3 Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Wed, 10 Jun 2020 10:00:16 +0200
Subject: [PATCH 09/14] proc_cpuview: cleanup add_cpu_usage()

Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
 src/proc_cpuview.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/proc_cpuview.c b/src/proc_cpuview.c
index 03089c0..7d78ae3 100644
--- a/src/proc_cpuview.c
+++ b/src/proc_cpuview.c
@@ -353,7 +353,10 @@ static void add_cpu_usage(uint64_t *surplus, struct cpuacct_usage *usage,
 	if (free_space > usage->idle)
 		free_space = usage->idle;
 
-	to_add = free_space > *surplus ? *surplus : free_space;
+	if (free_space > *surplus)
+		to_add = *surplus;
+	else
+		to_add = free_space;
 
 	*counter += to_add;
 	usage->idle -= to_add;

From 48f6862e8ce930f6059ba03c71addac5fb2dd4ee Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Wed, 10 Jun 2020 10:05:26 +0200
Subject: [PATCH 10/14] proc_cpuview: cleanup read_cpu_cfs_param()

Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
 src/proc_cpuview.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/src/proc_cpuview.c b/src/proc_cpuview.c
index 7d78ae3..a549336 100644
--- a/src/proc_cpuview.c
+++ b/src/proc_cpuview.c
@@ -409,23 +409,23 @@ static uint64_t diff_cpu_usage(struct cpuacct_usage *older,
 static bool read_cpu_cfs_param(const char *cg, const char *param, int64_t *value)
 {
 	__do_free char *str = NULL;
-	char file[11 + 6 + 1]; /* cpu.cfs__us + quota/period + \0 */
+	char file[STRLITERALLEN("cpu.cfs_period_us") + 1];
 	bool first = true;
+	int ret;
 
-	if (!pure_unified_layout(cgroup_ops)) {
-		snprintf(file, sizeof(file), "cpu.cfs_%s_us", param);
-        } else {
-		strcpy(file, "cpu.max");
+	if (pure_unified_layout(cgroup_ops)) {
 		first = !strcmp(param, "quota");
+		ret = snprintf(file, sizeof(file), "cpu.max");
+	} else {
+		ret = snprintf(file, sizeof(file), "cpu.cfs_%s_us", param);
 	}
-
-	if (!cgroup_ops->get(cgroup_ops, "cpu", cg, file, &str))
+	if (ret < 0 || (size_t)ret >= sizeof(file))
 		return false;
 
-	if (sscanf(str, first ? "%" PRId64 : "%*d %" PRId64, value) != 1)
+	if (!cgroup_ops->get(cgroup_ops, "cpu", cg, file, &str))
 		return false;
 
-	return true;
+	return sscanf(str, first ? "%" PRId64 : "%*d %" PRId64, value) == 1;
 }
 
 /*

From c602a0d0c3c96784fc23e374f678ba5d3b5e17d7 Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Wed, 10 Jun 2020 10:06:36 +0200
Subject: [PATCH 11/14] proc_cpuview: fix exact_cpu_count()

Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
 src/proc_cpuview.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/src/proc_cpuview.c b/src/proc_cpuview.c
index a549336..fd06176 100644
--- a/src/proc_cpuview.c
+++ b/src/proc_cpuview.c
@@ -438,8 +438,11 @@ static double exact_cpu_count(const char *cg)
 	int nprocs;
 	int64_t cfs_quota, cfs_period;
 
-	read_cpu_cfs_param(cg, "quota", &cfs_quota);
-	read_cpu_cfs_param(cg, "period", &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;

From 921bdfdbe23e731c9b4db2279c2185ad28152ae5 Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Wed, 10 Jun 2020 10:07:58 +0200
Subject: [PATCH 12/14] proc_cpuview: fix max_cpu_count()

Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
 src/proc_cpuview.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/src/proc_cpuview.c b/src/proc_cpuview.c
index fd06176..8408ae3 100644
--- a/src/proc_cpuview.c
+++ b/src/proc_cpuview.c
@@ -468,14 +468,17 @@ int max_cpu_count(const char *cg)
 	int64_t cfs_quota, cfs_period;
 	int nr_cpus_in_cpuset = 0;
 
-	read_cpu_cfs_param(cg, "quota", &cfs_quota);
-	read_cpu_cfs_param(cg, "period", &cfs_period);
+	if (!read_cpu_cfs_param(cg, "quota", &cfs_quota))
+		return 0;
+
+	if (!read_cpu_cfs_param(cg, "period", &cfs_period))
+		return 0;
 
 	cpuset = get_cpuset(cg);
 	if (cpuset)
 		nr_cpus_in_cpuset = cpu_number_in_cpuset(cpuset);
 
-	if (cfs_quota <= 0 || cfs_period <= 0){
+	if (cfs_quota <= 0 || cfs_period <= 0) {
 		if (nr_cpus_in_cpuset > 0)
 			return nr_cpus_in_cpuset;
 
@@ -484,7 +487,8 @@ int max_cpu_count(const char *cg)
 
 	rv = cfs_quota / cfs_period;
 
-	/* In case quota/period does not yield a whole number, add one CPU for
+	/*
+	 * In case quota/period does not yield a whole number, add one CPU for
 	 * the remainder.
 	 */
 	if ((cfs_quota % cfs_period) > 0)
@@ -494,7 +498,7 @@ int max_cpu_count(const char *cg)
 	if (rv > nprocs)
 		rv = nprocs;
 
-	/* use min value in cpu quota and cpuset */
+	/* Use min value in cpu quota and cpuset. */
 	if (nr_cpus_in_cpuset > 0 && nr_cpus_in_cpuset < rv)
 		rv = nr_cpus_in_cpuset;
 

From b457272231bd5452d7a46a930835253cb4994a65 Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Wed, 10 Jun 2020 10:11:35 +0200
Subject: [PATCH 13/14] proc_cpuview: cleanup cpuview_proc_stat()

Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
 src/proc_cpuview.c | 54 +++++++++++++++++++++++++---------------------
 1 file changed, 30 insertions(+), 24 deletions(-)

diff --git a/src/proc_cpuview.c b/src/proc_cpuview.c
index 8408ae3..b839e74 100644
--- a/src/proc_cpuview.c
+++ b/src/proc_cpuview.c
@@ -602,7 +602,7 @@ int cpuview_proc_stat(const char *cg, const char *cpuset,
 	if (!stat_node)
 		return log_error(0, "Failed to find/create stat node for %s", cg);
 
-	diff = malloc(sizeof(struct cpuacct_usage) * nprocs);
+	diff = zalloc(sizeof(struct cpuacct_usage) * nprocs);
 	if (!diff)
 		return 0;
 
@@ -630,13 +630,13 @@ int cpuview_proc_stat(const char *cg, const char *cpuset,
 
 		i++;
 
-		stat_node->usage[curcpu].user += diff[curcpu].user;
+		stat_node->usage[curcpu].user 	+= diff[curcpu].user;
 		stat_node->usage[curcpu].system += diff[curcpu].system;
-		stat_node->usage[curcpu].idle += diff[curcpu].idle;
+		stat_node->usage[curcpu].idle 	+= diff[curcpu].idle;
 
 		if (max_cpus > 0 && i >= max_cpus) {
-			user_surplus += diff[curcpu].user;
-			system_surplus += diff[curcpu].system;
+			user_surplus 	+= diff[curcpu].user;
+			system_surplus 	+= diff[curcpu].system;
 		}
 	}
 
@@ -690,20 +690,20 @@ int cpuview_proc_stat(const char *cg, const char *cpuset,
 			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;
+			stat_node->view[curcpu].user 	+= diff[curcpu].user;
+			stat_node->view[curcpu].system 	+= diff[curcpu].system;
+			stat_node->view[curcpu].idle 	+= diff[curcpu].idle;
 
-			user_sum += stat_node->view[curcpu].user;
-			system_sum += stat_node->view[curcpu].system;
-			idle_sum += stat_node->view[curcpu].idle;
+			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;
+			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;
+				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);
@@ -718,12 +718,18 @@ int cpuview_proc_stat(const char *cg, const char *cpuset,
 			lxcfs_v("revising cpu usage view to match the exact cpu count [%f]\n", exact_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;
+			if (idle_sum > delta)
+				idle_sum = idle_sum - delta;
+			else
+				idle_sum = 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;
+			if (stat_node->view[curcpu].idle > delta)
+				stat_node->view[curcpu].idle = stat_node->view[curcpu].idle - delta;
+			else
+				stat_node->view[curcpu].idle = 0;
 			lxcfs_v("curcpu: %d, idle after: %lu\n", curcpu, stat_node->view[curcpu].idle);
 		}
 	} else {
@@ -731,13 +737,13 @@ int cpuview_proc_stat(const char *cg, const char *cpuset,
 			if (!stat_node->usage[curcpu].online)
 				continue;
 
-			stat_node->view[curcpu].user = stat_node->usage[curcpu].user;
-			stat_node->view[curcpu].system = stat_node->usage[curcpu].system;
-			stat_node->view[curcpu].idle = stat_node->usage[curcpu].idle;
+			stat_node->view[curcpu].user 	= stat_node->usage[curcpu].user;
+			stat_node->view[curcpu].system 	= stat_node->usage[curcpu].system;
+			stat_node->view[curcpu].idle 	= stat_node->usage[curcpu].idle;
 
-			user_sum += stat_node->view[curcpu].user;
-			system_sum += stat_node->view[curcpu].system;
-			idle_sum += stat_node->view[curcpu].idle;
+			user_sum 	+= stat_node->view[curcpu].user;
+			system_sum 	+= stat_node->view[curcpu].system;
+			idle_sum 	+= stat_node->view[curcpu].idle;
 		}
 	}
 

From 9d7fc1a317a362d1276d7bba907a3846bf182ebd Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Wed, 10 Jun 2020 10:17:22 +0200
Subject: [PATCH 14/14] proc_cpuview: cleanup cpuview_init_head()

Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
 src/proc_cpuview.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/src/proc_cpuview.c b/src/proc_cpuview.c
index b839e74..7d6c0bd 100644
--- a/src/proc_cpuview.c
+++ b/src/proc_cpuview.c
@@ -1097,18 +1097,18 @@ int read_cpuacct_usage_all(char *cg, char *cpuset,
 
 static bool cpuview_init_head(struct cg_proc_stat_head **head)
 {
-	*head = malloc(sizeof(struct cg_proc_stat_head));
-	if (!(*head))
-		return log_error(false, "%s", strerror(errno));
+	__do_free struct cg_proc_stat_head *h;
 
-	(*head)->lastcheck = time(NULL);
-	(*head)->next = NULL;
+	h = zalloc(sizeof(struct cg_proc_stat_head));
+	if (!h)
+		return false;
 
-	if (pthread_rwlock_init(&(*head)->lock, NULL) != 0) {
-		free_disarm(*head);
-		return log_error(false, "Failed to initialize list lock");
-	}
+	if (pthread_rwlock_init(&h->lock, NULL))
+		return false;
+
+	h->lastcheck = time(NULL);
 
+	*head = move_ptr(h);
 	return true;
 }
 


More information about the lxc-devel mailing list