Re: [PATCH 08/25] staging: lustre: libcfs: add cpu distance handling

2018-04-16 Thread Dan Carpenter 
On Mon, Apr 16, 2018 at 12:09:50AM -0400, James Simmons wrote:
> +int cfs_cpt_distance_print(struct cfs_cpt_table *cptab, char *buf, int len)
> +{
> + char *tmp = buf;
> + int rc = -EFBIG;
> + int i;
> + int j;
> +
> + for (i = 0; i < cptab->ctb_nparts; i++) {
> + if (len <= 0)
> + goto err;
> +
> + rc = snprintf(tmp, len, "%d\t:", i);
> + len -= rc;
> +
> + if (len <= 0)
> + goto err;


The "goto err" here is sort of an example of a "do-nothing" goto.  There
are no resources to free or anything like that.

I don't like do-nothing gotos because "return -EFBIG;" is self
documenting code and "goto err;" is totally ambiguous what it does.  The
second problem is that do-nothing error labels easily turn into
do-everything one err style error paths which I loath.  And the third
problem is that they introduce "forgot to set the error code" bugs.

It looks like we forgot to set the error code here although it's also
possible that was intended.  This code is ambiguous.

> +
> + tmp += rc;
> + for (j = 0; j < cptab->ctb_nparts; j++) {
> + rc = snprintf(tmp, len, " %d:%d",
> +   j, cptab->ctb_parts[i].cpt_distance[j]);
> + len -= rc;
> + if (len <= 0)
> + goto err;
> + tmp += rc;
> + }
> +
> + *tmp = '\n';
> + tmp++;
> + len--;
> + }
> + rc = 0;
> +err:
> + if (rc < 0)
> + return rc;
> +
> + return tmp - buf;
> +}

regards,
dan carpenter

Re: [PATCH 08/25] staging: lustre: libcfs: add cpu distance handling

2018-04-16 Thread Dan Carpenter 
On Mon, Apr 16, 2018 at 12:09:50AM -0400, James Simmons wrote:
> +int cfs_cpt_distance_print(struct cfs_cpt_table *cptab, char *buf, int len)
> +{
> + char *tmp = buf;
> + int rc = -EFBIG;
> + int i;
> + int j;
> +
> + for (i = 0; i < cptab->ctb_nparts; i++) {
> + if (len <= 0)
> + goto err;
> +
> + rc = snprintf(tmp, len, "%d\t:", i);
> + len -= rc;
> +
> + if (len <= 0)
> + goto err;


The "goto err" here is sort of an example of a "do-nothing" goto.  There
are no resources to free or anything like that.

I don't like do-nothing gotos because "return -EFBIG;" is self
documenting code and "goto err;" is totally ambiguous what it does.  The
second problem is that do-nothing error labels easily turn into
do-everything one err style error paths which I loath.  And the third
problem is that they introduce "forgot to set the error code" bugs.

It looks like we forgot to set the error code here although it's also
possible that was intended.  This code is ambiguous.

> +
> + tmp += rc;
> + for (j = 0; j < cptab->ctb_nparts; j++) {
> + rc = snprintf(tmp, len, " %d:%d",
> +   j, cptab->ctb_parts[i].cpt_distance[j]);
> + len -= rc;
> + if (len <= 0)
> + goto err;
> + tmp += rc;
> + }
> +
> + *tmp = '\n';
> + tmp++;
> + len--;
> + }
> + rc = 0;
> +err:
> + if (rc < 0)
> + return rc;
> +
> + return tmp - buf;
> +}

regards,
dan carpenter

[PATCH 08/25] staging: lustre: libcfs: add cpu distance handling

2018-04-15 Thread James Simmons 
From: Amir Shehata 

Add functionality to calculate the distance between two CPTs.
Expose those distance in debugfs so people deploying a setup
can debug what is being created for CPTs.

Signed-off-by: Amir Shehata 
Intel-bug-id: https://jira.hpdd.intel.com/browse/LU-7734
Reviewed-on: http://review.whamcloud.com/18916
Reviewed-by: Olaf Weber 
Reviewed-by: Doug Oucharek 
Signed-off-by: James Simmons 
---
 .../lustre/include/linux/libcfs/libcfs_cpu.h   |  8 +++
 .../lustre/include/linux/libcfs/linux/linux-cpu.h  |  4 ++
 drivers/staging/lustre/lnet/libcfs/libcfs_cpu.c| 21 
 .../staging/lustre/lnet/libcfs/linux/linux-cpu.c   | 59 ++
 4 files changed, 92 insertions(+)

diff --git a/drivers/staging/lustre/include/linux/libcfs/libcfs_cpu.h 
b/drivers/staging/lustre/include/linux/libcfs/libcfs_cpu.h
index 839ec02..c0922fc 100644
--- a/drivers/staging/lustre/include/linux/libcfs/libcfs_cpu.h
+++ b/drivers/staging/lustre/include/linux/libcfs/libcfs_cpu.h
@@ -110,6 +110,10 @@ struct cfs_cpt_table {
  */
 struct cfs_cpt_table *cfs_cpt_table_alloc(unsigned int ncpt);
 /**
+ * print distance information of cpt-table
+ */
+int cfs_cpt_distance_print(struct cfs_cpt_table *cptab, char *buf, int len);
+/**
  * return total number of CPU partitions in \a cptab
  */
 int
@@ -143,6 +147,10 @@ struct cfs_cpt_table {
  */
 int cfs_cpt_of_node(struct cfs_cpt_table *cptab, int node);
 /**
+ * NUMA distance between \a cpt1 and \a cpt2 in \a cptab
+ */
+unsigned int cfs_cpt_distance(struct cfs_cpt_table *cptab, int cpt1, int cpt2);
+/**
  * bind current thread on a CPU-partition \a cpt of \a cptab
  */
 int cfs_cpt_bind(struct cfs_cpt_table *cptab, int cpt);
diff --git a/drivers/staging/lustre/include/linux/libcfs/linux/linux-cpu.h 
b/drivers/staging/lustre/include/linux/libcfs/linux/linux-cpu.h
index 1bed0ba..4ac1670 100644
--- a/drivers/staging/lustre/include/linux/libcfs/linux/linux-cpu.h
+++ b/drivers/staging/lustre/include/linux/libcfs/linux/linux-cpu.h
@@ -52,6 +52,8 @@ struct cfs_cpu_partition {
cpumask_var_t   cpt_cpumask;
/* nodes mask for this partition */
nodemask_t  *cpt_nodemask;
+   /* NUMA distance between CPTs */
+   unsigned int*cpt_distance;
/* spread rotor for NUMA allocator */
unsigned intcpt_spread_rotor;
 };
@@ -60,6 +62,8 @@ struct cfs_cpu_partition {
 struct cfs_cpt_table {
/* spread rotor for NUMA allocator */
unsigned intctb_spread_rotor;
+   /* maximum NUMA distance between all nodes in table */
+   unsigned intctb_distance;
/* # of CPU partitions */
unsigned intctb_nparts;
/* partitions tables */
diff --git a/drivers/staging/lustre/lnet/libcfs/libcfs_cpu.c 
b/drivers/staging/lustre/lnet/libcfs/libcfs_cpu.c
index e6d1512..7ac2796 100644
--- a/drivers/staging/lustre/lnet/libcfs/libcfs_cpu.c
+++ b/drivers/staging/lustre/lnet/libcfs/libcfs_cpu.c
@@ -41,6 +41,8 @@
 
 #define CFS_CPU_VERSION_MAGIC 0xbabecafe
 
+#define CFS_CPT_DISTANCE   1   /* Arbitrary positive value */
+
 struct cfs_cpt_table *
 cfs_cpt_table_alloc(unsigned int ncpt)
 {
@@ -90,6 +92,19 @@ struct cfs_cpt_table *
 EXPORT_SYMBOL(cfs_cpt_table_print);
 #endif /* CONFIG_SMP */
 
+int cfs_cpt_distance_print(struct cfs_cpt_table *cptab, char *buf, int len)
+{
+   int rc;
+
+   rc = snprintf(buf, len, "0\t: 0:%d\n", CFS_CPT_DISTANCE);
+   len -= rc;
+   if (len <= 0)
+   return -EFBIG;
+
+   return rc;
+}
+EXPORT_SYMBOL(cfs_cpt_distance_print);
+
 int
 cfs_cpt_number(struct cfs_cpt_table *cptab)
 {
@@ -124,6 +139,12 @@ cpumask_var_t *cfs_cpt_cpumask(struct cfs_cpt_table 
*cptab, int cpt)
 }
 EXPORT_SYMBOL(cfs_cpt_nodemask);
 
+unsigned int cfs_cpt_distance(struct cfs_cpt_table *cptab, int cpt1, int cpt2)
+{
+   return CFS_CPT_DISTANCE;
+}
+EXPORT_SYMBOL(cfs_cpt_distance);
+
 int
 cfs_cpt_set_cpu(struct cfs_cpt_table *cptab, int cpt, int cpu)
 {
diff --git a/drivers/staging/lustre/lnet/libcfs/linux/linux-cpu.c 
b/drivers/staging/lustre/lnet/libcfs/linux/linux-cpu.c
index fd0c451..1e184b1 100644
--- a/drivers/staging/lustre/lnet/libcfs/linux/linux-cpu.c
+++ b/drivers/staging/lustre/lnet/libcfs/linux/linux-cpu.c
@@ -76,6 +76,7 @@
struct cfs_cpu_partition *part = >ctb_parts[i];
 
kfree(part->cpt_nodemask);
+   kfree(part->cpt_distance);
free_cpumask_var(part->cpt_cpumask);
}
 
@@ -137,6 +138,12 @@ struct cfs_cpt_table *
if (!zalloc_cpumask_var(>cpt_cpumask, GFP_NOFS) ||
!part->cpt_nodemask)
goto failed;
+
+   part->cpt_distance = kvmalloc_array(cptab->ctb_nparts,
+

[PATCH 08/25] staging: lustre: libcfs: add cpu distance handling

2018-04-15 Thread James Simmons 
From: Amir Shehata 

Add functionality to calculate the distance between two CPTs.
Expose those distance in debugfs so people deploying a setup
can debug what is being created for CPTs.

Signed-off-by: Amir Shehata 
Intel-bug-id: https://jira.hpdd.intel.com/browse/LU-7734
Reviewed-on: http://review.whamcloud.com/18916
Reviewed-by: Olaf Weber 
Reviewed-by: Doug Oucharek 
Signed-off-by: James Simmons 
---
 .../lustre/include/linux/libcfs/libcfs_cpu.h   |  8 +++
 .../lustre/include/linux/libcfs/linux/linux-cpu.h  |  4 ++
 drivers/staging/lustre/lnet/libcfs/libcfs_cpu.c| 21 
 .../staging/lustre/lnet/libcfs/linux/linux-cpu.c   | 59 ++
 4 files changed, 92 insertions(+)

diff --git a/drivers/staging/lustre/include/linux/libcfs/libcfs_cpu.h 
b/drivers/staging/lustre/include/linux/libcfs/libcfs_cpu.h
index 839ec02..c0922fc 100644
--- a/drivers/staging/lustre/include/linux/libcfs/libcfs_cpu.h
+++ b/drivers/staging/lustre/include/linux/libcfs/libcfs_cpu.h
@@ -110,6 +110,10 @@ struct cfs_cpt_table {
  */
 struct cfs_cpt_table *cfs_cpt_table_alloc(unsigned int ncpt);
 /**
+ * print distance information of cpt-table
+ */
+int cfs_cpt_distance_print(struct cfs_cpt_table *cptab, char *buf, int len);
+/**
  * return total number of CPU partitions in \a cptab
  */
 int
@@ -143,6 +147,10 @@ struct cfs_cpt_table {
  */
 int cfs_cpt_of_node(struct cfs_cpt_table *cptab, int node);
 /**
+ * NUMA distance between \a cpt1 and \a cpt2 in \a cptab
+ */
+unsigned int cfs_cpt_distance(struct cfs_cpt_table *cptab, int cpt1, int cpt2);
+/**
  * bind current thread on a CPU-partition \a cpt of \a cptab
  */
 int cfs_cpt_bind(struct cfs_cpt_table *cptab, int cpt);
diff --git a/drivers/staging/lustre/include/linux/libcfs/linux/linux-cpu.h 
b/drivers/staging/lustre/include/linux/libcfs/linux/linux-cpu.h
index 1bed0ba..4ac1670 100644
--- a/drivers/staging/lustre/include/linux/libcfs/linux/linux-cpu.h
+++ b/drivers/staging/lustre/include/linux/libcfs/linux/linux-cpu.h
@@ -52,6 +52,8 @@ struct cfs_cpu_partition {
cpumask_var_t   cpt_cpumask;
/* nodes mask for this partition */
nodemask_t  *cpt_nodemask;
+   /* NUMA distance between CPTs */
+   unsigned int*cpt_distance;
/* spread rotor for NUMA allocator */
unsigned intcpt_spread_rotor;
 };
@@ -60,6 +62,8 @@ struct cfs_cpu_partition {
 struct cfs_cpt_table {
/* spread rotor for NUMA allocator */
unsigned intctb_spread_rotor;
+   /* maximum NUMA distance between all nodes in table */
+   unsigned intctb_distance;
/* # of CPU partitions */
unsigned intctb_nparts;
/* partitions tables */
diff --git a/drivers/staging/lustre/lnet/libcfs/libcfs_cpu.c 
b/drivers/staging/lustre/lnet/libcfs/libcfs_cpu.c
index e6d1512..7ac2796 100644
--- a/drivers/staging/lustre/lnet/libcfs/libcfs_cpu.c
+++ b/drivers/staging/lustre/lnet/libcfs/libcfs_cpu.c
@@ -41,6 +41,8 @@
 
 #define CFS_CPU_VERSION_MAGIC 0xbabecafe
 
+#define CFS_CPT_DISTANCE   1   /* Arbitrary positive value */
+
 struct cfs_cpt_table *
 cfs_cpt_table_alloc(unsigned int ncpt)
 {
@@ -90,6 +92,19 @@ struct cfs_cpt_table *
 EXPORT_SYMBOL(cfs_cpt_table_print);
 #endif /* CONFIG_SMP */
 
+int cfs_cpt_distance_print(struct cfs_cpt_table *cptab, char *buf, int len)
+{
+   int rc;
+
+   rc = snprintf(buf, len, "0\t: 0:%d\n", CFS_CPT_DISTANCE);
+   len -= rc;
+   if (len <= 0)
+   return -EFBIG;
+
+   return rc;
+}
+EXPORT_SYMBOL(cfs_cpt_distance_print);
+
 int
 cfs_cpt_number(struct cfs_cpt_table *cptab)
 {
@@ -124,6 +139,12 @@ cpumask_var_t *cfs_cpt_cpumask(struct cfs_cpt_table 
*cptab, int cpt)
 }
 EXPORT_SYMBOL(cfs_cpt_nodemask);
 
+unsigned int cfs_cpt_distance(struct cfs_cpt_table *cptab, int cpt1, int cpt2)
+{
+   return CFS_CPT_DISTANCE;
+}
+EXPORT_SYMBOL(cfs_cpt_distance);
+
 int
 cfs_cpt_set_cpu(struct cfs_cpt_table *cptab, int cpt, int cpu)
 {
diff --git a/drivers/staging/lustre/lnet/libcfs/linux/linux-cpu.c 
b/drivers/staging/lustre/lnet/libcfs/linux/linux-cpu.c
index fd0c451..1e184b1 100644
--- a/drivers/staging/lustre/lnet/libcfs/linux/linux-cpu.c
+++ b/drivers/staging/lustre/lnet/libcfs/linux/linux-cpu.c
@@ -76,6 +76,7 @@
struct cfs_cpu_partition *part = >ctb_parts[i];
 
kfree(part->cpt_nodemask);
+   kfree(part->cpt_distance);
free_cpumask_var(part->cpt_cpumask);
}
 
@@ -137,6 +138,12 @@ struct cfs_cpt_table *
if (!zalloc_cpumask_var(>cpt_cpumask, GFP_NOFS) ||
!part->cpt_nodemask)
goto failed;
+
+   part->cpt_distance = kvmalloc_array(cptab->ctb_nparts,
+   
sizeof(part->cpt_distance[0]),
+