Re: [PATCH v2 1/7] target: core: add common tpg/enable attribute

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 3/22/21 3:05 AM, Dmitry Bogdanov wrote:
> Many fabric modules provide their own implementation of enable
> attribute in tpg.
> The change provides a way to remove code duplication in the fabric
> modules and automatically add "enable" attribute if a fabric module has
> an implementation of fabric_enable_tpg() ops.
> 
> Reviewed-by: Roman Bolshakov <r.bolshakov@xxxxxxxxx>
> Signed-off-by: Dmitry Bogdanov <d.bogdanov@xxxxxxxxx>
> ---
> v2:
>     create enable atribute only for modules with enable_tpg ops 
> 
>  drivers/target/target_core_configfs.c        |  1 +
>  drivers/target/target_core_fabric_configfs.c | 40 ++++++++++++++++++-
>  drivers/target/target_core_internal.h        |  1 +
>  drivers/target/target_core_tpg.c             | 41 ++++++++++++++++++++
>  include/target/target_core_base.h            |  1 +
>  include/target/target_core_fabric.h          |  1 +
>  6 files changed, 83 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/target/target_core_configfs.c b/drivers/target/target_core_configfs.c
> index f04352285155..fc3949e91f9c 100644
> --- a/drivers/target/target_core_configfs.c
> +++ b/drivers/target/target_core_configfs.c
> @@ -490,6 +490,7 @@ void target_unregister_template(const struct target_core_fabric_ops *fo)
>  			 * fabric driver unload of TFO->module to proceed.
>  			 */
>  			rcu_barrier();
> +			kfree(t->tf_tpg_base_cit.ct_attrs);
>  			kfree(t);
>  			return;
>  		}
> diff --git a/drivers/target/target_core_fabric_configfs.c b/drivers/target/target_core_fabric_configfs.c
> index ee85602213f7..c5e1b0d54330 100644
> --- a/drivers/target/target_core_fabric_configfs.c
> +++ b/drivers/target/target_core_fabric_configfs.c
> @@ -815,8 +815,40 @@ static struct configfs_item_operations target_fabric_tpg_base_item_ops = {
>  	.release		= target_fabric_tpg_release,
>  };
>  
> -TF_CIT_SETUP_DRV(tpg_base, &target_fabric_tpg_base_item_ops, NULL);
> +static int
> +target_fabric_setup_tpg_base_cit(struct target_fabric_configfs *tf)
> +{
> +	int i, k, len = 0;
> +	struct config_item_type *cit = &tf->tf_tpg_base_cit;
> +	struct configfs_attribute **attrs;
> +
> +	if (tf->tf_ops->fabric_enable_tpg)
> +		for (i = 0; core_tpg_base_enable_attrs[i]; i++)
> +			len += sizeof(struct configfs_attribute *);
> +	if (tf->tf_ops->tfc_tpg_base_attrs)
> +		for (i = 0; tf->tf_ops->tfc_tpg_base_attrs[i]; i++)
> +			len += sizeof(struct configfs_attribute *);
> +	len += sizeof(struct configfs_attribute *);
> +
> +	attrs = kzalloc(len, GFP_KERNEL);
> +	if (!attrs)
> +		return -ENOMEM;
> +
> +	if (tf->tf_ops->fabric_enable_tpg)
> +		for (i = 0; core_tpg_base_enable_attrs[i]; i++)
> +			attrs[i] = core_tpg_base_enable_attrs[i];
> +	if (tf->tf_ops->tfc_tpg_base_attrs)
> +		for (k = 0; tf->tf_ops->tfc_tpg_base_attrs[k]; k++, i++)

If fabric_enable_tpg is not set then I think i is the value from above
when we did the tfc_tpg_base_attrs loop to calculate the total len needed
for the kzalloc.


> +			attrs[i] = tf->tf_ops->tfc_tpg_base_attrs[k];
> +	attrs[i] = NULL;

If fabric_enable_tpg and tfc_tpg_base_attrs attrs is not set then i is
not initialized.

I'm guessing you guys do more testing with tcm_qla. Maybe also do a quick
target creation test with iscsi and loop to hit some of the different combos.



> +
> +	cit->ct_item_ops = &target_fabric_tpg_base_item_ops;
> +	cit->ct_attrs = attrs;
> +	cit->ct_owner = tf->tf_ops->module;
> +	pr_debug("Setup generic tpg_base\n");
>  
> +	return 0;
> +}
>  /* End of tfc_tpg_base_cit */
>  
>  /* Start of tfc_tpg_cit */
> @@ -971,11 +1003,15 @@ TF_CIT_SETUP_DRV(discovery, NULL, NULL);
>  
>  int target_fabric_setup_cits(struct target_fabric_configfs *tf)
>  {
> +	int ret;
> +
>  	target_fabric_setup_discovery_cit(tf);
>  	target_fabric_setup_wwn_cit(tf);
>  	target_fabric_setup_wwn_fabric_stats_cit(tf);
>  	target_fabric_setup_tpg_cit(tf);
> -	target_fabric_setup_tpg_base_cit(tf);

Add newline

> +	ret = target_fabric_setup_tpg_base_cit(tf);
> +	if (ret)
> +		return ret;

Add newline

>  	target_fabric_setup_tpg_port_cit(tf);
>  	target_fabric_setup_tpg_port_stat_cit(tf);
>  	target_fabric_setup_tpg_lun_cit(tf);
> diff --git a/drivers/target/target_core_internal.h b/drivers/target/target_core_internal.h
> index e7b3c6e5d574..274fd670bf9c 100644
> --- a/drivers/target/target_core_internal.h
> +++ b/drivers/target/target_core_internal.h
> @@ -117,6 +117,7 @@ int	core_tmr_lun_reset(struct se_device *, struct se_tmr_req *,
>  
>  /* target_core_tpg.c */
>  extern struct se_device *g_lun0_dev;
> +extern struct configfs_attribute *core_tpg_base_enable_attrs[];
>  
>  struct se_node_acl *__core_tpg_get_initiator_node_acl(struct se_portal_group *tpg,
>  		const char *);
> diff --git a/drivers/target/target_core_tpg.c b/drivers/target/target_core_tpg.c
> index 736847c933e5..d06b3fbd8511 100644
> --- a/drivers/target/target_core_tpg.c
> +++ b/drivers/target/target_core_tpg.c
> @@ -650,3 +650,44 @@ void core_tpg_remove_lun(
>  
>  	percpu_ref_exit(&lun->lun_ref);
>  }
> +

Delete extra newline.

> +
> +static ssize_t core_tpg_base_enable_show(struct config_item *item, char *page)
> +{
> +	return sysfs_emit(page, "%d\n", to_tpg(item)->enabled);
> +}
> +
> +static ssize_t core_tpg_base_enable_store(struct config_item *item,
> +					  const char *page, size_t count)
> +{
> +	struct se_portal_group *se_tpg = to_tpg(item);
> +	int ret;
> +	u32 op;
> +
> +	ret = kstrtou32(page, 0, &op);
> +	if (ret)
> +		return ret;

Add a newline.

> +	if ((op != 1) && (op != 0)) {
> +		pr_err("Illegal value for tpg_enable: %u\n", op);
> +		return -EINVAL;
> +	}
> +
> +	if (se_tpg->enabled == op)
> +		return count;
> +
> +	ret = se_tpg->se_tpg_tfo->fabric_enable_tpg(se_tpg, op);
> +

Delete extra newline.

> +	if (ret)
> +		return ret;
> +
> +	se_tpg->enabled = op;
> +
> +	return count;
> +}



[Index of Archives]     [Linux SCSI]     [Kernel Newbies]     [Linux SCSI Target Infrastructure]     [Share Photos]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Device Mapper]

  Powered by Linux