Re: [PATCH 07/25] staging: lustre: libcfs: NUMA support

2018-04-16 Thread Dan Carpenter 
On Mon, Apr 16, 2018 at 12:09:49AM -0400, James Simmons wrote:
> @@ -114,6 +115,15 @@ struct cfs_cpt_table *
>   memset(cptab->ctb_cpu2cpt, -1,
>  nr_cpu_ids * sizeof(cptab->ctb_cpu2cpt[0]));
>  
> + cptab->ctb_node2cpt = kvmalloc_array(nr_node_ids,
> +  sizeof(cptab->ctb_node2cpt[0]),
> +  GFP_KERNEL);
> + if (!cptab->ctb_node2cpt)
> + goto failed;
> +
> + memset(cptab->ctb_node2cpt, -1,
> +nr_node_ids * sizeof(cptab->ctb_node2cpt[0]));
> +
>   cptab->ctb_parts = kvmalloc_array(ncpt, sizeof(cptab->ctb_parts[0]),
> GFP_KERNEL);
>   if (!cptab->ctb_parts)


You didn't introduce this, but I was explaining earlier that you should
always be suspicious of code which does "goto failed".  The bug here is
that cptab->ctb_parts is allocated with kvmalloc_array() which
doesn't zero out the memory.  So if we only initialize it part way
because art->cpt_nodemask = kzalloc() fails or something then it's
problem:

91  void
92  cfs_cpt_table_free(struct cfs_cpt_table *cptab)
93  {
94  int i;
95  
96  kvfree(cptab->ctb_cpu2cpt);
97  
98  for (i = 0; cptab->ctb_parts && i < cptab->ctb_nparts; i++) {
99  struct cfs_cpu_partition *part = >ctb_parts[i];
   100  
   101  kfree(part->cpt_nodemask);
 ^^^
   102  free_cpumask_var(part->cpt_cpumask);
 ^
These are uninitialized so it will crash.  It turns out there isn't a
kvcalloc() or kvzalloc_array() function.  We don't seem to have a
vcalloc() either...  Very strange.

   103  }
   104  
   105  kvfree(cptab->ctb_parts);
   106  
   107  kfree(cptab->ctb_nodemask);
   108  free_cpumask_var(cptab->ctb_cpumask);
   109  
   110  kfree(cptab);
   111  }

regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 07/25] staging: lustre: libcfs: NUMA support

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

This patch adds NUMA node support. NUMA node information is stored
in the CPT table. A NUMA node mask is maintained for the entire
table as well as for each CPT to track the NUMA nodes related to
each of the CPTs. Add new function cfs_cpt_of_node() which returns
the CPT of a particular NUMA node.

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 
---
 .../staging/lustre/include/linux/libcfs/libcfs_cpu.h  |  4 
 .../lustre/include/linux/libcfs/linux/linux-cpu.h |  2 ++
 drivers/staging/lustre/lnet/libcfs/libcfs_cpu.c   |  6 ++
 drivers/staging/lustre/lnet/libcfs/linux/linux-cpu.c  | 19 +++
 4 files changed, 31 insertions(+)

diff --git a/drivers/staging/lustre/include/linux/libcfs/libcfs_cpu.h 
b/drivers/staging/lustre/include/linux/libcfs/libcfs_cpu.h
index 070f8fe..839ec02 100644
--- a/drivers/staging/lustre/include/linux/libcfs/libcfs_cpu.h
+++ b/drivers/staging/lustre/include/linux/libcfs/libcfs_cpu.h
@@ -139,6 +139,10 @@ struct cfs_cpt_table {
  */
 int cfs_cpt_of_cpu(struct cfs_cpt_table *cptab, int cpu);
 /**
+ * shadow HW node ID \a NODE to CPU-partition ID by \a cptab
+ */
+int cfs_cpt_of_node(struct cfs_cpt_table *cptab, int node);
+/**
  * 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 e8bbbaa..1bed0ba 100644
--- a/drivers/staging/lustre/include/linux/libcfs/linux/linux-cpu.h
+++ b/drivers/staging/lustre/include/linux/libcfs/linux/linux-cpu.h
@@ -68,6 +68,8 @@ struct cfs_cpt_table {
int *ctb_cpu2cpt;
/* all cpus in this partition table */
cpumask_var_t   ctb_cpumask;
+   /* shadow HW node to CPU partition ID */
+   int *ctb_node2cpt;
/* all nodes in this partition table */
nodemask_t  *ctb_nodemask;
 };
diff --git a/drivers/staging/lustre/lnet/libcfs/libcfs_cpu.c 
b/drivers/staging/lustre/lnet/libcfs/libcfs_cpu.c
index 5ea294f..e6d1512 100644
--- a/drivers/staging/lustre/lnet/libcfs/libcfs_cpu.c
+++ b/drivers/staging/lustre/lnet/libcfs/libcfs_cpu.c
@@ -198,6 +198,12 @@ cpumask_var_t *cfs_cpt_cpumask(struct cfs_cpt_table 
*cptab, int cpt)
 }
 EXPORT_SYMBOL(cfs_cpt_of_cpu);
 
+int cfs_cpt_of_node(struct cfs_cpt_table *cptab, int node)
+{
+   return 0;
+}
+EXPORT_SYMBOL(cfs_cpt_of_node);
+
 int
 cfs_cpt_bind(struct cfs_cpt_table *cptab, int cpt)
 {
diff --git a/drivers/staging/lustre/lnet/libcfs/linux/linux-cpu.c 
b/drivers/staging/lustre/lnet/libcfs/linux/linux-cpu.c
index 741db69..fd0c451 100644
--- a/drivers/staging/lustre/lnet/libcfs/linux/linux-cpu.c
+++ b/drivers/staging/lustre/lnet/libcfs/linux/linux-cpu.c
@@ -70,6 +70,7 @@
int i;
 
kvfree(cptab->ctb_cpu2cpt);
+   kvfree(cptab->ctb_node2cpt);
 
for (i = 0; cptab->ctb_parts && i < cptab->ctb_nparts; i++) {
struct cfs_cpu_partition *part = >ctb_parts[i];
@@ -114,6 +115,15 @@ struct cfs_cpt_table *
memset(cptab->ctb_cpu2cpt, -1,
   nr_cpu_ids * sizeof(cptab->ctb_cpu2cpt[0]));
 
+   cptab->ctb_node2cpt = kvmalloc_array(nr_node_ids,
+sizeof(cptab->ctb_node2cpt[0]),
+GFP_KERNEL);
+   if (!cptab->ctb_node2cpt)
+   goto failed;
+
+   memset(cptab->ctb_node2cpt, -1,
+  nr_node_ids * sizeof(cptab->ctb_node2cpt[0]));
+
cptab->ctb_parts = kvmalloc_array(ncpt, sizeof(cptab->ctb_parts[0]),
  GFP_KERNEL);
if (!cptab->ctb_parts)
@@ -484,6 +494,15 @@ struct cfs_cpt_table *
 }
 EXPORT_SYMBOL(cfs_cpt_of_cpu);
 
+int cfs_cpt_of_node(struct cfs_cpt_table *cptab, int node)
+{
+   if (node < 0 || node > nr_node_ids)
+   return CFS_CPT_ANY;
+
+   return cptab->ctb_node2cpt[node];
+}
+EXPORT_SYMBOL(cfs_cpt_of_node);
+
 int
 cfs_cpt_bind(struct cfs_cpt_table *cptab, int cpt)
 {
-- 
1.8.3.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel