[lustre-devel] [PATCH v3 07/26] staging: lustre: libcfs: NUMA support
NeilBrown
neilb at suse.com
Sun Jun 24 17:39:31 PDT 2018
On Sun, Jun 24 2018, James Simmons wrote:
> From: Amir Shehata <amir.shehata at intel.com>
>
> 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.
I note that you didn't respond to Greg's questions about this patch.
I'll accept it anyway in the interests of moving forward, but I think
his comments were probably valid, and need to be considered at some
stage.
There is a bug though....
>
> Signed-off-by: Amir Shehata <amir.shehata at intel.com>
> WC-bug-id: https://jira.whamcloud.com/browse/LU-7734
> Reviewed-on: http://review.whamcloud.com/18916
> Reviewed-by: Olaf Weber <olaf at sgi.com>
> Reviewed-by: Doug Oucharek <dougso at me.com>
> Signed-off-by: James Simmons <jsimmons at infradead.org>
> ---
> .../lustre/include/linux/libcfs/libcfs_cpu.h | 11 +++++++++++
> drivers/staging/lustre/lnet/libcfs/libcfs_cpu.c | 21 +++++++++++++++++++++
> 2 files changed, 32 insertions(+)
>
> diff --git a/drivers/staging/lustre/include/linux/libcfs/libcfs_cpu.h b/drivers/staging/lustre/include/linux/libcfs/libcfs_cpu.h
> index 1b4333d..ff3ecf5 100644
> --- a/drivers/staging/lustre/include/linux/libcfs/libcfs_cpu.h
> +++ b/drivers/staging/lustre/include/linux/libcfs/libcfs_cpu.h
> @@ -103,6 +103,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;
> };
> @@ -143,6 +145,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);
> @@ -299,6 +305,11 @@ void cfs_cpt_unset_nodemask(struct cfs_cpt_table *cptab,
> return 0;
> }
>
> +static inline int cfs_cpt_of_node(struct cfs_cpt_table *cptab, int node)
> +{
> + return 0;
> +}
> +
> static inline int
> cfs_cpt_bind(struct cfs_cpt_table *cptab, int cpt)
> {
> diff --git a/drivers/staging/lustre/lnet/libcfs/libcfs_cpu.c b/drivers/staging/lustre/lnet/libcfs/libcfs_cpu.c
> index 33294da..8c5cf7b 100644
> --- a/drivers/staging/lustre/lnet/libcfs/libcfs_cpu.c
> +++ b/drivers/staging/lustre/lnet/libcfs/libcfs_cpu.c
> @@ -102,6 +102,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_alloc_node2cpt;
> +
> + 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)
> @@ -133,6 +142,8 @@ struct cfs_cpt_table *
>
> kvfree(cptab->ctb_parts);
> failed_alloc_ctb_parts:
> + kvfree(cptab->ctb_node2cpt);
> +failed_alloc_node2cpt:
> kvfree(cptab->ctb_cpu2cpt);
> failed_alloc_cpu2cpt:
> kfree(cptab->ctb_nodemask);
> @@ -150,6 +161,7 @@ struct cfs_cpt_table *
> 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 = &cptab->ctb_parts[i];
> @@ -515,6 +527,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];
> +}
So if node == nr_node_ids, we access beyond the end of the ctb_node2cpt array.
Oops.
I've fixed this before applying.
Thanks,
NeilBrown
> +EXPORT_SYMBOL(cfs_cpt_of_node);
> +
> int
> cfs_cpt_bind(struct cfs_cpt_table *cptab, int cpt)
> {
> --
> 1.8.3.1
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 832 bytes
Desc: not available
URL: <http://lists.lustre.org/pipermail/lustre-devel-lustre.org/attachments/20180625/770edf72/attachment.sig>
More information about the lustre-devel
mailing list