[lustre-devel] [PATCH v3 25/26] staging: lustre: libcfs: make cfs_cpt_tab a static structure

NeilBrown neilb at suse.com
Sun Jun 24 18:32:38 PDT 2018


On Sun, Jun 24 2018, James Simmons wrote:

> Only one cfs_cpt_tab exist and its created only at libcfs modules
> loading and removal. Instead of dynamically allocating it lets
> statically allocate it. This will help to reenable UMP support.
>
> WC-bug-id: https://jira.whamcloud.com/browse/LU-9856
> Signed-off-by: James Simmons <jsimmons at infradead.org>

While this patch is quite possibly a good idea, I'm not applying it or
the following one until you explain what is currently broken with "UMP"
support (I assume you mean UP - uni-processor).

Again, the WC-bug-id you linked gives no useful hint.

Thanks,
NeilBrown


> ---
>  .../lustre/include/linux/libcfs/libcfs_cpu.h       |   4 +-
>  drivers/staging/lustre/lnet/libcfs/libcfs_cpu.c    | 111 ++++++++++-----------
>  drivers/staging/lustre/lnet/libcfs/module.c        |  10 +-
>  drivers/staging/lustre/lnet/lnet/api-ni.c          |   4 +-
>  drivers/staging/lustre/lnet/selftest/framework.c   |   2 +-
>  drivers/staging/lustre/lustre/ptlrpc/client.c      |   4 +-
>  drivers/staging/lustre/lustre/ptlrpc/ptlrpcd.c     |  10 +-
>  drivers/staging/lustre/lustre/ptlrpc/service.c     |   4 +-
>  8 files changed, 73 insertions(+), 76 deletions(-)
>
> diff --git a/drivers/staging/lustre/include/linux/libcfs/libcfs_cpu.h b/drivers/staging/lustre/include/linux/libcfs/libcfs_cpu.h
> index 32776d2..df7e16b 100644
> --- a/drivers/staging/lustre/include/linux/libcfs/libcfs_cpu.h
> +++ b/drivers/staging/lustre/include/linux/libcfs/libcfs_cpu.h
> @@ -115,7 +115,7 @@ struct cfs_cpt_table {
>  	nodemask_t			*ctb_nodemask;
>  };
>  
> -extern struct cfs_cpt_table	*cfs_cpt_tab;
> +extern struct cfs_cpt_table	cfs_cpt_tab;
>  
>  /**
>   * return cpumask of CPU partition \a cpt
> @@ -215,8 +215,6 @@ void cfs_cpt_unset_nodemask(struct cfs_cpt_table *cptab,
>  void cfs_cpu_fini(void);
>  
>  #else /* !CONFIG_SMP */
> -struct cfs_cpt_table;
> -#define cfs_cpt_tab ((struct cfs_cpt_table *)NULL)
>  
>  static inline int cfs_cpt_table_print(struct cfs_cpt_table *cptab,
>  				      char *buf, int len)
> diff --git a/drivers/staging/lustre/lnet/libcfs/libcfs_cpu.c b/drivers/staging/lustre/lnet/libcfs/libcfs_cpu.c
> index 3f4a7c7..9fd324d 100644
> --- a/drivers/staging/lustre/lnet/libcfs/libcfs_cpu.c
> +++ b/drivers/staging/lustre/lnet/libcfs/libcfs_cpu.c
> @@ -41,10 +41,6 @@
>  #include <linux/libcfs/libcfs_string.h>
>  #include <linux/libcfs/libcfs.h>
>  
> -/** Global CPU partition table */
> -struct cfs_cpt_table   *cfs_cpt_tab __read_mostly;
> -EXPORT_SYMBOL(cfs_cpt_tab);
> -
>  /**
>   * modparam for setting number of partitions
>   *
> @@ -73,15 +69,10 @@
>  module_param(cpu_pattern, charp, 0444);
>  MODULE_PARM_DESC(cpu_pattern, "CPU partitions pattern");
>  
> -struct cfs_cpt_table *cfs_cpt_table_alloc(int ncpt)
> +static int cfs_cpt_table_setup(struct cfs_cpt_table *cptab, int ncpt)
>  {
> -	struct cfs_cpt_table *cptab;
>  	int i;
>  
> -	cptab = kzalloc(sizeof(*cptab), GFP_NOFS);
> -	if (!cptab)
> -		return NULL;
> -
>  	cptab->ctb_nparts = ncpt;
>  
>  	if (!zalloc_cpumask_var(&cptab->ctb_cpumask, GFP_NOFS))
> @@ -138,7 +129,7 @@ struct cfs_cpt_table *cfs_cpt_table_alloc(int ncpt)
>  		       cptab->ctb_nparts * sizeof(part->cpt_distance[0]));
>  	}
>  
> -	return cptab;
> +	return 0;
>  
>  failed_setting_ctb_parts:
>  	while (i-- >= 0) {
> @@ -159,8 +150,24 @@ struct cfs_cpt_table *cfs_cpt_table_alloc(int ncpt)
>  failed_alloc_nodemask:
>  	free_cpumask_var(cptab->ctb_cpumask);
>  failed_alloc_cpumask:
> -	kfree(cptab);
> -	return NULL;
> +	return -ENOMEM;
> +}
> +
> +struct cfs_cpt_table *cfs_cpt_table_alloc(int ncpt)
> +{
> +	struct cfs_cpt_table *cptab;
> +	int rc;
> +
> +	cptab = kzalloc(sizeof(*cptab), GFP_NOFS);
> +	if (!cptab)
> +		return NULL;
> +
> +	rc = cfs_cpt_table_setup(cptab, ncpt);
> +	if (rc) {
> +		kfree(cptab);
> +		cptab = NULL;
> +	}
> +	return cptab;
>  }
>  EXPORT_SYMBOL(cfs_cpt_table_alloc);
>  
> @@ -183,8 +190,6 @@ void cfs_cpt_table_free(struct cfs_cpt_table *cptab)
>  
>  	kfree(cptab->ctb_nodemask);
>  	free_cpumask_var(cptab->ctb_cpumask);
> -
> -	kfree(cptab);
>  }
>  EXPORT_SYMBOL(cfs_cpt_table_free);
>  
> @@ -822,9 +827,8 @@ static int cfs_cpt_num_estimate(void)
>  	return ncpt;
>  }
>  
> -static struct cfs_cpt_table *cfs_cpt_table_create(int ncpt)
> +static int cfs_cpt_table_create(int ncpt)
>  {
> -	struct cfs_cpt_table *cptab = NULL;
>  	cpumask_var_t node_mask;
>  	int cpt = 0;
>  	int node;
> @@ -841,10 +845,9 @@ static struct cfs_cpt_table *cfs_cpt_table_create(int ncpt)
>  		      ncpt, num);
>  	}
>  
> -	cptab = cfs_cpt_table_alloc(ncpt);
> -	if (!cptab) {
> -		CERROR("Failed to allocate CPU map(%d)\n", ncpt);
> -		rc = -ENOMEM;
> +	rc = cfs_cpt_table_setup(&cfs_cpt_tab, ncpt);
> +	if (rc) {
> +		CERROR("Failed to setup CPU map(%d)\n", ncpt);
>  		goto failed;
>  	}
>  
> @@ -860,10 +863,13 @@ static struct cfs_cpt_table *cfs_cpt_table_create(int ncpt)
>  		cpumask_copy(node_mask, cpumask_of_node(node));
>  
>  		while (cpt < ncpt && !cpumask_empty(node_mask)) {
> -			struct cfs_cpu_partition *part = &cptab->ctb_parts[cpt];
> -			int ncpu = cpumask_weight(part->cpt_cpumask);
> +			struct cfs_cpu_partition *part;
> +			int ncpu;
> +
> +			part = &cfs_cpt_tab.ctb_parts[cpt];
> +			ncpu = cpumask_weight(part->cpt_cpumask);
>  
> -			rc = cfs_cpt_choose_ncpus(cptab, cpt, node_mask,
> +			rc = cfs_cpt_choose_ncpus(&cfs_cpt_tab, cpt, node_mask,
>  						  num - ncpu);
>  			if (rc < 0) {
>  				rc = -EINVAL;
> @@ -880,7 +886,7 @@ static struct cfs_cpt_table *cfs_cpt_table_create(int ncpt)
>  
>  	free_cpumask_var(node_mask);
>  
> -	return cptab;
> +	return 0;
>  
>  failed_mask:
>  	free_cpumask_var(node_mask);
> @@ -888,15 +894,13 @@ static struct cfs_cpt_table *cfs_cpt_table_create(int ncpt)
>  	CERROR("Failed (rc = %d) to setup CPU partition table with %d partitions, online HW NUMA nodes: %d, HW CPU cores: %d.\n",
>  	       rc, ncpt, num_online_nodes(), num_online_cpus());
>  
> -	if (cptab)
> -		cfs_cpt_table_free(cptab);
> +	cfs_cpt_table_free(&cfs_cpt_tab);
>  
> -	return ERR_PTR(rc);
> +	return rc;
>  }
>  
> -static struct cfs_cpt_table *cfs_cpt_table_create_pattern(const char *pattern)
> +static int cfs_cpt_table_create_pattern(const char *pattern)
>  {
> -	struct cfs_cpt_table *cptab;
>  	char *pattern_dup;
>  	char *bracket;
>  	char *str;
> @@ -911,7 +915,7 @@ static struct cfs_cpt_table *cfs_cpt_table_create_pattern(const char *pattern)
>  	pattern_dup = kstrdup(pattern, GFP_KERNEL);
>  	if (!pattern_dup) {
>  		CERROR("Failed to duplicate pattern '%s'\n", pattern);
> -		return ERR_PTR(-ENOMEM);
> +		return -ENOMEM;
>  	}
>  
>  	str = strim(pattern_dup);
> @@ -948,10 +952,9 @@ static struct cfs_cpt_table *cfs_cpt_table_create_pattern(const char *pattern)
>  		goto err_free_str;
>  	}
>  
> -	cptab = cfs_cpt_table_alloc(ncpt);
> -	if (!cptab) {
> -		CERROR("Failed to allocate CPU partition table\n");
> -		rc = -ENOMEM;
> +	rc = cfs_cpt_table_setup(&cfs_cpt_tab, ncpt);
> +	if (rc) {
> +		CERROR("Failed to setup CPU partition table\n");
>  		goto err_free_str;
>  	}
>  
> @@ -960,14 +963,14 @@ static struct cfs_cpt_table *cfs_cpt_table_create_pattern(const char *pattern)
>  			if (cpumask_empty(cpumask_of_node(i)))
>  				continue;
>  
> -			rc = cfs_cpt_set_node(cptab, cpt++, i);
> +			rc = cfs_cpt_set_node(&cfs_cpt_tab, cpt++, i);
>  			if (!rc) {
>  				rc = -EINVAL;
>  				goto err_free_table;
>  			}
>  		}
>  		kfree(pattern_dup);
> -		return cptab;
> +		return 0;
>  	}
>  
>  	high = node ? nr_node_ids - 1 : nr_cpu_ids - 1;
> @@ -1006,7 +1009,7 @@ static struct cfs_cpt_table *cfs_cpt_table_create_pattern(const char *pattern)
>  			goto err_free_table;
>  		}
>  
> -		if (cfs_cpt_weight(cptab, cpt)) {
> +		if (cfs_cpt_weight(&cfs_cpt_tab, cpt)) {
>  			CERROR("Partition %d has already been set.\n", cpt);
>  			rc = -EPERM;
>  			goto err_free_table;
> @@ -1040,8 +1043,8 @@ static struct cfs_cpt_table *cfs_cpt_table_create_pattern(const char *pattern)
>  				if ((i - range->re_lo) % range->re_stride)
>  					continue;
>  
> -				rc = node ? cfs_cpt_set_node(cptab, cpt, i) :
> -					    cfs_cpt_set_cpu(cptab, cpt, i);
> +				rc = node ? cfs_cpt_set_node(&cfs_cpt_tab, cpt, i) :
> +					    cfs_cpt_set_cpu(&cfs_cpt_tab, cpt, i);
>  				if (!rc) {
>  					cfs_expr_list_free(el);
>  					rc = -EINVAL;
> @@ -1052,7 +1055,7 @@ static struct cfs_cpt_table *cfs_cpt_table_create_pattern(const char *pattern)
>  
>  		cfs_expr_list_free(el);
>  
> -		if (!cfs_cpt_online(cptab, cpt)) {
> +		if (!cfs_cpt_online(&cfs_cpt_tab, cpt)) {
>  			CERROR("No online CPU is found on partition %d\n", cpt);
>  			rc = -ENODEV;
>  			goto err_free_table;
> @@ -1062,13 +1065,13 @@ static struct cfs_cpt_table *cfs_cpt_table_create_pattern(const char *pattern)
>  	}
>  
>  	kfree(pattern_dup);
> -	return cptab;
> +	return 0;
>  
>  err_free_table:
> -	cfs_cpt_table_free(cptab);
> +	cfs_cpt_table_free(&cfs_cpt_tab);
>  err_free_str:
>  	kfree(pattern_dup);
> -	return ERR_PTR(rc);
> +	return rc;
>  }
>  
>  #ifdef CONFIG_HOTPLUG_CPU
> @@ -1095,8 +1098,7 @@ static int cfs_cpu_dead(unsigned int cpu)
>  
>  void cfs_cpu_fini(void)
>  {
> -	if (!IS_ERR_OR_NULL(cfs_cpt_tab))
> -		cfs_cpt_table_free(cfs_cpt_tab);
> +	cfs_cpt_table_free(&cfs_cpt_tab);
>  
>  #ifdef CONFIG_HOTPLUG_CPU
>  	if (lustre_cpu_online > 0)
> @@ -1109,8 +1111,6 @@ int cfs_cpu_init(void)
>  {
>  	int ret;
>  
> -	LASSERT(!cfs_cpt_tab);
> -
>  #ifdef CONFIG_HOTPLUG_CPU
>  	ret = cpuhp_setup_state_nocalls(CPUHP_LUSTRE_CFS_DEAD,
>  					"staging/lustre/cfe:dead", NULL,
> @@ -1128,20 +1128,18 @@ int cfs_cpu_init(void)
>  #endif
>  	get_online_cpus();
>  	if (*cpu_pattern) {
> -		cfs_cpt_tab = cfs_cpt_table_create_pattern(cpu_pattern);
> -		if (IS_ERR(cfs_cpt_tab)) {
> +		ret = cfs_cpt_table_create_pattern(cpu_pattern);
> +		if (ret) {
>  			CERROR("Failed to create cptab from pattern '%s'\n",
>  			       cpu_pattern);
> -			ret = PTR_ERR(cfs_cpt_tab);
>  			goto failed_alloc_table;
>  		}
>  
>  	} else {
> -		cfs_cpt_tab = cfs_cpt_table_create(cpu_npartitions);
> -		if (IS_ERR(cfs_cpt_tab)) {
> +		ret = cfs_cpt_table_create(cpu_npartitions);
> +		if (ret) {
>  			CERROR("Failed to create cptab with npartitions %d\n",
>  			       cpu_npartitions);
> -			ret = PTR_ERR(cfs_cpt_tab);
>  			goto failed_alloc_table;
>  		}
>  	}
> @@ -1150,14 +1148,13 @@ int cfs_cpu_init(void)
>  
>  	LCONSOLE(0, "HW NUMA nodes: %d, HW CPU cores: %d, npartitions: %d\n",
>  		 num_online_nodes(), num_online_cpus(),
> -		 cfs_cpt_number(cfs_cpt_tab));
> +		 cfs_cpt_number(&cfs_cpt_tab));
>  	return 0;
>  
>  failed_alloc_table:
>  	put_online_cpus();
>  
> -	if (!IS_ERR_OR_NULL(cfs_cpt_tab))
> -		cfs_cpt_table_free(cfs_cpt_tab);
> +	cfs_cpt_table_free(&cfs_cpt_tab);
>  
>  	ret = -EINVAL;
>  #ifdef CONFIG_HOTPLUG_CPU
> diff --git a/drivers/staging/lustre/lnet/libcfs/module.c b/drivers/staging/lustre/lnet/libcfs/module.c
> index 2281f08..35c3959 100644
> --- a/drivers/staging/lustre/lnet/libcfs/module.c
> +++ b/drivers/staging/lustre/lnet/libcfs/module.c
> @@ -66,6 +66,10 @@ struct lnet_debugfs_symlink_def {
>  
>  static struct dentry *lnet_debugfs_root;
>  
> +/** Global CPU partition table */
> +struct cfs_cpt_table cfs_cpt_tab __read_mostly;
> +EXPORT_SYMBOL(cfs_cpt_tab);
> +
>  BLOCKING_NOTIFIER_HEAD(libcfs_ioctl_list);
>  EXPORT_SYMBOL(libcfs_ioctl_list);
>  
> @@ -402,7 +406,7 @@ static int proc_cpt_table(struct ctl_table *table, int write,
>  		if (!buf)
>  			return -ENOMEM;
>  
> -		rc = cfs_cpt_table_print(cfs_cpt_tab, buf, len);
> +		rc = cfs_cpt_table_print(&cfs_cpt_tab, buf, len);
>  		if (rc >= 0)
>  			break;
>  
> @@ -437,14 +441,12 @@ static int proc_cpt_distance(struct ctl_table *table, int write,
>  	if (write)
>  		return -EPERM;
>  
> -	LASSERT(cfs_cpt_tab);
> -
>  	while (1) {
>  		buf = kzalloc(len, GFP_KERNEL);
>  		if (!buf)
>  			return -ENOMEM;
>  
> -		rc = cfs_cpt_distance_print(cfs_cpt_tab, buf, len);
> +		rc = cfs_cpt_distance_print(&cfs_cpt_tab, buf, len);
>  		if (rc >= 0)
>  			break;
>  
> diff --git a/drivers/staging/lustre/lnet/lnet/api-ni.c b/drivers/staging/lustre/lnet/lnet/api-ni.c
> index f9ed697..98a4942 100644
> --- a/drivers/staging/lustre/lnet/lnet/api-ni.c
> +++ b/drivers/staging/lustre/lnet/lnet/api-ni.c
> @@ -1414,8 +1414,8 @@ int lnet_lib_init(void)
>  	memset(&the_lnet, 0, sizeof(the_lnet));
>  
>  	/* refer to global cfs_cpt_tab for now */
> -	the_lnet.ln_cpt_table	= cfs_cpt_tab;
> -	the_lnet.ln_cpt_number	= cfs_cpt_number(cfs_cpt_tab);
> +	the_lnet.ln_cpt_table = &cfs_cpt_tab;
> +	the_lnet.ln_cpt_number = cfs_cpt_number(&cfs_cpt_tab);
>  
>  	LASSERT(the_lnet.ln_cpt_number > 0);
>  	if (the_lnet.ln_cpt_number > LNET_CPT_MAX) {
> diff --git a/drivers/staging/lustre/lnet/selftest/framework.c b/drivers/staging/lustre/lnet/selftest/framework.c
> index 741af10..939b7ec 100644
> --- a/drivers/staging/lustre/lnet/selftest/framework.c
> +++ b/drivers/staging/lustre/lnet/selftest/framework.c
> @@ -588,7 +588,7 @@
>  
>  	CDEBUG(D_NET, "Reserved %d buffers for test %s\n",
>  	       nbuf * (srpc_serv_is_framework(svc) ?
> -		       2 : cfs_cpt_number(cfs_cpt_tab)), svc->sv_name);
> +		       2 : cfs_cpt_number(&cfs_cpt_tab)), svc->sv_name);
>  	return 0;
>  }
>  
> diff --git a/drivers/staging/lustre/lustre/ptlrpc/client.c b/drivers/staging/lustre/lustre/ptlrpc/client.c
> index c1b82bf..c569a8b 100644
> --- a/drivers/staging/lustre/lustre/ptlrpc/client.c
> +++ b/drivers/staging/lustre/lustre/ptlrpc/client.c
> @@ -940,9 +940,9 @@ struct ptlrpc_request_set *ptlrpc_prep_set(void)
>  	struct ptlrpc_request_set *set;
>  	int cpt;
>  
> -	cpt = cfs_cpt_current(cfs_cpt_tab, 0);
> +	cpt = cfs_cpt_current(&cfs_cpt_tab, 0);
>  	set = kzalloc_node(sizeof(*set), GFP_NOFS,
> -			   cfs_cpt_spread_node(cfs_cpt_tab, cpt));
> +			   cfs_cpt_spread_node(&cfs_cpt_tab, cpt));
>  	if (!set)
>  		return NULL;
>  	atomic_set(&set->set_refcount, 1);
> diff --git a/drivers/staging/lustre/lustre/ptlrpc/ptlrpcd.c b/drivers/staging/lustre/lustre/ptlrpc/ptlrpcd.c
> index 5310054..d496521 100644
> --- a/drivers/staging/lustre/lustre/ptlrpc/ptlrpcd.c
> +++ b/drivers/staging/lustre/lustre/ptlrpc/ptlrpcd.c
> @@ -177,7 +177,7 @@ void ptlrpcd_wake(struct ptlrpc_request *req)
>  	if (req && req->rq_send_state != LUSTRE_IMP_FULL)
>  		return &ptlrpcd_rcv;
>  
> -	cpt = cfs_cpt_current(cfs_cpt_tab, 1);
> +	cpt = cfs_cpt_current(&cfs_cpt_tab, 1);
>  	if (!ptlrpcds_cpt_idx)
>  		idx = cpt;
>  	else
> @@ -389,7 +389,7 @@ static int ptlrpcd(void *arg)
>  	int exit = 0;
>  
>  	unshare_fs_struct();
> -	if (cfs_cpt_bind(cfs_cpt_tab, pc->pc_cpt) != 0)
> +	if (cfs_cpt_bind(&cfs_cpt_tab, pc->pc_cpt) != 0)
>  		CWARN("Failed to bind %s on CPT %d\n", pc->pc_name, pc->pc_cpt);
>  
>  	/*
> @@ -531,7 +531,7 @@ static int ptlrpcd_partners(struct ptlrpcd *pd, int index)
>  
>  	size = sizeof(struct ptlrpcd_ctl *) * pc->pc_npartners;
>  	pc->pc_partners = kzalloc_node(size, GFP_NOFS,
> -				       cfs_cpt_spread_node(cfs_cpt_tab,
> +				       cfs_cpt_spread_node(&cfs_cpt_tab,
>  							   pc->pc_cpt));
>  	if (!pc->pc_partners) {
>  		pc->pc_npartners = 0;
> @@ -677,7 +677,7 @@ static int ptlrpcd_init(void)
>  	/*
>  	 * Determine the CPTs that ptlrpcd threads will run on.
>  	 */
> -	cptable = cfs_cpt_tab;
> +	cptable = &cfs_cpt_tab;
>  	ncpts = cfs_cpt_number(cptable);
>  	if (ptlrpcd_cpts) {
>  		struct cfs_expr_list *el;
> @@ -831,7 +831,7 @@ static int ptlrpcd_init(void)
>  
>  		size = offsetof(struct ptlrpcd, pd_threads[nthreads]);
>  		pd = kzalloc_node(size, GFP_NOFS,
> -				  cfs_cpt_spread_node(cfs_cpt_tab, cpt));
> +				  cfs_cpt_spread_node(&cfs_cpt_tab, cpt));
>  		if (!pd) {
>  			rc = -ENOMEM;
>  			goto out;
> diff --git a/drivers/staging/lustre/lustre/ptlrpc/service.c b/drivers/staging/lustre/lustre/ptlrpc/service.c
> index 8e74a45..853676f 100644
> --- a/drivers/staging/lustre/lustre/ptlrpc/service.c
> +++ b/drivers/staging/lustre/lustre/ptlrpc/service.c
> @@ -565,7 +565,7 @@ struct ptlrpc_service *
>  
>  	cptable = cconf->cc_cptable;
>  	if (!cptable)
> -		cptable = cfs_cpt_tab;
> +		cptable = &cfs_cpt_tab;
>  
>  	if (!conf->psc_thr.tc_cpu_affinity) {
>  		ncpts = 1;
> @@ -2520,7 +2520,7 @@ int ptlrpc_hr_init(void)
>  	int weight;
>  
>  	memset(&ptlrpc_hr, 0, sizeof(ptlrpc_hr));
> -	ptlrpc_hr.hr_cpt_table = cfs_cpt_tab;
> +	ptlrpc_hr.hr_cpt_table = &cfs_cpt_tab;
>  
>  	ptlrpc_hr.hr_partitions = cfs_percpt_alloc(ptlrpc_hr.hr_cpt_table,
>  						   sizeof(*hrp));
> -- 
> 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/02020e75/attachment.sig>


More information about the lustre-devel mailing list