[lxc-devel] [lxcfs/master] cgroups: add getter instead of open-coded cgfs_get_value()

brauner on Github lxc-bot at linuxcontainers.org
Thu Feb 20 21:06:52 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/20200220/3821356f/attachment.bin>
-------------- next part --------------
From 1ca6a46742b72f7d4b11f034146fe832915831d0 Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Thu, 20 Feb 2020 22:06:10 +0100
Subject: [PATCH] cgroups: add getter instead of open-coded cgfs_get_value()

Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
 bindings.c       | 63 +++++++++++++++---------------------------------
 cgroups/cgfsng.c | 16 ++++++++++++
 cgroups/cgroup.h |  2 ++
 3 files changed, 38 insertions(+), 43 deletions(-)

diff --git a/bindings.c b/bindings.c
index ab0cd71..9d8cc33 100644
--- a/bindings.c
+++ b/bindings.c
@@ -1102,29 +1102,6 @@ void free_keys(struct cgfs_files **keys)
 	free(keys);
 }
 
-bool cgfs_get_value(const char *controller, const char *cgroup, const char *file, char **value)
-{
-	int ret, cfd;
-	size_t len;
-	char *fnam;
-
-	cfd = find_mounted_controller(controller);
-	if (cfd < 0)
-		return false;
-
-	/* Make sure we pass a relative path to *at() family of functions.
-	 * . + /cgroup + / + file + \0
-	 */
-	len = strlen(cgroup) + strlen(file) + 3;
-	fnam = alloca(len);
-	ret = snprintf(fnam, len, "%s%s/%s", *cgroup == '/' ? "." : "", cgroup, file);
-	if (ret < 0 || (size_t)ret >= len)
-		return false;
-
-	*value = readat_file(cfd, fnam);
-	return *value != NULL;
-}
-
 bool cgfs_param_exist(const char *controller, const char *cgroup, const char *file)
 {
 	int ret, cfd;
@@ -2509,7 +2486,7 @@ bool do_read_pids(pid_t tpid, const char *contrl, const char *cg, const char *fi
 	struct ucred cred;
 	size_t sz = 0, asz = 0;
 
-	if (!cgfs_get_value(contrl, cg, file, &tmpdata))
+	if (!cgroup_ops->get(cgroup_ops, contrl, cg, file, &tmpdata))
 		return false;
 
 	/*
@@ -2623,7 +2600,7 @@ int cg_read(const char *path, char *buf, size_t size, off_t offset,
 		// special case - we have to translate the pids
 		r = do_read_pids(fc->pid, f->controller, f->cgroup, f->file, &data);
 	else
-		r = cgfs_get_value(f->controller, f->cgroup, f->file, &data);
+		r = cgroup_ops->get(cgroup_ops, f->controller, f->cgroup, f->file, &data);
 
 	if (!r) {
 		ret = -EINVAL;
@@ -3311,7 +3288,7 @@ 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, file, &memlimit_str))
+	if (cgroup_ops->get(cgroup_ops, "memory", cgroup, file, &memlimit_str))
 		memlimit = strtoul(memlimit_str, NULL, 10);
 
 	free(memlimit_str);
@@ -3375,15 +3352,15 @@ static int proc_meminfo_read(char *buf, size_t size, off_t offset,
 	prune_init_slice(cg);
 
 	memlimit = get_min_memlimit(cg, "memory.limit_in_bytes");
-	if (!cgfs_get_value("memory", cg, "memory.usage_in_bytes", &memusage_str))
+	if (!cgroup_ops->get(cgroup_ops, "memory", cg, "memory.usage_in_bytes", &memusage_str))
 		goto err;
-	if (!cgfs_get_value("memory", cg, "memory.stat", &memstat_str))
+	if (!cgroup_ops->get(cgroup_ops, "memory", cg, "memory.stat", &memstat_str))
 		goto err;
 
 	// Following values are allowed to fail, because swapaccount might be turned
 	// off for current kernel
-	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(cgroup_ops->get(cgroup_ops, "memory", cg, "memory.memsw.limit_in_bytes", &memswlimit_str) &&
+		cgroup_ops->get(cgroup_ops, "memory", cg, "memory.memsw.usage_in_bytes", &memswusage_str))
 	{
 		memswlimit = get_min_memlimit(cg, "memory.memsw.limit_in_bytes");
 		memswusage = strtoul(memswusage_str, NULL, 10);
@@ -3536,7 +3513,7 @@ char *get_cpuset(const char *cg)
 {
 	char *answer;
 
-	if (!cgfs_get_value("cpuset", cg, "cpuset.cpus", &answer))
+	if (!cgroup_ops->get(cgroup_ops, "cpuset", cg, "cpuset.cpus", &answer))
 		return NULL;
 	return answer;
 }
@@ -3564,7 +3541,7 @@ static bool read_cpu_cfs_param(const char *cg, const char *param, int64_t *value
 
 	sprintf(file, "cpu.cfs_%s_us", param);
 
-	if (!cgfs_get_value("cpu", cg, file, &str))
+	if (!cgroup_ops->get(cgroup_ops, "cpu", cg, file, &str))
 		goto err;
 
 	if (sscanf(str, "%ld", value) != 1)
@@ -4026,10 +4003,10 @@ static int read_cpuacct_usage_all(char *cg, char *cpuset, struct cpuacct_usage *
 		return -ENOMEM;
 
 	memset(cpu_usage, 0, sizeof(struct cpuacct_usage) * cpucount);
-	if (!cgfs_get_value("cpuacct", cg, "cpuacct.usage_all", &usage_str)) {
+	if (!cgroup_ops->get(cgroup_ops, "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)) {
+		if (!cgroup_ops->get(cgroup_ops, "cpuacct", cg, "cpuacct.usage_percpu", &usage_str)) {
 			rv = -1;
 			goto err;
 		}
@@ -5051,7 +5028,7 @@ static double get_reaper_busy(pid_t task)
 	if (!cgroup)
 		goto out;
 	prune_init_slice(cgroup);
-	if (!cgfs_get_value("cpuacct", cgroup, "cpuacct.usage", &usage_str))
+	if (!cgroup_ops->get(cgroup_ops, "cpuacct", cgroup, "cpuacct.usage", &usage_str))
 		goto out;
 	usage = strtoul(usage_str, NULL, 10);
 	res = (double)usage / 1000000000;
@@ -5168,15 +5145,15 @@ static int proc_diskstats_read(char *buf, size_t size, off_t offset,
 		return read_file_fuse("/proc/diskstats", buf, size, d);
 	prune_init_slice(cg);
 
-	if (!cgfs_get_value("blkio", cg, "blkio.io_serviced_recursive", &io_serviced_str))
+	if (!cgroup_ops->get(cgroup_ops, "blkio", cg, "blkio.io_serviced_recursive", &io_serviced_str))
 		goto err;
-	if (!cgfs_get_value("blkio", cg, "blkio.io_merged_recursive", &io_merged_str))
+	if (!cgroup_ops->get(cgroup_ops, "blkio", cg, "blkio.io_merged_recursive", &io_merged_str))
 		goto err;
-	if (!cgfs_get_value("blkio", cg, "blkio.io_service_bytes_recursive", &io_service_bytes_str))
+	if (!cgroup_ops->get(cgroup_ops, "blkio", cg, "blkio.io_service_bytes_recursive", &io_service_bytes_str))
 		goto err;
-	if (!cgfs_get_value("blkio", cg, "blkio.io_wait_time_recursive", &io_wait_time_str))
+	if (!cgroup_ops->get(cgroup_ops, "blkio", cg, "blkio.io_wait_time_recursive", &io_wait_time_str))
 		goto err;
-	if (!cgfs_get_value("blkio", cg, "blkio.io_service_time_recursive", &io_service_time_str))
+	if (!cgroup_ops->get(cgroup_ops, "blkio", cg, "blkio.io_service_time_recursive", &io_service_time_str))
 		goto err;
 
 
@@ -5292,13 +5269,13 @@ static int proc_swaps_read(char *buf, size_t size, off_t offset,
 
 	memlimit = get_min_memlimit(cg, "memory.limit_in_bytes");
 
-	if (!cgfs_get_value("memory", cg, "memory.usage_in_bytes", &memusage_str))
+	if (!cgroup_ops->get(cgroup_ops, "memory", cg, "memory.usage_in_bytes", &memusage_str))
 		goto err;
 
 	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 (cgroup_ops->get(cgroup_ops, "memory", cg, "memory.memsw.usage_in_bytes", &memswusage_str) &&
+	    cgroup_ops->get(cgroup_ops, "memory", cg, "memory.memsw.limit_in_bytes", &memswlimit_str)) {
 
 		memswlimit = get_min_memlimit(cg, "memory.memsw.limit_in_bytes");
 		memswusage = strtoul(memswusage_str, NULL, 10);
diff --git a/cgroups/cgfsng.c b/cgroups/cgfsng.c
index 08b719d..c1a6f7e 100644
--- a/cgroups/cgfsng.c
+++ b/cgroups/cgfsng.c
@@ -588,6 +588,21 @@ static bool cgfsng_get_hierarchies(struct cgroup_ops *ops, int n, char ***out)
 	return true;
 }
 
+static bool cgfsng_get(struct cgroup_ops *ops, const char *controller,
+		       const char *cgroup, const char *file, char **value)
+{
+	__do_free char *path = NULL;
+	struct hierarchy *h;
+
+	h = ops->get_hierarchy(ops, controller);
+	if (!h)
+		return false;
+
+	path = must_make_path(*cgroup == '/' ? "." : "", cgroup, file, NULL);
+	*value = readat_file(h->fd, path);
+	return *value != NULL;
+}
+
 /* At startup, parse_hierarchies finds all the info we need about cgroup
  * mountpoints and current cgroups, and stores it in @d.
  */
@@ -776,6 +791,7 @@ struct cgroup_ops *cgfsng_ops_init(void)
 		return NULL;
 
 	cgfsng_ops->num_hierarchies = cgfsng_num_hierarchies;
+	cgfsng_ops->get = cgfsng_get;
 	cgfsng_ops->get_hierarchies = cgfsng_get_hierarchies;
 	cgfsng_ops->get_hierarchy = get_hierarchy;
 	cgfsng_ops->driver = "cgfsng";
diff --git a/cgroups/cgroup.h b/cgroups/cgroup.h
index 8895533..808e5d0 100644
--- a/cgroups/cgroup.h
+++ b/cgroups/cgroup.h
@@ -120,6 +120,8 @@ struct cgroup_ops {
 	int (*nrtasks)(struct cgroup_ops *ops);
 	struct hierarchy *(*get_hierarchy)(struct cgroup_ops *ops,
 					   const char *controller);
+	bool (*get)(struct cgroup_ops *ops, const char *controller,
+		    const char *cgroup, const char *file, char **value);
 };
 
 extern struct cgroup_ops *cgroup_init(void);


More information about the lxc-devel mailing list