From 62bc3afea210eed59dd25fa4cf0fd5ecd083a7ae Mon Sep 17 00:00:00 2001 From: Dmitry Eremin Date: Mon, 12 Jun 2017 20:59:16 -0400 Subject: [PATCH] LU-8703 libcfs: rework CPU pattern parsing code Rewrite CPU pattern parsing code to avoid passed buffer change add real errors propogation to caller function. Change-Id: I8dfee2c0013fcfccd3d99c361d3ec626594689bd Signed-off-by: Dmitry Eremin Reviewed-on: https://review.whamcloud.com/23306 Tested-by: Jenkins Tested-by: Maloo Reviewed-by: James Simmons Reviewed-by: Andreas Dilger Reviewed-by: Patrick Farrell Reviewed-by: Olaf Weber Reviewed-by: Oleg Drokin --- libcfs/libcfs/linux/linux-cpu.c | 208 ++++++++++++++++++++++------------------ 1 file changed, 113 insertions(+), 95 deletions(-) diff --git a/libcfs/libcfs/linux/linux-cpu.c b/libcfs/libcfs/linux/linux-cpu.c index a58bc49..7db6a0f 100644 --- a/libcfs/libcfs/linux/linux-cpu.c +++ b/libcfs/libcfs/linux/linux-cpu.c @@ -449,7 +449,7 @@ int cfs_cpt_set_cpu(struct cfs_cpt_table *cptab, int cpt, int cpu) } if (cpumask_test_cpu(cpu, cptab->ctb_parts[cpt].cpt_cpumask)) { CDEBUG(D_INFO, "CPU %d is already in partition %d cpumask\n", - cpu, cptab->ctb_cpu2cpt[cpu]); + cpu, cptab->ctb_cpu2cpt[cpu]); return 0; } @@ -479,8 +479,7 @@ void cfs_cpt_unset_cpu(struct cfs_cpt_table *cptab, int cpt, int cpu) } } else if (cpt != cptab->ctb_cpu2cpt[cpu]) { - CDEBUG(D_INFO, - "CPU %d is not in cpu-partition %d\n", cpu, cpt); + CDEBUG(D_INFO, "CPU %d is not in CPU partition %d\n", cpu, cpt); return; } @@ -673,11 +672,12 @@ int cfs_cpt_bind(struct cfs_cpt_table *cptab, int cpt) nodemask = cptab->ctb_parts[cpt].cpt_nodemask; } - if (cpumask_any_and(cpumask, cpu_online_mask) >= nr_cpu_ids) { - CERROR("No online CPU found in CPU partition %d, did someone " - "do CPU hotplug on system? You might need to reload " - "Lustre modules to keep system working well.\n", cpt); - return -EINVAL; + if (!cpumask_intersects(cpumask, cpu_online_mask)) { + CDEBUG(D_INFO, "No online CPU found in CPU partition %d, did " + "someone do CPU hotplug on system? You might need to " + "reload Lustre modules to keep system working well.\n", + cpt); + return -ENODEV; } for_each_online_cpu(cpu) { @@ -839,12 +839,14 @@ static struct cfs_cpt_table *cfs_cpt_table_create(int ncpt) cptab = cfs_cpt_table_alloc(ncpt); if (cptab == NULL) { CERROR("Failed to allocate CPU map(%d)\n", ncpt); + rc = -ENOMEM; goto failed; } LIBCFS_ALLOC(node_mask, cpumask_size()); if (node_mask == NULL) { CERROR("Failed to allocate scratch cpumask\n"); + rc = -ENOMEM; goto failed; } @@ -859,8 +861,10 @@ static struct cfs_cpt_table *cfs_cpt_table_create(int ncpt) rc = cfs_cpt_choose_ncpus(cptab, cpt, node_mask, num - ncpu); - if (rc < 0) + if (rc < 0) { + rc = -EINVAL; goto failed; + } ncpu = cpumask_weight(part->cpt_cpumask); if (ncpu == num + !!(rem > 0)) { @@ -884,125 +888,140 @@ failed: if (cptab != NULL) cfs_cpt_table_free(cptab); - return NULL; + return ERR_PTR(rc); } -static struct cfs_cpt_table * -cfs_cpt_table_create_pattern(char *pattern) +static struct cfs_cpt_table *cfs_cpt_table_create_pattern(const char *pattern) { - struct cfs_cpt_table *cptab; - char *str; - int node = 0; - int ncpt = 0; - int high; - int cpt; - int rc; - int c; - int i; - - str = cfs_trimwhite(pattern); - if (*str == 'n' || *str == 'N') { - pattern = str + 1; - if (*pattern != '\0') { - node = 1; /* numa pattern */ + struct cfs_cpt_table *cptab; + char *pattern_dup; + char *bracket; + char *str; + int node = 0; + int ncpt = 0; + int cpt = 0; + int high; + int rc; + int c; + int i; + + pattern_dup = kstrdup(pattern, GFP_KERNEL); + if (pattern_dup == NULL) { + CERROR("Failed to duplicate pattern '%s'\n", pattern); + return ERR_PTR(-ENOMEM); + } - } else { /* shortcut to create CPT from NUMA & CPU topology */ + str = cfs_trimwhite(pattern_dup); + if (*str == 'n' || *str == 'N') { + str++; /* skip 'N' char */ + node = 1; /* NUMA pattern */ + if (*str == '\0') { node = -1; - ncpt = num_online_nodes(); + for_each_online_node(i) { + if (!cpumask_empty(cpumask_of_node(i))) + ncpt++; + } } } if (ncpt == 0) { /* scanning bracket which is mark of partition */ - for (str = pattern;; str++, ncpt++) { - str = strchr(str, '['); - if (str == NULL) - break; - } + for (bracket = str; bracket != NULL; bracket++, ncpt++) + bracket = strchr(bracket, '['); } if (ncpt == 0 || (node && ncpt > num_online_nodes()) || (!node && ncpt > num_online_cpus())) { - CERROR("Invalid pattern %s, or too many partitions %d\n", - pattern, ncpt); - return NULL; + CERROR("Invalid pattern '%s', or too many partitions %d\n", + pattern_dup, ncpt); + rc = -EINVAL; + goto err_free_str; } cptab = cfs_cpt_table_alloc(ncpt); if (cptab == NULL) { - CERROR("Failed to allocate cpu partition table\n"); - return NULL; + CERROR("Failed to allocate CPU partition table\n"); + rc = -ENOMEM; + goto err_free_str; } if (node < 0) { /* shortcut to create CPT from NUMA & CPU topology */ - cpt = 0; for_each_online_node(i) { - if (cpt >= ncpt) { - CERROR("CPU changed while setting CPU " - "partition table, %d/%d\n", cpt, ncpt); - goto failed; - } + if (cpumask_empty(cpumask_of_node(i))) + continue; rc = cfs_cpt_set_node(cptab, cpt++, i); - if (!rc) - goto failed; + if (!rc) { + rc = -EINVAL; + goto err_free_table; + } } + kfree(pattern_dup); return cptab; } high = node ? nr_node_ids - 1 : nr_cpu_ids - 1; - for (str = cfs_trimwhite(pattern), c = 0;; c++) { - struct cfs_range_expr *range; - struct cfs_expr_list *el; - char *bracket = strchr(str, '['); - int n; + for (str = cfs_trimwhite(str), c = 0; /* until break */; c++) { + struct cfs_range_expr *range; + struct cfs_expr_list *el; + int n; + bracket = strchr(str, '['); if (bracket == NULL) { if (*str != 0) { - CERROR("Invalid pattern %s\n", str); - goto failed; + CERROR("Invalid pattern '%s'\n", str); + rc = -EINVAL; + goto err_free_table; } else if (c != ncpt) { - CERROR("expect %d partitions but found %d\n", - ncpt, c); - goto failed; + CERROR("Expect %d partitions but found %d\n", + ncpt, c); + rc = -EINVAL; + goto err_free_table; } break; } if (sscanf(str, "%d%n", &cpt, &n) < 1) { - CERROR("Invalid cpu pattern %s\n", str); - goto failed; + CERROR("Invalid CPU pattern '%s'\n", str); + rc = -EINVAL; + goto err_free_table; } if (cpt < 0 || cpt >= ncpt) { CERROR("Invalid partition id %d, total partitions %d\n", cpt, ncpt); - goto failed; + rc = -EINVAL; + goto err_free_table; } if (cfs_cpt_weight(cptab, cpt) != 0) { CERROR("Partition %d has already been set.\n", cpt); - goto failed; + rc = -EPERM; + goto err_free_table; } str = cfs_trimwhite(str + n); if (str != bracket) { - CERROR("Invalid pattern %s\n", str); - goto failed; + CERROR("Invalid pattern '%s'\n", str); + rc = -EINVAL; + goto err_free_table; } bracket = strchr(str, ']'); if (bracket == NULL) { - CERROR("missing right bracket for cpt %d, %s\n", - cpt, str); - goto failed; + CERROR("Missing right bracket for partition " + "%d in '%s'\n", cpt, str); + rc = -EINVAL; + goto err_free_table; } - if (cfs_expr_list_parse(str, (bracket - str) + 1, - 0, high, &el) != 0) { - CERROR("Can't parse number range: %s\n", str); - goto failed; + rc = cfs_expr_list_parse(str, (bracket - str) + 1, 0, high, + &el); + if (rc) { + CERROR("Can't parse number range in '%s'\n", str); + rc = -ERANGE; + goto err_free_table; } list_for_each_entry(range, &el->el_exprs, re_link) { @@ -1010,11 +1029,12 @@ cfs_cpt_table_create_pattern(char *pattern) if ((i - range->re_lo) % range->re_stride != 0) continue; - rc = node ? cfs_cpt_set_node(cptab, cpt, i) : - cfs_cpt_set_cpu(cptab, cpt, i); + rc = node ? cfs_cpt_set_node(cptab, cpt, i) + : cfs_cpt_set_cpu(cptab, cpt, i); if (!rc) { cfs_expr_list_free(el); - goto failed; + rc = -EINVAL; + goto err_free_table; } } } @@ -1023,17 +1043,21 @@ cfs_cpt_table_create_pattern(char *pattern) if (!cfs_cpt_online(cptab, cpt)) { CERROR("No online CPU is found on partition %d\n", cpt); - goto failed; + rc = -ENODEV; + goto err_free_table; } str = cfs_trimwhite(bracket + 1); } + kfree(pattern_dup); return cptab; - failed: +err_free_table: cfs_cpt_table_free(cptab); - return NULL; +err_free_str: + kfree(pattern_dup); + return ERR_PTR(rc); } #ifdef CONFIG_HOTPLUG_CPU @@ -1092,7 +1116,7 @@ static struct notifier_block cfs_cpu_notifier = { void cfs_cpu_fini(void) { - if (cfs_cpt_table != NULL) + if (!IS_ERR_OR_NULL(cfs_cpt_table)) cfs_cpt_table_free(cfs_cpt_table); #ifdef CONFIG_HOTPLUG_CPU @@ -1133,34 +1157,28 @@ int cfs_cpu_init(void) get_online_cpus(); if (*cpu_pattern != 0) { - char *cpu_pattern_dup = kstrdup(cpu_pattern, GFP_KERNEL); - - if (cpu_pattern_dup == NULL) { - CERROR("Failed to duplicate cpu_pattern\n"); - goto failed; - } - - cfs_cpt_table = cfs_cpt_table_create_pattern(cpu_pattern_dup); - kfree(cpu_pattern_dup); - if (cfs_cpt_table == NULL) { - CERROR("Failed to create cptab from pattern %s\n", - cpu_pattern); + cfs_cpt_table = cfs_cpt_table_create_pattern(cpu_pattern); + if (IS_ERR(cfs_cpt_table)) { + CERROR("Failed to create cptab from pattern '%s'\n", + cpu_pattern); + ret = PTR_ERR(cfs_cpt_table); goto failed; } } else { cfs_cpt_table = cfs_cpt_table_create(cpu_npartitions); - if (cfs_cpt_table == NULL) { - CERROR("Failed to create ptable with npartitions %d\n", - cpu_npartitions); + if (IS_ERR(cfs_cpt_table)) { + CERROR("Failed to create cptab with npartitions %d\n", + cpu_npartitions); + ret = PTR_ERR(cfs_cpt_table); goto failed; } } put_online_cpus(); - LCONSOLE(0, "HW nodes: %d, HW CPU cores: %d, npartitions: %d\n", - num_online_nodes(), num_online_cpus(), - cfs_cpt_number(cfs_cpt_table)); + LCONSOLE(0, "HW NUMA nodes: %d, HW CPU cores: %d, npartitions: %d\n", + num_online_nodes(), num_online_cpus(), + cfs_cpt_number(cfs_cpt_table)); return 0; failed: -- 1.8.3.1