On Wed, 2013-10-16 at 09:20 +0200, Hannes Reinecke wrote:
> We should only allocate ALUA metadata if we're actually going
> to write them.
> 
> Signed-off-by: Hannes Reinecke <h...@suse.de>
> ---
>  drivers/target/target_core_alua.c | 70 
> +++++++++++++++++----------------------
>  drivers/target/target_core_alua.h |  3 ++
>  include/target/target_core_base.h |  3 --
>  3 files changed, 34 insertions(+), 42 deletions(-)
> 

Looks reasonable to me..

--nab

> diff --git a/drivers/target/target_core_alua.c 
> b/drivers/target/target_core_alua.c
> index a420778..b1d08bf 100644
> --- a/drivers/target/target_core_alua.c
> +++ b/drivers/target/target_core_alua.c
> @@ -762,16 +762,22 @@ static int core_alua_write_tpg_metadata(
>   */
>  static int core_alua_update_tpg_primary_metadata(
>       struct t10_alua_tg_pt_gp *tg_pt_gp,
> -     int primary_state,
> -     unsigned char *md_buf)
> +     int primary_state)
>  {
> +     unsigned char *md_buf;
>       struct t10_wwn *wwn = &tg_pt_gp->tg_pt_gp_dev->t10_wwn;
>       char path[ALUA_METADATA_PATH_LEN];
> -     int len;
> +     int len, rc;
> +
> +     md_buf = kzalloc(ALUA_MD_BUF_LEN, GFP_KERNEL);
> +     if (!md_buf) {
> +             pr_err("Unable to allocate buf for ALUA metadata\n");
> +             return -ENOMEM;
> +     }
>  
>       memset(path, 0, ALUA_METADATA_PATH_LEN);
>  
> -     len = snprintf(md_buf, tg_pt_gp->tg_pt_gp_md_buf_len,
> +     len = snprintf(md_buf, ALUA_MD_BUF_LEN,
>                       "tg_pt_gp_id=%hu\n"
>                       "alua_access_state=0x%02x\n"
>                       "alua_access_status=0x%02x\n",
> @@ -782,14 +788,15 @@ static int core_alua_update_tpg_primary_metadata(
>               "/var/target/alua/tpgs_%s/%s", &wwn->unit_serial[0],
>               config_item_name(&tg_pt_gp->tg_pt_gp_group.cg_item));
>  
> -     return core_alua_write_tpg_metadata(path, md_buf, len);
> +     rc = core_alua_write_tpg_metadata(path, md_buf, len);
> +     kfree(md_buf);
> +     return rc;
>  }
>  
>  static int core_alua_do_transition_tg_pt(
>       struct t10_alua_tg_pt_gp *tg_pt_gp,
>       struct se_port *l_port,
>       struct se_node_acl *nacl,
> -     unsigned char *md_buf,
>       int new_state,
>       int explicit)
>  {
> @@ -877,8 +884,7 @@ static int core_alua_do_transition_tg_pt(
>        */
>       if (tg_pt_gp->tg_pt_gp_write_metadata) {
>               mutex_lock(&tg_pt_gp->tg_pt_gp_md_mutex);
> -             core_alua_update_tpg_primary_metadata(tg_pt_gp,
> -                                     new_state, md_buf);
> +             core_alua_update_tpg_primary_metadata(tg_pt_gp, new_state);
>               mutex_unlock(&tg_pt_gp->tg_pt_gp_md_mutex);
>       }
>       /*
> @@ -909,19 +915,12 @@ int core_alua_do_port_transition(
>       struct t10_alua_lu_gp *lu_gp;
>       struct t10_alua_lu_gp_member *lu_gp_mem, *local_lu_gp_mem;
>       struct t10_alua_tg_pt_gp *tg_pt_gp;
> -     unsigned char *md_buf;
>       int primary, valid_states;
>  
>       valid_states = l_tg_pt_gp->tg_pt_gp_alua_supported_states;
>       if (core_alua_check_transition(new_state, valid_states, &primary) != 0)
>               return -EINVAL;
>  
> -     md_buf = kzalloc(l_tg_pt_gp->tg_pt_gp_md_buf_len, GFP_KERNEL);
> -     if (!md_buf) {
> -             pr_err("Unable to allocate buf for ALUA metadata\n");
> -             return -ENOMEM;
> -     }
> -
>       local_lu_gp_mem = l_dev->dev_alua_lu_gp_mem;
>       spin_lock(&local_lu_gp_mem->lu_gp_mem_lock);
>       lu_gp = local_lu_gp_mem->lu_gp;
> @@ -939,10 +938,9 @@ int core_alua_do_port_transition(
>                * success.
>                */
>               core_alua_do_transition_tg_pt(l_tg_pt_gp, l_port, l_nacl,
> -                                     md_buf, new_state, explicit);
> +                                     new_state, explicit);
>               atomic_dec(&lu_gp->lu_gp_ref_cnt);
>               smp_mb__after_atomic_dec();
> -             kfree(md_buf);
>               return 0;
>       }
>       /*
> @@ -992,7 +990,7 @@ int core_alua_do_port_transition(
>                        * success.
>                        */
>                       core_alua_do_transition_tg_pt(tg_pt_gp, port,
> -                                     nacl, md_buf, new_state, explicit);
> +                                     nacl, new_state, explicit);
>  
>                       spin_lock(&dev->t10_alua.tg_pt_gps_lock);
>                       atomic_dec(&tg_pt_gp->tg_pt_gp_ref_cnt);
> @@ -1014,7 +1012,6 @@ int core_alua_do_port_transition(
>  
>       atomic_dec(&lu_gp->lu_gp_ref_cnt);
>       smp_mb__after_atomic_dec();
> -     kfree(md_buf);
>       return 0;
>  }
>  
> @@ -1023,13 +1020,18 @@ int core_alua_do_port_transition(
>   */
>  static int core_alua_update_tpg_secondary_metadata(
>       struct t10_alua_tg_pt_gp_member *tg_pt_gp_mem,
> -     struct se_port *port,
> -     unsigned char *md_buf,
> -     u32 md_buf_len)
> +     struct se_port *port)
>  {
> +     unsigned char *md_buf;
>       struct se_portal_group *se_tpg = port->sep_tpg;
>       char path[ALUA_METADATA_PATH_LEN], wwn[ALUA_SECONDARY_METADATA_WWN_LEN];
> -     int len;
> +     int len, rc;
> +
> +     md_buf = kzalloc(ALUA_MD_BUF_LEN, GFP_KERNEL);
> +     if (!md_buf) {
> +             pr_err("Unable to allocate buf for ALUA metadata\n");
> +             return -ENOMEM;
> +     }
>  
>       memset(path, 0, ALUA_METADATA_PATH_LEN);
>       memset(wwn, 0, ALUA_SECONDARY_METADATA_WWN_LEN);
> @@ -1041,7 +1043,7 @@ static int core_alua_update_tpg_secondary_metadata(
>               snprintf(wwn+len, ALUA_SECONDARY_METADATA_WWN_LEN-len, "+%hu",
>                               se_tpg->se_tpg_tfo->tpg_get_tag(se_tpg));
>  
> -     len = snprintf(md_buf, md_buf_len, "alua_tg_pt_offline=%d\n"
> +     len = snprintf(md_buf, ALUA_MD_BUF_LEN, "alua_tg_pt_offline=%d\n"
>                       "alua_tg_pt_status=0x%02x\n",
>                       atomic_read(&port->sep_tg_pt_secondary_offline),
>                       port->sep_tg_pt_secondary_stat);
> @@ -1050,7 +1052,10 @@ static int core_alua_update_tpg_secondary_metadata(
>                       se_tpg->se_tpg_tfo->get_fabric_name(), wwn,
>                       port->sep_lun->unpacked_lun);
>  
> -     return core_alua_write_tpg_metadata(path, md_buf, len);
> +     rc = core_alua_write_tpg_metadata(path, md_buf, len);
> +     kfree(md_buf);
> +
> +     return rc;
>  }
>  
>  static int core_alua_set_tg_pt_secondary_state(
> @@ -1060,8 +1065,6 @@ static int core_alua_set_tg_pt_secondary_state(
>       int offline)
>  {
>       struct t10_alua_tg_pt_gp *tg_pt_gp;
> -     unsigned char *md_buf;
> -     u32 md_buf_len;
>       int trans_delay_msecs;
>  
>       spin_lock(&tg_pt_gp_mem->tg_pt_gp_mem_lock);
> @@ -1082,7 +1085,6 @@ static int core_alua_set_tg_pt_secondary_state(
>       else
>               atomic_set(&port->sep_tg_pt_secondary_offline, 0);
>  
> -     md_buf_len = tg_pt_gp->tg_pt_gp_md_buf_len;
>       port->sep_tg_pt_secondary_stat = (explicit) ?
>                       ALUA_STATUS_ALTERED_BY_EXPLICIT_STPG :
>                       ALUA_STATUS_ALTERED_BY_IMPLICIT_ALUA;
> @@ -1104,18 +1106,9 @@ static int core_alua_set_tg_pt_secondary_state(
>        * secondary state and status
>        */
>       if (port->sep_tg_pt_secondary_write_md) {
> -             md_buf = kzalloc(md_buf_len, GFP_KERNEL);
> -             if (!md_buf) {
> -                     pr_err("Unable to allocate md_buf for"
> -                             " secondary ALUA access metadata\n");
> -                     return -ENOMEM;
> -             }
>               mutex_lock(&port->sep_tg_pt_md_mutex);
> -             core_alua_update_tpg_secondary_metadata(tg_pt_gp_mem, port,
> -                             md_buf, md_buf_len);
> +             core_alua_update_tpg_secondary_metadata(tg_pt_gp_mem, port);
>               mutex_unlock(&port->sep_tg_pt_md_mutex);
> -
> -             kfree(md_buf);
>       }
>  
>       return 0;
> @@ -1374,7 +1367,6 @@ struct t10_alua_tg_pt_gp 
> *core_alua_allocate_tg_pt_gp(struct se_device *dev,
>       spin_lock_init(&tg_pt_gp->tg_pt_gp_lock);
>       atomic_set(&tg_pt_gp->tg_pt_gp_ref_cnt, 0);
>       tg_pt_gp->tg_pt_gp_dev = dev;
> -     tg_pt_gp->tg_pt_gp_md_buf_len = ALUA_MD_BUF_LEN;
>       atomic_set(&tg_pt_gp->tg_pt_gp_alua_access_state,
>               ALUA_ACCESS_STATE_ACTIVE_OPTIMIZED);
>       /*
> diff --git a/drivers/target/target_core_alua.h 
> b/drivers/target/target_core_alua.h
> index 88e2e83..1a152cd 100644
> --- a/drivers/target/target_core_alua.h
> +++ b/drivers/target/target_core_alua.h
> @@ -78,6 +78,9 @@
>   */
>  #define ALUA_SECONDARY_METADATA_WWN_LEN                      256
>  
> +/* Used by core_alua_update_tpg_(primary,secondary)_metadata */
> +#define ALUA_MD_BUF_LEN                                      1024
> +
>  extern struct kmem_cache *t10_alua_lu_gp_cache;
>  extern struct kmem_cache *t10_alua_lu_gp_mem_cache;
>  extern struct kmem_cache *t10_alua_tg_pt_gp_cache;
> diff --git a/include/target/target_core_base.h 
> b/include/target/target_core_base.h
> index 21f4bd5..933c59d 100644
> --- a/include/target/target_core_base.h
> +++ b/include/target/target_core_base.h
> @@ -290,9 +290,6 @@ struct t10_alua_tg_pt_gp {
>       int     tg_pt_gp_implicit_trans_secs;
>       int     tg_pt_gp_pref;
>       int     tg_pt_gp_write_metadata;
> -     /* Used by struct t10_alua_tg_pt_gp->tg_pt_gp_md_buf_len */
> -#define ALUA_MD_BUF_LEN                              1024
> -     u32     tg_pt_gp_md_buf_len;
>       u32     tg_pt_gp_members;
>       atomic_t tg_pt_gp_alua_access_state;
>       atomic_t tg_pt_gp_ref_cnt;


--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to