Discussion:
[PATCH 10/10] qla_target: rearrange struct qla_tgt_prm
Joern Engel
2014-09-16 20:23:19 UTC
Permalink
On most (non-x86) 64bit platforms this will remove 8 padding bytes
from the structure.

Signed-off-by: Joern Engel <***@logfs.org>
---
drivers/scsi/qla2xxx/qla_target.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/qla2xxx/qla_target.h b/drivers/scsi/qla2xxx/qla_target.h
index bc1aee78d934..c93eeab0cfb2 100644
--- a/drivers/scsi/qla2xxx/qla_target.h
+++ b/drivers/scsi/qla2xxx/qla_target.h
@@ -971,11 +971,11 @@ struct qla_tgt_prm {
struct qla_tgt *tgt;
void *pkt;
struct scatterlist *sg; /* cmd data buffer SG vector */
+ unsigned char *sense_buffer;
int seg_cnt;
int req_cnt;
uint16_t rq_result;
uint16_t scsi_status;
- unsigned char *sense_buffer;
int sense_buffer_len;
int residual;
int add_status_pkt;
--
2.0.0.rc0.1.g7b2ba98
Joern Engel
2014-09-16 20:23:12 UTC
Permalink
The target code has a rather generous helping of smp_mb__after_atomic()
throughout the code base. Most atomic operations were followed by one
and none were preceded by smp_mb__before_atomic(), nor accompanied by a
comment explaining the need for a barrier.

Instead of trying to prove for every case whether or not it is needed,
this patch introduces atomic_inc_mb() and atomic_dec_mb(), which
explicitly include the memory barriers before and after the atomic
operation. For now they are defined in a target header, although they
could be of general use.

Most of the existing atomic/mb combinations were replaced by the new
helpers. In a few cases the atomic was sandwiched in
spin_lock/spin_unlock and I simply removed the barrier.

I suspect that in most cases the correct conversion would have been to
drop the barrier. I also suspect that a few cases exist where a) the
barrier was necessary and b) a second barrier before the atomic would
have been necessary and got added by this patch.

Signed-off-by: Joern Engel <***@logfs.org>
---
drivers/target/loopback/tcm_loop.c | 6 +--
drivers/target/target_core_alua.c | 33 ++++---------
drivers/target/target_core_device.c | 9 ++--
drivers/target/target_core_pr.c | 90 ++++++++++++----------------------
drivers/target/target_core_transport.c | 18 +++----
drivers/target/target_core_ua.c | 15 ++----
include/target/target_core_base.h | 14 ++++++
7 files changed, 70 insertions(+), 115 deletions(-)

diff --git a/drivers/target/loopback/tcm_loop.c b/drivers/target/loopback/tcm_loop.c
index 340de9d92b15..a7f6dc646045 100644
--- a/drivers/target/loopback/tcm_loop.c
+++ b/drivers/target/loopback/tcm_loop.c
@@ -960,8 +960,7 @@ static int tcm_loop_port_link(
struct tcm_loop_tpg, tl_se_tpg);
struct tcm_loop_hba *tl_hba = tl_tpg->tl_hba;

- atomic_inc(&tl_tpg->tl_tpg_port_count);
- smp_mb__after_atomic();
+ atomic_inc_mb(&tl_tpg->tl_tpg_port_count);
/*
* Add Linux/SCSI struct scsi_device by HCTL
*/
@@ -995,8 +994,7 @@ static void tcm_loop_port_unlink(
scsi_remove_device(sd);
scsi_device_put(sd);

- atomic_dec(&tl_tpg->tl_tpg_port_count);
- smp_mb__after_atomic();
+ atomic_dec_mb(&tl_tpg->tl_tpg_port_count);

pr_debug("TCM_Loop_ConfigFS: Port Unlink Successful\n");
}
diff --git a/drivers/target/target_core_alua.c b/drivers/target/target_core_alua.c
index fbc5ebb5f761..fb87780929d2 100644
--- a/drivers/target/target_core_alua.c
+++ b/drivers/target/target_core_alua.c
@@ -392,8 +392,7 @@ target_emulate_set_target_port_groups(struct se_cmd *cmd)
if (tg_pt_id != tg_pt_gp->tg_pt_gp_id)
continue;

- atomic_inc(&tg_pt_gp->tg_pt_gp_ref_cnt);
- smp_mb__after_atomic();
+ atomic_inc_mb(&tg_pt_gp->tg_pt_gp_ref_cnt);

spin_unlock(&dev->t10_alua.tg_pt_gps_lock);

@@ -403,8 +402,7 @@ target_emulate_set_target_port_groups(struct se_cmd *cmd)
found = true;

spin_lock(&dev->t10_alua.tg_pt_gps_lock);
- atomic_dec(&tg_pt_gp->tg_pt_gp_ref_cnt);
- smp_mb__after_atomic();
+ atomic_dec_mb(&tg_pt_gp->tg_pt_gp_ref_cnt);
break;
}
spin_unlock(&dev->t10_alua.tg_pt_gps_lock);
@@ -998,8 +996,7 @@ static void core_alua_do_transition_tg_pt_work(struct work_struct *work)
* every I_T nexus other than the I_T nexus on which the SET
* TARGET PORT GROUPS command
*/
- atomic_inc(&mem->tg_pt_gp_mem_ref_cnt);
- smp_mb__after_atomic();
+ atomic_inc_mb(&mem->tg_pt_gp_mem_ref_cnt);
spin_unlock(&tg_pt_gp->tg_pt_gp_lock);

spin_lock_bh(&port->sep_alua_lock);
@@ -1028,8 +1025,7 @@ static void core_alua_do_transition_tg_pt_work(struct work_struct *work)
spin_unlock_bh(&port->sep_alua_lock);

spin_lock(&tg_pt_gp->tg_pt_gp_lock);
- atomic_dec(&mem->tg_pt_gp_mem_ref_cnt);
- smp_mb__after_atomic();
+ atomic_dec_mb(&mem->tg_pt_gp_mem_ref_cnt);
}
spin_unlock(&tg_pt_gp->tg_pt_gp_lock);
/*
@@ -1063,7 +1059,6 @@ static void core_alua_do_transition_tg_pt_work(struct work_struct *work)
core_alua_dump_state(tg_pt_gp->tg_pt_gp_alua_pending_state));
spin_lock(&dev->t10_alua.tg_pt_gps_lock);
atomic_dec(&tg_pt_gp->tg_pt_gp_ref_cnt);
- smp_mb__after_atomic();
spin_unlock(&dev->t10_alua.tg_pt_gps_lock);

if (tg_pt_gp->tg_pt_gp_transition_complete)
@@ -1125,7 +1120,6 @@ static int core_alua_do_transition_tg_pt(
*/
spin_lock(&dev->t10_alua.tg_pt_gps_lock);
atomic_inc(&tg_pt_gp->tg_pt_gp_ref_cnt);
- smp_mb__after_atomic();
spin_unlock(&dev->t10_alua.tg_pt_gps_lock);

if (!explicit && tg_pt_gp->tg_pt_gp_implicit_trans_secs) {
@@ -1168,7 +1162,6 @@ int core_alua_do_port_transition(
spin_lock(&local_lu_gp_mem->lu_gp_mem_lock);
lu_gp = local_lu_gp_mem->lu_gp;
atomic_inc(&lu_gp->lu_gp_ref_cnt);
- smp_mb__after_atomic();
spin_unlock(&local_lu_gp_mem->lu_gp_mem_lock);
/*
* For storage objects that are members of the 'default_lu_gp',
@@ -1184,8 +1177,7 @@ int core_alua_do_port_transition(
l_tg_pt_gp->tg_pt_gp_alua_nacl = l_nacl;
rc = core_alua_do_transition_tg_pt(l_tg_pt_gp,
new_state, explicit);
- atomic_dec(&lu_gp->lu_gp_ref_cnt);
- smp_mb__after_atomic();
+ atomic_dec_mb(&lu_gp->lu_gp_ref_cnt);
return rc;
}
/*
@@ -1198,8 +1190,7 @@ int core_alua_do_port_transition(
lu_gp_mem_list) {

dev = lu_gp_mem->lu_gp_mem_dev;
- atomic_inc(&lu_gp_mem->lu_gp_mem_ref_cnt);
- smp_mb__after_atomic();
+ atomic_inc_mb(&lu_gp_mem->lu_gp_mem_ref_cnt);
spin_unlock(&lu_gp->lu_gp_lock);

spin_lock(&dev->t10_alua.tg_pt_gps_lock);
@@ -1227,8 +1218,7 @@ int core_alua_do_port_transition(
tg_pt_gp->tg_pt_gp_alua_port = NULL;
tg_pt_gp->tg_pt_gp_alua_nacl = NULL;
}
- atomic_inc(&tg_pt_gp->tg_pt_gp_ref_cnt);
- smp_mb__after_atomic();
+ atomic_inc_mb(&tg_pt_gp->tg_pt_gp_ref_cnt);
spin_unlock(&dev->t10_alua.tg_pt_gps_lock);
/*
* core_alua_do_transition_tg_pt() will always return
@@ -1238,16 +1228,14 @@ int core_alua_do_port_transition(
new_state, explicit);

spin_lock(&dev->t10_alua.tg_pt_gps_lock);
- atomic_dec(&tg_pt_gp->tg_pt_gp_ref_cnt);
- smp_mb__after_atomic();
+ atomic_dec_mb(&tg_pt_gp->tg_pt_gp_ref_cnt);
if (rc)
break;
}
spin_unlock(&dev->t10_alua.tg_pt_gps_lock);

spin_lock(&lu_gp->lu_gp_lock);
- atomic_dec(&lu_gp_mem->lu_gp_mem_ref_cnt);
- smp_mb__after_atomic();
+ atomic_dec_mb(&lu_gp_mem->lu_gp_mem_ref_cnt);
}
spin_unlock(&lu_gp->lu_gp_lock);

@@ -1260,8 +1248,7 @@ int core_alua_do_port_transition(
core_alua_dump_state(new_state));
}

- atomic_dec(&lu_gp->lu_gp_ref_cnt);
- smp_mb__after_atomic();
+ atomic_dec_mb(&lu_gp->lu_gp_ref_cnt);
return rc;
}

diff --git a/drivers/target/target_core_device.c b/drivers/target/target_core_device.c
index 98da90167159..bb3c2399d85f 100644
--- a/drivers/target/target_core_device.c
+++ b/drivers/target/target_core_device.c
@@ -224,8 +224,7 @@ struct se_dev_entry *core_get_se_deve_from_rtpi(
if (port->sep_rtpi != rtpi)
continue;

- atomic_inc(&deve->pr_ref_count);
- smp_mb__after_atomic();
+ atomic_inc_mb(&deve->pr_ref_count);
spin_unlock_irq(&nacl->device_list_lock);

return deve;
@@ -1396,8 +1395,7 @@ int core_dev_add_initiator_node_lun_acl(

spin_lock(&lun->lun_acl_lock);
list_add_tail(&lacl->lacl_list, &lun->lun_acl_list);
- atomic_inc(&lun->lun_acl_count);
- smp_mb__after_atomic();
+ atomic_inc_mb(&lun->lun_acl_count);
spin_unlock(&lun->lun_acl_lock);

pr_debug("%s_TPG[%hu]_LUN[%u->%u] - Added %s ACL for "
@@ -1430,8 +1428,7 @@ int core_dev_del_initiator_node_lun_acl(

spin_lock(&lun->lun_acl_lock);
list_del(&lacl->lacl_list);
- atomic_dec(&lun->lun_acl_count);
- smp_mb__after_atomic();
+ atomic_dec_mb(&lun->lun_acl_count);
spin_unlock(&lun->lun_acl_lock);

core_disable_device_list_for_node(lun, NULL, lacl->mapped_lun,
diff --git a/drivers/target/target_core_pr.c b/drivers/target/target_core_pr.c
index df357862286e..70a4b09931d7 100644
--- a/drivers/target/target_core_pr.c
+++ b/drivers/target/target_core_pr.c
@@ -674,8 +674,7 @@ static struct t10_pr_registration *__core_scsi3_alloc_registration(
*/
spin_lock(&dev->se_port_lock);
list_for_each_entry_safe(port, port_tmp, &dev->dev_sep_list, sep_list) {
- atomic_inc(&port->sep_tg_pt_ref_cnt);
- smp_mb__after_atomic();
+ atomic_inc_mb(&port->sep_tg_pt_ref_cnt);
spin_unlock(&dev->se_port_lock);

spin_lock_bh(&port->sep_alua_lock);
@@ -709,8 +708,7 @@ static struct t10_pr_registration *__core_scsi3_alloc_registration(
if (strcmp(nacl->initiatorname, nacl_tmp->initiatorname))
continue;

- atomic_inc(&deve_tmp->pr_ref_count);
- smp_mb__after_atomic();
+ atomic_inc_mb(&deve_tmp->pr_ref_count);
spin_unlock_bh(&port->sep_alua_lock);
/*
* Grab a configfs group dependency that is released
@@ -722,10 +720,8 @@ static struct t10_pr_registration *__core_scsi3_alloc_registration(
if (ret < 0) {
pr_err("core_scsi3_lunacl_depend"
"_item() failed\n");
- atomic_dec(&port->sep_tg_pt_ref_cnt);
- smp_mb__after_atomic();
- atomic_dec(&deve_tmp->pr_ref_count);
- smp_mb__after_atomic();
+ atomic_dec_mb(&port->sep_tg_pt_ref_cnt);
+ atomic_dec_mb(&deve_tmp->pr_ref_count);
goto out;
}
/*
@@ -739,10 +735,8 @@ static struct t10_pr_registration *__core_scsi3_alloc_registration(
nacl_tmp, deve_tmp, NULL,
sa_res_key, all_tg_pt, aptpl);
if (!pr_reg_atp) {
- atomic_dec(&port->sep_tg_pt_ref_cnt);
- smp_mb__after_atomic();
- atomic_dec(&deve_tmp->pr_ref_count);
- smp_mb__after_atomic();
+ atomic_dec_mb(&port->sep_tg_pt_ref_cnt);
+ atomic_dec_mb(&deve_tmp->pr_ref_count);
core_scsi3_lunacl_undepend_item(deve_tmp);
goto out;
}
@@ -754,8 +748,7 @@ static struct t10_pr_registration *__core_scsi3_alloc_registration(
spin_unlock_bh(&port->sep_alua_lock);

spin_lock(&dev->se_port_lock);
- atomic_dec(&port->sep_tg_pt_ref_cnt);
- smp_mb__after_atomic();
+ atomic_dec_mb(&port->sep_tg_pt_ref_cnt);
}
spin_unlock(&dev->se_port_lock);

@@ -1109,8 +1102,7 @@ static struct t10_pr_registration *__core_scsi3_locate_pr_reg(
if (dev->dev_attrib.enforce_pr_isids)
continue;
}
- atomic_inc(&pr_reg->pr_res_holders);
- smp_mb__after_atomic();
+ atomic_inc_mb(&pr_reg->pr_res_holders);
spin_unlock(&pr_tmpl->registration_lock);
return pr_reg;
}
@@ -1124,8 +1116,7 @@ static struct t10_pr_registration *__core_scsi3_locate_pr_reg(
if (strcmp(isid, pr_reg->pr_reg_isid))
continue;

- atomic_inc(&pr_reg->pr_res_holders);
- smp_mb__after_atomic();
+ atomic_inc_mb(&pr_reg->pr_res_holders);
spin_unlock(&pr_tmpl->registration_lock);
return pr_reg;
}
@@ -1154,8 +1145,7 @@ static struct t10_pr_registration *core_scsi3_locate_pr_reg(

static void core_scsi3_put_pr_reg(struct t10_pr_registration *pr_reg)
{
- atomic_dec(&pr_reg->pr_res_holders);
- smp_mb__after_atomic();
+ atomic_dec_mb(&pr_reg->pr_res_holders);
}

static int core_scsi3_check_implicit_release(
@@ -1348,8 +1338,7 @@ static void core_scsi3_tpg_undepend_item(struct se_portal_group *tpg)
configfs_undepend_item(tpg->se_tpg_tfo->tf_subsys,
&tpg->tpg_group.cg_item);

- atomic_dec(&tpg->tpg_pr_ref_count);
- smp_mb__after_atomic();
+ atomic_dec_mb(&tpg->tpg_pr_ref_count);
}

static int core_scsi3_nodeacl_depend_item(struct se_node_acl *nacl)
@@ -1368,16 +1357,14 @@ static void core_scsi3_nodeacl_undepend_item(struct se_node_acl *nacl)
struct se_portal_group *tpg = nacl->se_tpg;

if (nacl->dynamic_node_acl) {
- atomic_dec(&nacl->acl_pr_ref_count);
- smp_mb__after_atomic();
+ atomic_dec_mb(&nacl->acl_pr_ref_count);
return;
}

configfs_undepend_item(tpg->se_tpg_tfo->tf_subsys,
&nacl->acl_group.cg_item);

- atomic_dec(&nacl->acl_pr_ref_count);
- smp_mb__after_atomic();
+ atomic_dec_mb(&nacl->acl_pr_ref_count);
}

static int core_scsi3_lunacl_depend_item(struct se_dev_entry *se_deve)
@@ -1407,8 +1394,7 @@ static void core_scsi3_lunacl_undepend_item(struct se_dev_entry *se_deve)
* For nacl->dynamic_node_acl=1
*/
if (!lun_acl) {
- atomic_dec(&se_deve->pr_ref_count);
- smp_mb__after_atomic();
+ atomic_dec_mb(&se_deve->pr_ref_count);
return;
}
nacl = lun_acl->se_lun_nacl;
@@ -1417,8 +1403,7 @@ static void core_scsi3_lunacl_undepend_item(struct se_dev_entry *se_deve)
configfs_undepend_item(tpg->se_tpg_tfo->tf_subsys,
&lun_acl->se_lun_group.cg_item);

- atomic_dec(&se_deve->pr_ref_count);
- smp_mb__after_atomic();
+ atomic_dec_mb(&se_deve->pr_ref_count);
}

static sense_reason_t
@@ -1551,15 +1536,13 @@ core_scsi3_decode_spec_i_port(
if (!i_str)
continue;

- atomic_inc(&tmp_tpg->tpg_pr_ref_count);
- smp_mb__after_atomic();
+ atomic_inc_mb(&tmp_tpg->tpg_pr_ref_count);
spin_unlock(&dev->se_port_lock);

if (core_scsi3_tpg_depend_item(tmp_tpg)) {
pr_err(" core_scsi3_tpg_depend_item()"
" for tmp_tpg\n");
- atomic_dec(&tmp_tpg->tpg_pr_ref_count);
- smp_mb__after_atomic();
+ atomic_dec_mb(&tmp_tpg->tpg_pr_ref_count);
ret = TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
goto out_unmap;
}
@@ -1571,10 +1554,8 @@ core_scsi3_decode_spec_i_port(
spin_lock_irq(&tmp_tpg->acl_node_lock);
dest_node_acl = __core_tpg_get_initiator_node_acl(
tmp_tpg, i_str);
- if (dest_node_acl) {
- atomic_inc(&dest_node_acl->acl_pr_ref_count);
- smp_mb__after_atomic();
- }
+ if (dest_node_acl)
+ atomic_inc_mb(&dest_node_acl->acl_pr_ref_count);
spin_unlock_irq(&tmp_tpg->acl_node_lock);

if (!dest_node_acl) {
@@ -1586,8 +1567,7 @@ core_scsi3_decode_spec_i_port(
if (core_scsi3_nodeacl_depend_item(dest_node_acl)) {
pr_err("configfs_depend_item() failed"
" for dest_node_acl->acl_group\n");
- atomic_dec(&dest_node_acl->acl_pr_ref_count);
- smp_mb__after_atomic();
+ atomic_dec_mb(&dest_node_acl->acl_pr_ref_count);
core_scsi3_tpg_undepend_item(tmp_tpg);
ret = TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
goto out_unmap;
@@ -1646,8 +1626,7 @@ core_scsi3_decode_spec_i_port(
if (core_scsi3_lunacl_depend_item(dest_se_deve)) {
pr_err("core_scsi3_lunacl_depend_item()"
" failed\n");
- atomic_dec(&dest_se_deve->pr_ref_count);
- smp_mb__after_atomic();
+ atomic_dec_mb(&dest_se_deve->pr_ref_count);
core_scsi3_nodeacl_undepend_item(dest_node_acl);
core_scsi3_tpg_undepend_item(dest_tpg);
ret = TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
@@ -3167,15 +3146,13 @@ core_scsi3_emulate_pro_register_and_move(struct se_cmd *cmd, u64 res_key,
if (!dest_tf_ops)
continue;

- atomic_inc(&dest_se_tpg->tpg_pr_ref_count);
- smp_mb__after_atomic();
+ atomic_inc_mb(&dest_se_tpg->tpg_pr_ref_count);
spin_unlock(&dev->se_port_lock);

if (core_scsi3_tpg_depend_item(dest_se_tpg)) {
pr_err("core_scsi3_tpg_depend_item() failed"
" for dest_se_tpg\n");
- atomic_dec(&dest_se_tpg->tpg_pr_ref_count);
- smp_mb__after_atomic();
+ atomic_dec_mb(&dest_se_tpg->tpg_pr_ref_count);
ret = TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
goto out_put_pr_reg;
}
@@ -3271,10 +3248,8 @@ after_iport_check:
spin_lock_irq(&dest_se_tpg->acl_node_lock);
dest_node_acl = __core_tpg_get_initiator_node_acl(dest_se_tpg,
initiator_str);
- if (dest_node_acl) {
- atomic_inc(&dest_node_acl->acl_pr_ref_count);
- smp_mb__after_atomic();
- }
+ if (dest_node_acl)
+ atomic_inc_mb(&dest_node_acl->acl_pr_ref_count);
spin_unlock_irq(&dest_se_tpg->acl_node_lock);

if (!dest_node_acl) {
@@ -3288,8 +3263,7 @@ after_iport_check:
if (core_scsi3_nodeacl_depend_item(dest_node_acl)) {
pr_err("core_scsi3_nodeacl_depend_item() for"
" dest_node_acl\n");
- atomic_dec(&dest_node_acl->acl_pr_ref_count);
- smp_mb__after_atomic();
+ atomic_dec_mb(&dest_node_acl->acl_pr_ref_count);
dest_node_acl = NULL;
ret = TCM_INVALID_PARAMETER_LIST;
goto out;
@@ -3313,8 +3287,7 @@ after_iport_check:

if (core_scsi3_lunacl_depend_item(dest_se_deve)) {
pr_err("core_scsi3_lunacl_depend_item() failed\n");
- atomic_dec(&dest_se_deve->pr_ref_count);
- smp_mb__after_atomic();
+ atomic_dec_mb(&dest_se_deve->pr_ref_count);
dest_se_deve = NULL;
ret = TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
goto out;
@@ -3879,8 +3852,7 @@ core_scsi3_pri_read_full_status(struct se_cmd *cmd)
se_tpg = pr_reg->pr_reg_nacl->se_tpg;
add_desc_len = 0;

- atomic_inc(&pr_reg->pr_res_holders);
- smp_mb__after_atomic();
+ atomic_inc_mb(&pr_reg->pr_res_holders);
spin_unlock(&pr_tmpl->registration_lock);
/*
* Determine expected length of $FABRIC_MOD specific
@@ -3893,8 +3865,7 @@ core_scsi3_pri_read_full_status(struct se_cmd *cmd)
pr_warn("SPC-3 PRIN READ_FULL_STATUS ran"
" out of buffer: %d\n", cmd->data_length);
spin_lock(&pr_tmpl->registration_lock);
- atomic_dec(&pr_reg->pr_res_holders);
- smp_mb__after_atomic();
+ atomic_dec_mb(&pr_reg->pr_res_holders);
break;
}
/*
@@ -3955,8 +3926,7 @@ core_scsi3_pri_read_full_status(struct se_cmd *cmd)
se_nacl, pr_reg, &format_code, &buf[off+4]);

spin_lock(&pr_tmpl->registration_lock);
- atomic_dec(&pr_reg->pr_res_holders);
- smp_mb__after_atomic();
+ atomic_dec_mb(&pr_reg->pr_res_holders);
/*
* Set the ADDITIONAL DESCRIPTOR LENGTH
*/
diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c
index 0b43761ed85f..115632ee3ec8 100644
--- a/drivers/target/target_core_transport.c
+++ b/drivers/target/target_core_transport.c
@@ -752,8 +752,7 @@ void target_qf_do_work(struct work_struct *work)

list_for_each_entry_safe(cmd, cmd_tmp, &qf_cmd_list, se_qf_node) {
list_del(&cmd->se_qf_node);
- atomic_dec(&dev->dev_qf_count);
- smp_mb__after_atomic();
+ atomic_dec_mb(&dev->dev_qf_count);

pr_debug("Processing %s cmd: %p QUEUE_FULL in work queue"
" context: %s\n", cmd->se_tfo->get_fabric_name(), cmd,
@@ -1721,8 +1720,7 @@ static bool target_handle_task_attr(struct se_cmd *cmd)
cmd->t_task_cdb[0], cmd->se_ordered_id);
return false;
case MSG_ORDERED_TAG:
- atomic_inc(&dev->dev_ordered_sync);
- smp_mb__after_atomic();
+ atomic_inc_mb(&dev->dev_ordered_sync);

pr_debug("Added ORDERED for CDB: 0x%02x to ordered list, "
" se_ordered_id: %u\n",
@@ -1739,8 +1737,7 @@ static bool target_handle_task_attr(struct se_cmd *cmd)
/*
* For SIMPLE and UNTAGGED Task Attribute commands
*/
- atomic_inc(&dev->simple_cmds);
- smp_mb__after_atomic();
+ atomic_inc_mb(&dev->simple_cmds);
break;
}

@@ -1844,8 +1841,7 @@ static void transport_complete_task_attr(struct se_cmd *cmd)
return;

if (cmd->sam_task_attr == MSG_SIMPLE_TAG) {
- atomic_dec(&dev->simple_cmds);
- smp_mb__after_atomic();
+ atomic_dec_mb(&dev->simple_cmds);
dev->dev_cur_ordered_id++;
pr_debug("Incremented dev->dev_cur_ordered_id: %u for"
" SIMPLE: %u\n", dev->dev_cur_ordered_id,
@@ -1856,8 +1852,7 @@ static void transport_complete_task_attr(struct se_cmd *cmd)
" HEAD_OF_QUEUE: %u\n", dev->dev_cur_ordered_id,
cmd->se_ordered_id);
} else if (cmd->sam_task_attr == MSG_ORDERED_TAG) {
- atomic_dec(&dev->dev_ordered_sync);
- smp_mb__after_atomic();
+ atomic_dec_mb(&dev->dev_ordered_sync);

dev->dev_cur_ordered_id++;
pr_debug("Incremented dev_cur_ordered_id: %u for ORDERED:"
@@ -1915,8 +1910,7 @@ static void transport_handle_queue_full(
{
spin_lock_irq(&dev->qf_cmd_lock);
list_add_tail(&cmd->se_qf_node, &cmd->se_dev->qf_cmd_list);
- atomic_inc(&dev->dev_qf_count);
- smp_mb__after_atomic();
+ atomic_inc_mb(&dev->dev_qf_count);
spin_unlock_irq(&cmd->se_dev->qf_cmd_lock);

schedule_work(&cmd->se_dev->qf_work_queue);
diff --git a/drivers/target/target_core_ua.c b/drivers/target/target_core_ua.c
index 101858e245b3..1738b1646988 100644
--- a/drivers/target/target_core_ua.c
+++ b/drivers/target/target_core_ua.c
@@ -161,8 +161,7 @@ int core_scsi3_ua_allocate(
spin_unlock(&deve->ua_lock);
spin_unlock_irq(&nacl->device_list_lock);

- atomic_inc(&deve->ua_count);
- smp_mb__after_atomic();
+ atomic_inc_mb(&deve->ua_count);
return 0;
}
list_add_tail(&ua->ua_nacl_list, &deve->ua_list);
@@ -174,8 +173,7 @@ int core_scsi3_ua_allocate(
nacl->se_tpg->se_tpg_tfo->get_fabric_name(), unpacked_lun,
asc, ascq);

- atomic_inc(&deve->ua_count);
- smp_mb__after_atomic();
+ atomic_inc_mb(&deve->ua_count);
return 0;
}

@@ -189,8 +187,7 @@ void core_scsi3_ua_release_all(
list_del(&ua->ua_nacl_list);
kmem_cache_free(se_ua_cache, ua);

- atomic_dec(&deve->ua_count);
- smp_mb__after_atomic();
+ atomic_dec_mb(&deve->ua_count);
}
spin_unlock(&deve->ua_lock);
}
@@ -250,8 +247,7 @@ void core_scsi3_ua_for_check_condition(
list_del(&ua->ua_nacl_list);
kmem_cache_free(se_ua_cache, ua);

- atomic_dec(&deve->ua_count);
- smp_mb__after_atomic();
+ atomic_dec_mb(&deve->ua_count);
}
spin_unlock(&deve->ua_lock);
spin_unlock_irq(&nacl->device_list_lock);
@@ -309,8 +305,7 @@ int core_scsi3_ua_clear_for_request_sense(
list_del(&ua->ua_nacl_list);
kmem_cache_free(se_ua_cache, ua);

- atomic_dec(&deve->ua_count);
- smp_mb__after_atomic();
+ atomic_dec_mb(&deve->ua_count);
}
spin_unlock(&deve->ua_lock);
spin_unlock_irq(&nacl->device_list_lock);
diff --git a/include/target/target_core_base.h b/include/target/target_core_base.h
index 9ec9864ecf38..b106240d8385 100644
--- a/include/target/target_core_base.h
+++ b/include/target/target_core_base.h
@@ -903,4 +903,18 @@ struct se_wwn {
struct config_group fabric_stat_group;
};

+static inline void atomic_inc_mb(atomic_t *v)
+{
+ smp_mb__before_atomic();
+ atomic_inc(v);
+ smp_mb__after_atomic();
+}
+
+static inline void atomic_dec_mb(atomic_t *v)
+{
+ smp_mb__before_atomic();
+ atomic_dec(v);
+ smp_mb__after_atomic();
+}
+
#endif /* TARGET_CORE_BASE_H */
--
2.0.0.rc0.1.g7b2ba98
Joern Engel
2014-09-16 20:23:11 UTC
Permalink
atomic_inc_return() already does an implicit memory barrier and the
second case was moved from an atomic to a plain flag operation. If a
barrier were needed in the second case, it would have to be smp_mb(),
not a variant optimized away for x86 and other architectures.

Signed-off-by: Joern Engel <***@logfs.org>
---
drivers/target/target_core_transport.c | 2 --
1 file changed, 2 deletions(-)

diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c
index 7fa62fc93e0b..0b43761ed85f 100644
--- a/drivers/target/target_core_transport.c
+++ b/drivers/target/target_core_transport.c
@@ -1166,7 +1166,6 @@ transport_check_alloc_task_attr(struct se_cmd *cmd)
* Dormant to Active status.
*/
cmd->se_ordered_id = atomic_inc_return(&dev->dev_ordered_id);
- smp_mb__after_atomic();
pr_debug("Allocated se_ordered_id: %u for Task Attr: 0x%02x on %s\n",
cmd->se_ordered_id, cmd->sam_task_attr,
dev->transport->name);
@@ -2896,7 +2895,6 @@ void transport_send_task_abort(struct se_cmd *cmd)
if (cmd->se_tfo->write_pending_status(cmd) != 0) {
cmd->transport_state |= CMD_T_ABORTED;
cmd->se_cmd_flags |= SCF_SEND_DELAYED_TAS;
- smp_mb__after_atomic();
return;
}
}
--
2.0.0.rc0.1.g7b2ba98
Joern Engel
2014-09-16 20:23:13 UTC
Permalink
list_for_each_entry_safe is necessary if list objects are deleted from
the list while traversing it. Not the case here, so we can use the base
list_for_each_entry variant.

Signed-off-by: Joern Engel <***@logfs.org>
---
drivers/target/target_core_tmr.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/target/target_core_tmr.c b/drivers/target/target_core_tmr.c
index 83de7aec4aac..b426746589c0 100644
--- a/drivers/target/target_core_tmr.c
+++ b/drivers/target/target_core_tmr.c
@@ -115,13 +115,12 @@ void core_tmr_abort_task(
struct se_tmr_req *tmr,
struct se_session *se_sess)
{
- struct se_cmd *se_cmd, *tmp_cmd;
+ truct se_cmd *se_cmd;
unsigned long flags;
int ref_tag;

spin_lock_irqsave(&se_sess->sess_cmd_lock, flags);
- list_for_each_entry_safe(se_cmd, tmp_cmd,
- &se_sess->sess_cmd_list, se_cmd_list) {
+ list_for_each_entry(se_cmd, &se_sess->sess_cmd_list, se_cmd_list) {

if (dev != se_cmd->se_dev)
continue;
--
2.0.0.rc0.1.g7b2ba98
Andy Grover
2014-09-16 22:55:19 UTC
Permalink
Post by Joern Engel
list_for_each_entry_safe is necessary if list objects are deleted from
the list while traversing it. Not the case here, so we can use the base
list_for_each_entry variant.
---
drivers/target/target_core_tmr.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/target/target_core_tmr.c b/drivers/target/target_core_tmr.c
index 83de7aec4aac..b426746589c0 100644
--- a/drivers/target/target_core_tmr.c
+++ b/drivers/target/target_core_tmr.c
@@ -115,13 +115,12 @@ void core_tmr_abort_task(
struct se_tmr_req *tmr,
struct se_session *se_sess)
{
- struct se_cmd *se_cmd, *tmp_cmd;
+ truct se_cmd *se_cmd;
Typo.

HTH -- Andy
Jörn Engel
2014-09-17 21:24:06 UTC
Permalink
list_for_each_entry_safe is necessary if list objects are deleted fr=
om
the list while traversing it. Not the case here, so we can use the =
base
list_for_each_entry variant.
---
drivers/target/target_core_tmr.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/target/target_core_tmr.c b/drivers/target/targe=
t_core_tmr.c
index 83de7aec4aac..b426746589c0 100644
--- a/drivers/target/target_core_tmr.c
+++ b/drivers/target/target_core_tmr.c
@@ -115,13 +115,12 @@ void core_tmr_abort_task(
struct se_tmr_req *tmr,
struct se_session *se_sess)
{
- struct se_cmd *se_cmd, *tmp_cmd;
+ truct se_cmd *se_cmd;
=20
Typo.
Looks like Some Idiot(tm) should be redoing this patchset. Thanks!

J=C3=B6rn

--
Invincibility is in oneself, vulnerability is in the opponent.
-- Sun Tzu
Joern Engel
2014-09-16 20:23:14 UTC
Permalink
Signed-off-by: Joern Engel <***@logfs.org>
---
drivers/scsi/qla2xxx/qla_target.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_target.c b/drivers/scsi/qla2xxx/qla_target.c
index e632e14180cf..577058e4537f 100644
--- a/drivers/scsi/qla2xxx/qla_target.c
+++ b/drivers/scsi/qla2xxx/qla_target.c
@@ -401,7 +401,7 @@ static int qlt_reset(struct scsi_qla_host *vha, void *iocb, int mcmd)
#if 0 /* FIXME: Re-enable Global event handling.. */
/* Global event */
atomic_inc(&ha->tgt.qla_tgt->tgt_global_resets_count);
- qlt_clear_tgt_db(ha->tgt.qla_tgt, 1);
+ qlt_clear_tgt_db(ha->tgt.qla_tgt);
if (!list_empty(&ha->tgt.qla_tgt->sess_list)) {
sess = list_entry(ha->tgt.qla_tgt->sess_list.next,
typeof(*sess), sess_list_entry);
@@ -483,7 +483,7 @@ static void qlt_schedule_sess_for_deletion(struct qla_tgt_sess *sess,
}

/* ha->hardware_lock supposed to be held on entry */
-static void qlt_clear_tgt_db(struct qla_tgt *tgt, bool local_only)
+static void qlt_clear_tgt_db(struct qla_tgt *tgt)
{
struct qla_tgt_sess *sess;

@@ -835,7 +835,7 @@ int qlt_stop_phase1(struct qla_tgt *tgt)
mutex_lock(&vha->vha_tgt.tgt_mutex);
spin_lock_irqsave(&ha->hardware_lock, flags);
tgt->tgt_stop = 1;
- qlt_clear_tgt_db(tgt, true);
+ qlt_clear_tgt_db(tgt);
spin_unlock_irqrestore(&ha->hardware_lock, flags);
mutex_unlock(&vha->vha_tgt.tgt_mutex);
mutex_unlock(&qla_tgt_mutex);
--
2.0.0.rc0.1.g7b2ba98
Joern Engel
2014-09-16 20:23:16 UTC
Permalink
If you read the code, it is clearly wrong.

Signed-off-by: Joern Engel <***@logfs.org>
---
drivers/scsi/qla2xxx/qla_target.c | 3 ---
1 file changed, 3 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_target.c b/drivers/scsi/qla2xxx/qla_target.c
index c22b1c141fec..c87732b07f3f 100644
--- a/drivers/scsi/qla2xxx/qla_target.c
+++ b/drivers/scsi/qla2xxx/qla_target.c
@@ -1690,9 +1690,6 @@ static inline int qlt_has_data(struct qla_tgt_cmd *cmd)
return cmd->bufflen > 0;
}

-/*
- * Called without ha->hardware_lock held
- */
static int qlt_pre_xmit_response(struct qla_tgt_cmd *cmd,
struct qla_tgt_prm *prm, int xmit_type, uint8_t scsi_status,
uint32_t *full_req_cnt)
--
2.0.0.rc0.1.g7b2ba98
Joern Engel
2014-09-16 20:23:10 UTC
Permalink
And while at it, do minimal coding style fixes in the area.

Signed-off-by: Joern Engel <***@logfs.org>
---
drivers/target/target_core_tmr.c | 19 +++++++------------
1 file changed, 7 insertions(+), 12 deletions(-)

diff --git a/drivers/target/target_core_tmr.c b/drivers/target/target_core_tmr.c
index f7cd95e8111a..83de7aec4aac 100644
--- a/drivers/target/target_core_tmr.c
+++ b/drivers/target/target_core_tmr.c
@@ -64,21 +64,17 @@ int core_tmr_alloc_req(
}
EXPORT_SYMBOL(core_tmr_alloc_req);

-void core_tmr_release_req(
- struct se_tmr_req *tmr)
+void core_tmr_release_req(struct se_tmr_req *tmr)
{
struct se_device *dev = tmr->tmr_dev;
unsigned long flags;

- if (!dev) {
- kfree(tmr);
- return;
+ if (dev) {
+ spin_lock_irqsave(&dev->se_tmr_lock, flags);
+ list_del(&tmr->tmr_list);
+ spin_unlock_irqrestore(&dev->se_tmr_lock, flags);
}

- spin_lock_irqsave(&dev->se_tmr_lock, flags);
- list_del(&tmr->tmr_list);
- spin_unlock_irqrestore(&dev->se_tmr_lock, flags);
-
kfree(tmr);
}

@@ -90,9 +86,8 @@ static void core_tmr_handle_tas_abort(
bool remove = true;
/*
* TASK ABORTED status (TAS) bit support
- */
- if ((tmr_nacl &&
- (tmr_nacl != cmd->se_sess->se_node_acl)) && tas) {
+ */
+ if ((tmr_nacl && (tmr_nacl != cmd->se_sess->se_node_acl)) && tas) {
remove = false;
transport_send_task_abort(cmd);
}
--
2.0.0.rc0.1.g7b2ba98
Joern Engel
2014-09-16 20:23:15 UTC
Permalink
Also removes the declarations from the header - including two
declarations without function definitions or callers.

Signed-off-by: Joern Engel <***@logfs.org>
---
drivers/scsi/qla2xxx/qla_target.c | 10 +++++-----
drivers/scsi/qla2xxx/qla_target.h | 10 ----------
2 files changed, 5 insertions(+), 15 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_target.c b/drivers/scsi/qla2xxx/qla_target.c
index 577058e4537f..c22b1c141fec 100644
--- a/drivers/scsi/qla2xxx/qla_target.c
+++ b/drivers/scsi/qla2xxx/qla_target.c
@@ -101,6 +101,7 @@ static void qlt_send_term_exchange(struct scsi_qla_host *ha, struct qla_tgt_cmd
*cmd, struct atio_from_isp *atio, int ha_locked);
static void qlt_reject_free_srr_imm(struct scsi_qla_host *ha,
struct qla_tgt_srr_imm *imm, int ha_lock);
+static void qlt_disable_vha(struct scsi_qla_host *vha);
/*
* Global Variables
*/
@@ -178,7 +179,7 @@ struct scsi_qla_host *qlt_find_host_by_vp_idx(struct scsi_qla_host *vha,
return NULL;
}

-void qlt_24xx_atio_pkt_all_vps(struct scsi_qla_host *vha,
+static void qlt_24xx_atio_pkt_all_vps(struct scsi_qla_host *vha,
struct atio_from_isp *atio)
{
ql_dbg(ql_dbg_tgt, vha, 0xe072,
@@ -5024,7 +5025,7 @@ void qlt_lport_deregister(struct scsi_qla_host *vha)
EXPORT_SYMBOL(qlt_lport_deregister);

/* Must be called under HW lock */
-void qlt_set_mode(struct scsi_qla_host *vha)
+static void qlt_set_mode(struct scsi_qla_host *vha)
{
struct qla_hw_data *ha = vha->hw;

@@ -5045,7 +5046,7 @@ void qlt_set_mode(struct scsi_qla_host *vha)
}

/* Must be called under HW lock */
-void qlt_clear_mode(struct scsi_qla_host *vha)
+static void qlt_clear_mode(struct scsi_qla_host *vha)
{
struct qla_hw_data *ha = vha->hw;

@@ -5109,8 +5110,7 @@ EXPORT_SYMBOL(qlt_enable_vha);
*
* Disable Target Mode and reset the adapter
*/
-void
-qlt_disable_vha(struct scsi_qla_host *vha)
+static void qlt_disable_vha(struct scsi_qla_host *vha)
{
struct qla_hw_data *ha = vha->hw;
struct qla_tgt *tgt = vha->vha_tgt.qla_tgt;
diff --git a/drivers/scsi/qla2xxx/qla_target.h b/drivers/scsi/qla2xxx/qla_target.h
index d1d24fb0160a..bc1aee78d934 100644
--- a/drivers/scsi/qla2xxx/qla_target.h
+++ b/drivers/scsi/qla2xxx/qla_target.h
@@ -1003,10 +1003,6 @@ struct qla_tgt_srr_ctio {


extern struct qla_tgt_data qla_target;
-/*
- * Internal function prototypes
- */
-void qlt_disable_vha(struct scsi_qla_host *);

/*
* Function prototypes for qla_target.c logic used by qla2xxx LLD code.
@@ -1019,8 +1015,6 @@ extern void qlt_lport_deregister(struct scsi_qla_host *);
extern void qlt_unreg_sess(struct qla_tgt_sess *);
extern void qlt_fc_port_added(struct scsi_qla_host *, fc_port_t *);
extern void qlt_fc_port_deleted(struct scsi_qla_host *, fc_port_t *);
-extern void qlt_set_mode(struct scsi_qla_host *ha);
-extern void qlt_clear_mode(struct scsi_qla_host *ha);
extern int __init qlt_init(void);
extern void qlt_exit(void);
extern void qlt_update_vp_map(struct scsi_qla_host *, int);
@@ -1053,13 +1047,9 @@ static inline void qla_reverse_ini_mode(struct scsi_qla_host *ha)
/*
* Exported symbols from qla_target.c LLD logic used by qla2xxx code..
*/
-extern void qlt_24xx_atio_pkt_all_vps(struct scsi_qla_host *,
- struct atio_from_isp *);
extern void qlt_response_pkt_all_vps(struct scsi_qla_host *, response_t *);
extern int qlt_rdy_to_xfer(struct qla_tgt_cmd *);
extern int qlt_xmit_response(struct qla_tgt_cmd *, int, uint8_t);
-extern int qlt_rdy_to_xfer_dif(struct qla_tgt_cmd *);
-extern int qlt_xmit_response_dif(struct qla_tgt_cmd *, int, uint8_t);
extern void qlt_xmit_tm_rsp(struct qla_tgt_mgmt_cmd *);
extern void qlt_free_mcmd(struct qla_tgt_mgmt_cmd *);
extern void qlt_free_cmd(struct qla_tgt_cmd *cmd);
--
2.0.0.rc0.1.g7b2ba98
Joern Engel
2014-09-16 20:23:17 UTC
Permalink
Removes the goto logic with two identical function calls and in
particular removes the "just in case" msleep(250). The latter has no
reasonable explanation why it might be useful and in my experience is
not.

Signed-off-by: Joern Engel <***@logfs.org>
---
drivers/scsi/qla2xxx/qla_target.c | 14 ++++----------
1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_target.c b/drivers/scsi/qla2xxx/qla_target.c
index c87732b07f3f..275becf7e939 100644
--- a/drivers/scsi/qla2xxx/qla_target.c
+++ b/drivers/scsi/qla2xxx/qla_target.c
@@ -2680,14 +2680,11 @@ static void qlt_send_term_exchange(struct scsi_qla_host *vha,
if (qlt_issue_marker(vha, ha_locked) < 0)
return;

- if (ha_locked) {
- rc = __qlt_send_term_exchange(vha, cmd, atio);
- goto done;
- }
- spin_lock_irqsave(&vha->hw->hardware_lock, flags);
+ if (!ha_locked)
+ spin_lock_irqsave(&vha->hw->hardware_lock, flags);
rc = __qlt_send_term_exchange(vha, cmd, atio);
- spin_unlock_irqrestore(&vha->hw->hardware_lock, flags);
-done:
+ if (!ha_locked)
+ spin_unlock_irqrestore(&vha->hw->hardware_lock, flags);
/*
* Terminate exchange will tell fw to release any active CTIO
* that's in FW posession and cleanup the exchange.
@@ -2700,9 +2697,6 @@ done:
* back w/some err. Free the cmd now.
*/
if ((rc == 1) && (cmd->state != QLA_TGT_STATE_ABORTED)) {
- if (!ha_locked && !in_interrupt())
- msleep(250); /* just in case */
-
if (cmd->sg_mapped)
qlt_unmap_sg(vha, cmd);
vha->hw->tgt.tgt_ops->free_cmd(cmd);
--
2.0.0.rc0.1.g7b2ba98
Joern Engel
2014-09-16 20:23:18 UTC
Permalink
Remove the inline attribute. Modern compilers ignore it and the
function has grown beyond where inline made sense anyway.
Remove the BUG_ON(!cmd->sg_mapped), and instead return if sg_mapped is
not set. Every caller is doing this check, so we might as well have it
in one place instead of four.

Signed-off-by: Joern Engel <***@logfs.org>
---
drivers/scsi/qla2xxx/qla_target.c | 19 ++++++++-----------
1 file changed, 8 insertions(+), 11 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_target.c b/drivers/scsi/qla2xxx/qla_target.c
index 275becf7e939..ae3a0e52f8b1 100644
--- a/drivers/scsi/qla2xxx/qla_target.c
+++ b/drivers/scsi/qla2xxx/qla_target.c
@@ -1409,12 +1409,13 @@ out_err:
return -1;
}

-static inline void qlt_unmap_sg(struct scsi_qla_host *vha,
- struct qla_tgt_cmd *cmd)
+static void qlt_unmap_sg(struct scsi_qla_host *vha, struct qla_tgt_cmd *cmd)
{
struct qla_hw_data *ha = vha->hw;

- BUG_ON(!cmd->sg_mapped);
+ if (!cmd->sg_mapped)
+ return;
+
pci_unmap_sg(ha->pdev, cmd->sg, cmd->sg_cnt, cmd->dma_data_direction);
cmd->sg_mapped = 0;

@@ -2400,8 +2401,7 @@ int qlt_xmit_response(struct qla_tgt_cmd *cmd, int xmit_type,
return 0;

out_unmap_unlock:
- if (cmd->sg_mapped)
- qlt_unmap_sg(vha, cmd);
+ qlt_unmap_sg(vha, cmd);
spin_unlock_irqrestore(&ha->hardware_lock, flags);

return res;
@@ -2465,8 +2465,7 @@ int qlt_rdy_to_xfer(struct qla_tgt_cmd *cmd)
return res;

out_unlock_free_unmap:
- if (cmd->sg_mapped)
- qlt_unmap_sg(vha, cmd);
+ qlt_unmap_sg(vha, cmd);
spin_unlock_irqrestore(&ha->hardware_lock, flags);

return res;
@@ -2697,8 +2696,7 @@ static void qlt_send_term_exchange(struct scsi_qla_host *vha,
* back w/some err. Free the cmd now.
*/
if ((rc == 1) && (cmd->state != QLA_TGT_STATE_ABORTED)) {
- if (cmd->sg_mapped)
- qlt_unmap_sg(vha, cmd);
+ qlt_unmap_sg(vha, cmd);
vha->hw->tgt.tgt_ops->free_cmd(cmd);
}
return;
@@ -2918,8 +2916,7 @@ static void qlt_do_ctio_completion(struct scsi_qla_host *vha, uint32_t handle,
se_cmd = &cmd->se_cmd;
tfo = se_cmd->se_tfo;

- if (cmd->sg_mapped)
- qlt_unmap_sg(vha, cmd);
+ qlt_unmap_sg(vha, cmd);

if (unlikely(status != CTIO_SUCCESS)) {
switch (status & 0xFFFF) {
--
2.0.0.rc0.1.g7b2ba98
Nicholas A. Bellinger
2014-09-19 22:54:58 UTC
Permalink
Hi Joern,
Came across these while reading code. Nothing exciting.
target: simplify core_tmr_release_req()
target: remove some smp_mb__after_atomic()s
target: encapsulate smp_mb__after_atomic()
target: simplify core_tmr_abort_task
qla_target: remove unused parameter
qla_target: make some global functions static
qla_target: remove misleading comment
qla_target: simplify qlt_send_term_exchange()
qla_target: improve qlt_unmap_sg()
qla_target: rearrange struct qla_tgt_prm
drivers/scsi/qla2xxx/qla_target.c | 52 ++++++++------------
drivers/scsi/qla2xxx/qla_target.h | 12 +----
drivers/target/loopback/tcm_loop.c | 6 +--
drivers/target/target_core_alua.c | 33 ++++---------
drivers/target/target_core_device.c | 9 ++--
drivers/target/target_core_pr.c | 90 ++++++++++++----------------------
drivers/target/target_core_tmr.c | 24 ++++-----
drivers/target/target_core_transport.c | 20 +++-----
drivers/target/target_core_ua.c | 15 ++----
include/target/target_core_base.h | 14 ++++++
10 files changed, 100 insertions(+), 175 deletions(-)
Are you planning to resend these (with compile testing.. ;), or should
they be applied as is with the minor typo fix..?

--nab
Jörn Engel
2014-09-19 23:08:53 UTC
Permalink
Came across these while reading code. Nothing exciting.
=20
target: simplify core_tmr_release_req()
target: remove some smp_mb__after_atomic()s
target: encapsulate smp_mb__after_atomic()
target: simplify core_tmr_abort_task
qla_target: remove unused parameter
qla_target: make some global functions static
qla_target: remove misleading comment
qla_target: simplify qlt_send_term_exchange()
qla_target: improve qlt_unmap_sg()
qla_target: rearrange struct qla_tgt_prm
=20
drivers/scsi/qla2xxx/qla_target.c | 52 ++++++++------------
drivers/scsi/qla2xxx/qla_target.h | 12 +----
drivers/target/loopback/tcm_loop.c | 6 +--
drivers/target/target_core_alua.c | 33 ++++---------
drivers/target/target_core_device.c | 9 ++--
drivers/target/target_core_pr.c | 90 ++++++++++++----------=
------------
drivers/target/target_core_tmr.c | 24 ++++-----
drivers/target/target_core_transport.c | 20 +++-----
drivers/target/target_core_ua.c | 15 ++----
include/target/target_core_base.h | 14 ++++++
10 files changed, 100 insertions(+), 175 deletions(-)
=20
=20
Are you planning to resend these (with compile testing.. ;), or shoul=
d
they be applied as is with the minor typo fix..?
I was planning on the former before $WORK happened. If you have free
cycles and are fine with doing the latter, I don't mind either.

J=C3=B6rn

--
When people work hard for you for a pat on the back, you've got
to give them that pat.
-- Robert Heinlein

Loading...