irqbalance/reduce-redundant-code-for-file-processing.patch

From c7122666e0f72634512284fe90258daf1dca5914 Mon Sep 17 00:00:00 2001
From: Yunfeng Ye <yeyunfeng@huawei.com>
Date: Sat, 12 Oct 2019 12:00:59 +0800
Subject: [PATCH 24/53] reduce redundant code for file processing

There are some redundant codes about file processing which with the same
logic. so abstract the same logic to the one function, and invoke it
where it need.

A different is that in add_one_node(), not return if fopen() fail, and
remove the ferror() judgement, I think it has no effect.

Signed-off-by: Yunfeng Ye <yeyunfeng@huawei.com>
---
 activate.c   |  14 +-----
 classify.c   |  21 ++------
 cputree.c    | 132 +++++++++++++++++++++++----------------------------
 irqbalance.h |   3 ++
 numa.c       |  24 ++--------
 5 files changed, 71 insertions(+), 123 deletions(-)

diff --git a/activate.c b/activate.c
index 2812976..ab9702d 100644
--- a/activate.c
+++ b/activate.c
@@ -36,22 +36,10 @@ static int check_affinity(struct irq_info *info, cpumask_t applied_mask)
 {
 	cpumask_t current_mask;
 	char buf[PATH_MAX];
-	char *line = NULL;
-	size_t size = 0;
-	FILE *file;
 
 	sprintf(buf, "/proc/irq/%i/smp_affinity", info->irq);
-	file = fopen(buf, "r");
-	if (!file)
+	if (process_one_line(buf, get_mask_from_bitmap, &current_mask) < 0)
 		return 1;
-	if (getline(&line, &size, file)<=0) {
-		free(line);
-		fclose(file);
-		return 1;
-	}
-	cpumask_parse_user(line, strlen(line), current_mask);
-	fclose(file);
-	free(line);
 
 	return cpus_equal(applied_mask, current_mask);
 }
diff --git a/classify.c b/classify.c
index be1ad0c..c1ac230 100644
--- a/classify.c
+++ b/classify.c
@@ -350,10 +350,7 @@ static struct irq_info *add_one_irq_to_db(const char *devpath, struct irq_info *
 	int numa_node;
 	char path[PATH_MAX];
 	FILE *fd;
-	char *lcpu_mask;
 	GList *entry;
-	ssize_t ret;
-	size_t blen;
 
 	/*
 	 * First check to make sure this isn't a duplicate entry
@@ -409,23 +406,11 @@ get_numa_node:
 	else
 		new->numa_node = get_numa_node(numa_node);
 
+	cpus_setall(new->cpumask);
+
 	sprintf(path, "%s/local_cpus", devpath);
-	fd = fopen(path, "r");
-	if (!fd) {
-		cpus_setall(new->cpumask);
-		goto out;
-	}
-	lcpu_mask = NULL;
-	ret = getline(&lcpu_mask, &blen, fd);
-	fclose(fd);
-	if (ret <= 0) {
-		cpus_setall(new->cpumask);
-	} else {
-		cpumask_parse_user(lcpu_mask, ret, new->cpumask);
-	}
-	free(lcpu_mask);
+	process_one_line(path, get_mask_from_bitmap, &new->cpumask);
 
-out:
 	log(TO_CONSOLE, LOG_INFO, "Adding IRQ %d to database\n", irq);
 	return new;
 }
diff --git a/cputree.c b/cputree.c
index 8b9413b..acf6a47 100644
--- a/cputree.c
+++ b/cputree.c
@@ -59,6 +59,37 @@ cpumask_t cpu_possible_map;
 */
 cpumask_t unbanned_cpus;
 
+int process_one_line(char *path, void (*cb)(char *line, void *data), void *data)
+{
+	FILE *file;
+	char *line = NULL;
+	size_t size = 0;
+	int ret = -1;
+
+	file = fopen(path, "r");
+	if (!file)
+		return ret;
+
+	if (getline(&line, &size, file) > 0) {
+		cb(line, data);
+		ret = 0;
+	}
+	free(line);
+	fclose(file);
+	return ret;
+}
+
+void get_mask_from_bitmap(char *line, void *mask)
+{
+	cpumask_parse_user(line, strlen(line), *(cpumask_t *)mask);
+}
+
+static void get_mask_from_cpulist(char *line, void *mask)
+{
+	if (strlen(line) && line[0] != '\n')
+		cpulist_parse(line, strlen(line), *(cpumask_t *)mask);
+}
+
 /*
  * By default do not place IRQs on CPUs the kernel keeps isolated or
  * nohz_full, as specified through the boot commandline. Users can
@@ -66,9 +97,7 @@ cpumask_t unbanned_cpus;
  */
 static void setup_banned_cpus(void)
 {
-	FILE *file;
-	char *line = NULL;
-	size_t size = 0;
+	char *path = NULL;
 	char buffer[4096];
 	cpumask_t nohz_full;
 	cpumask_t isolated_cpus;
@@ -86,29 +115,12 @@ static void setup_banned_cpus(void)
 		cpumask_parse_user(getenv("IRQBALANCE_BANNED_CPUS"), strlen(getenv("IRQBALANCE_BANNED_CPUS")), banned_cpus);
 		goto out;
 	}
-	file = fopen("/sys/devices/system/cpu/isolated", "r");
-	if (file) {
-		if (getline(&line, &size, file) > 0) {
-			if (strlen(line) && line[0] != '\n')
-				cpulist_parse(line, strlen(line), isolated_cpus);
-		}
-		free(line);
-		line = NULL;
-		size = 0;
-		fclose(file);
-	}
 
-	file = fopen("/sys/devices/system/cpu/nohz_full", "r");
-	if (file) {
-		if (getline(&line, &size, file) > 0) {
-			if (strlen(line) && line[0] != '\n')
-				cpulist_parse(line, strlen(line), nohz_full);
-		}
-		free(line);
-		line = NULL;
-		size = 0;
-		fclose(file);
-	}
+	path = "/sys/devices/system/cpu/isolated";
+	process_one_line(path, get_mask_from_cpulist, &isolated_cpus);
+
+	path = "/sys/devices/system/cpu/nohz_full";
+	process_one_line(path, get_mask_from_cpulist, &nohz_full);
 
 	cpus_or(banned_cpus, nohz_full, isolated_cpus);
 
@@ -258,11 +270,24 @@ static struct topo_obj* add_cpu_to_cache_domain(struct topo_obj *cpu,
 	return cache;
 }
 
+static void get_offline_status(char *line, void *data)
+{
+	int *status = (int *)data;
+
+	*status = (line && line[0] == '0') ? 1 : 0;
+}
+
+static void get_packageid(char *line, void *data)
+{
+	int *packageid = (int *)data;
+
+	*packageid = strtoul(line, NULL, 10);
+}
+
 #define ADJ_SIZE(r,s) PATH_MAX-strlen(r)-strlen(#s) 
 static void do_one_cpu(char *path)
 {
 	struct topo_obj *cpu;
-	FILE *file;
 	char new_path[PATH_MAX];
 	cpumask_t cache_mask, package_mask;
 	struct topo_obj *cache;
@@ -271,21 +296,13 @@ static void do_one_cpu(char *path)
 	int nodeid;
 	int packageid = 0;
 	unsigned int max_cache_index, cache_index, cache_stat;
-	int ret = 1;
+	int offline_status = 0;
 
 	/* skip offline cpus */
 	snprintf(new_path, ADJ_SIZE(path,"/online"), "%s/online", path);
-	file = fopen(new_path, "r");
-	if (file) {
-		char *line = NULL;
-		size_t size = 0;
-		if (getline(&line, &size, file)>0)
-			ret = (line && line[0]=='0') ? 1 : 0;
-		fclose(file);
-		free(line);
-		if (ret)
-			return;
-	}
+	process_one_line(new_path, get_offline_status, &offline_status);
+	if (offline_status)
+		return;
 
 	cpu = calloc(sizeof(struct topo_obj), 1);
 	if (!cpu)
@@ -313,36 +330,17 @@ static void do_one_cpu(char *path)
 		return;
 	}
 
+	cpu_set(cpu->number, package_mask);
 
 	/* try to read the package mask; if it doesn't exist assume solitary */
 	snprintf(new_path, ADJ_SIZE(path, "/topology/core_siblings"),
 		 "%s/topology/core_siblings", path);
-	file = fopen(new_path, "r");
-	cpu_set(cpu->number, package_mask);
-	if (file) {
-		char *line = NULL;
-		size_t size = 0;
-		if (getline(&line, &size, file) > 0)
-			cpumask_parse_user(line, strlen(line), package_mask);
-		fclose(file);
-		free(line);
-		line = NULL;
-		size = 0;
-	}
+	process_one_line(new_path, get_mask_from_bitmap, &package_mask);
+
 	/* try to read the package id */
 	snprintf(new_path, ADJ_SIZE(path, "/topology/physical_package_id"),
 		 "%s/topology/physical_package_id", path);
-	file = fopen(new_path, "r");
-	if (file) {
-		char *line = NULL;
-		size_t size = 0;
-		if (getline(&line, &size, file) > 0)
-			packageid = strtoul(line, NULL, 10);
-		fclose(file);
-		free(line);
-		line = NULL;
-		size = 0;
-	}
+	process_one_line(new_path, get_packageid, &packageid);
 
 	/* try to read the cache mask; if it doesn't exist assume solitary */
 	/* We want the deepest cache level available */
@@ -367,17 +365,7 @@ static void do_one_cpu(char *path)
 		/* Extra 10 subtraction is for the max character length of %d */
 		snprintf(new_path, ADJ_SIZE(path, "/cache/index%d/shared_cpu_map") - 10,
 			 "%s/cache/index%d/shared_cpu_map", path, max_cache_index);
-		file = fopen(new_path, "r");
-		if (file) {
-			char *line = NULL;
-			size_t size = 0;
-			if (getline(&line, &size, file) > 0)
-				cpumask_parse_user(line, strlen(line), cache_mask);
-			fclose(file);
-			free(line);
-			line = NULL;
-			size = 0;
-		}
+		process_one_line(new_path, get_mask_from_bitmap, &cache_mask);
 	}
 
 	nodeid=-1;
diff --git a/irqbalance.h b/irqbalance.h
index 5895e34..a3e561e 100644
--- a/irqbalance.h
+++ b/irqbalance.h
@@ -160,5 +160,8 @@ extern unsigned int log_mask;
 #define SOCKET_PATH "irqbalance"
 #define SOCKET_TMPFS "/run/irqbalance/"
 
+extern int process_one_line(char *path, void (*cb)(char *line, void *data), void *data);
+extern void get_mask_from_bitmap(char *line, void *mask);
+
 #endif /* __INCLUDE_GUARD_IRQBALANCE_H_ */
 
diff --git a/numa.c b/numa.c
index 542e1f4..f1284da 100644
--- a/numa.c
+++ b/numa.c
@@ -56,31 +56,15 @@ static void add_one_node(const char *nodename)
 {
 	char path[PATH_MAX];
 	struct topo_obj *new;
-	char *cpustr = NULL;
-	FILE *f;
-	ssize_t ret;
-	size_t blen;
 
 	new = calloc(1, sizeof(struct topo_obj));
 	if (!new)
 		return;
+
+	cpus_clear(new->mask);
 	sprintf(path, "%s/%s/cpumap", SYSFS_NODE_PATH, nodename);
-	f = fopen(path, "r");
-	if (!f) {
-		free(new);
-		return;
-	}
-	if (ferror(f)) {
-		cpus_clear(new->mask);
-	} else {
-		ret = getline(&cpustr, &blen, f);
-		if (ret <= 0)
-			cpus_clear(new->mask);
-		else
-			cpumask_parse_user(cpustr, ret, new->mask);
-		free(cpustr);
-	}
-	fclose(f);
+	process_one_line(path, get_mask_from_bitmap, &new->mask);
+
 	new->obj_type = OBJ_TYPE_NODE;	
 	new->number = strtoul(&nodename[4], NULL, 10);
 	new->obj_type_list = &numa_nodes;
-- 
2.23.0