* [PATCH 1/3] target: remove iscsit_find_cmd_from_itt_or_dump()
2013-07-03 15:22 [PATCH 0/3] target: three cleanup patches Joern Engel
@ 2013-07-03 15:22 ` Joern Engel
2013-07-07 0:52 ` Nicholas A. Bellinger
2013-07-03 15:22 ` [PATCH 2/3] target: remove unused codes from enum tcm_tmrsp_table Joern Engel
2013-07-03 15:22 ` [PATCH 3/3] target: make queue_tm_rsp() return void Joern Engel
2 siblings, 1 reply; 7+ messages in thread
From: Joern Engel @ 2013-07-03 15:22 UTC (permalink / raw)
To: Nicholas A. Bellinger; +Cc: linux-kernel, target-devel, Joern Engel
Code is a copy of iscsit_find_cmd_from_itt(). Afaics this is debug code
from at least two years ago. Either the bug in question has been long
fixed or this debug code doesn't help fixing it. Whichever way you look
at it, we should remove the debug code.
Signed-off-by: Joern Engel <joern@logfs.org>
---
drivers/target/iscsi/iscsi_target.c | 3 +--
drivers/target/iscsi/iscsi_target_util.c | 24 ------------------------
drivers/target/iscsi/iscsi_target_util.h | 2 --
3 files changed, 1 insertion(+), 28 deletions(-)
diff --git a/drivers/target/iscsi/iscsi_target.c b/drivers/target/iscsi/iscsi_target.c
index 7ea246a..49346b3 100644
--- a/drivers/target/iscsi/iscsi_target.c
+++ b/drivers/target/iscsi/iscsi_target.c
@@ -1213,8 +1213,7 @@ static int iscsit_handle_data_out(struct iscsi_conn *conn, unsigned char *buf)
buf, conn);
}
- cmd = iscsit_find_cmd_from_itt_or_dump(conn, hdr->itt,
- payload_length);
+ cmd = iscsit_find_cmd_from_itt(conn, hdr->itt);
if (!cmd)
return 0;
diff --git a/drivers/target/iscsi/iscsi_target_util.c b/drivers/target/iscsi/iscsi_target_util.c
index 7ce3505..e59dec0 100644
--- a/drivers/target/iscsi/iscsi_target_util.c
+++ b/drivers/target/iscsi/iscsi_target_util.c
@@ -369,30 +369,6 @@ struct iscsi_cmd *iscsit_find_cmd_from_itt(
return NULL;
}
-struct iscsi_cmd *iscsit_find_cmd_from_itt_or_dump(
- struct iscsi_conn *conn,
- itt_t init_task_tag,
- u32 length)
-{
- struct iscsi_cmd *cmd;
-
- spin_lock_bh(&conn->cmd_lock);
- list_for_each_entry(cmd, &conn->conn_cmd_list, i_conn_node) {
- if (cmd->init_task_tag == init_task_tag) {
- spin_unlock_bh(&conn->cmd_lock);
- return cmd;
- }
- }
- spin_unlock_bh(&conn->cmd_lock);
-
- pr_err("Unable to locate ITT: 0x%08x on CID: %hu,"
- " dumping payload\n", init_task_tag, conn->cid);
- if (length)
- iscsit_dump_data_payload(conn, length, 1);
-
- return NULL;
-}
-
struct iscsi_cmd *iscsit_find_cmd_from_ttt(
struct iscsi_conn *conn,
u32 targ_xfer_tag)
diff --git a/drivers/target/iscsi/iscsi_target_util.h b/drivers/target/iscsi/iscsi_target_util.h
index 894d0f8..9614cb9 100644
--- a/drivers/target/iscsi/iscsi_target_util.h
+++ b/drivers/target/iscsi/iscsi_target_util.h
@@ -15,8 +15,6 @@ extern struct iscsi_r2t *iscsit_get_holder_for_r2tsn(struct iscsi_cmd *, u32);
int iscsit_sequence_cmd(struct iscsi_conn *conn, struct iscsi_cmd *cmd, __be32 cmdsn);
extern int iscsit_check_unsolicited_dataout(struct iscsi_cmd *, unsigned char *);
extern struct iscsi_cmd *iscsit_find_cmd_from_itt(struct iscsi_conn *, itt_t);
-extern struct iscsi_cmd *iscsit_find_cmd_from_itt_or_dump(struct iscsi_conn *,
- itt_t, u32);
extern struct iscsi_cmd *iscsit_find_cmd_from_ttt(struct iscsi_conn *, u32);
extern int iscsit_find_cmd_for_recovery(struct iscsi_session *, struct iscsi_cmd **,
struct iscsi_conn_recovery **, itt_t);
--
1.7.10.4
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH 1/3] target: remove iscsit_find_cmd_from_itt_or_dump()
2013-07-03 15:22 ` [PATCH 1/3] target: remove iscsit_find_cmd_from_itt_or_dump() Joern Engel
@ 2013-07-07 0:52 ` Nicholas A. Bellinger
0 siblings, 0 replies; 7+ messages in thread
From: Nicholas A. Bellinger @ 2013-07-07 0:52 UTC (permalink / raw)
To: Joern Engel; +Cc: linux-kernel, target-devel
On Wed, 2013-07-03 at 11:22 -0400, Joern Engel wrote:
> Code is a copy of iscsit_find_cmd_from_itt(). Afaics this is debug code
> from at least two years ago. Either the bug in question has been long
> fixed or this debug code doesn't help fixing it. Whichever way you look
> at it, we should remove the debug code.
>
> Signed-off-by: Joern Engel <joern@logfs.org>
> ---
NAK on this one.
iscsit_handle_data_out() is expected to dump the incoming per
ISCSI_OP_SCSI_DATA_OUT payload for allowing existing code to determine
the next PDU header to process after this specific 'ITT not found'
exception case occurs.
Otherwise, the traditional iscsi connection will need to be reset when
the remaining DATA_OUT payload_length is not cleared from conn_sock.
--nab
> drivers/target/iscsi/iscsi_target.c | 3 +--
> drivers/target/iscsi/iscsi_target_util.c | 24 ------------------------
> drivers/target/iscsi/iscsi_target_util.h | 2 --
> 3 files changed, 1 insertion(+), 28 deletions(-)
>
> diff --git a/drivers/target/iscsi/iscsi_target.c b/drivers/target/iscsi/iscsi_target.c
> index 7ea246a..49346b3 100644
> --- a/drivers/target/iscsi/iscsi_target.c
> +++ b/drivers/target/iscsi/iscsi_target.c
> @@ -1213,8 +1213,7 @@ static int iscsit_handle_data_out(struct iscsi_conn *conn, unsigned char *buf)
> buf, conn);
> }
>
> - cmd = iscsit_find_cmd_from_itt_or_dump(conn, hdr->itt,
> - payload_length);
> + cmd = iscsit_find_cmd_from_itt(conn, hdr->itt);
> if (!cmd)
> return 0;
>
> diff --git a/drivers/target/iscsi/iscsi_target_util.c b/drivers/target/iscsi/iscsi_target_util.c
> index 7ce3505..e59dec0 100644
> --- a/drivers/target/iscsi/iscsi_target_util.c
> +++ b/drivers/target/iscsi/iscsi_target_util.c
> @@ -369,30 +369,6 @@ struct iscsi_cmd *iscsit_find_cmd_from_itt(
> return NULL;
> }
>
> -struct iscsi_cmd *iscsit_find_cmd_from_itt_or_dump(
> - struct iscsi_conn *conn,
> - itt_t init_task_tag,
> - u32 length)
> -{
> - struct iscsi_cmd *cmd;
> -
> - spin_lock_bh(&conn->cmd_lock);
> - list_for_each_entry(cmd, &conn->conn_cmd_list, i_conn_node) {
> - if (cmd->init_task_tag == init_task_tag) {
> - spin_unlock_bh(&conn->cmd_lock);
> - return cmd;
> - }
> - }
> - spin_unlock_bh(&conn->cmd_lock);
> -
> - pr_err("Unable to locate ITT: 0x%08x on CID: %hu,"
> - " dumping payload\n", init_task_tag, conn->cid);
> - if (length)
> - iscsit_dump_data_payload(conn, length, 1);
> -
> - return NULL;
> -}
> -
> struct iscsi_cmd *iscsit_find_cmd_from_ttt(
> struct iscsi_conn *conn,
> u32 targ_xfer_tag)
> diff --git a/drivers/target/iscsi/iscsi_target_util.h b/drivers/target/iscsi/iscsi_target_util.h
> index 894d0f8..9614cb9 100644
> --- a/drivers/target/iscsi/iscsi_target_util.h
> +++ b/drivers/target/iscsi/iscsi_target_util.h
> @@ -15,8 +15,6 @@ extern struct iscsi_r2t *iscsit_get_holder_for_r2tsn(struct iscsi_cmd *, u32);
> int iscsit_sequence_cmd(struct iscsi_conn *conn, struct iscsi_cmd *cmd, __be32 cmdsn);
> extern int iscsit_check_unsolicited_dataout(struct iscsi_cmd *, unsigned char *);
> extern struct iscsi_cmd *iscsit_find_cmd_from_itt(struct iscsi_conn *, itt_t);
> -extern struct iscsi_cmd *iscsit_find_cmd_from_itt_or_dump(struct iscsi_conn *,
> - itt_t, u32);
> extern struct iscsi_cmd *iscsit_find_cmd_from_ttt(struct iscsi_conn *, u32);
> extern int iscsit_find_cmd_for_recovery(struct iscsi_session *, struct iscsi_cmd **,
> struct iscsi_conn_recovery **, itt_t);
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/3] target: remove unused codes from enum tcm_tmrsp_table
2013-07-03 15:22 [PATCH 0/3] target: three cleanup patches Joern Engel
2013-07-03 15:22 ` [PATCH 1/3] target: remove iscsit_find_cmd_from_itt_or_dump() Joern Engel
@ 2013-07-03 15:22 ` Joern Engel
2013-07-07 0:33 ` Nicholas A. Bellinger
2013-07-03 15:22 ` [PATCH 3/3] target: make queue_tm_rsp() return void Joern Engel
2 siblings, 1 reply; 7+ messages in thread
From: Joern Engel @ 2013-07-03 15:22 UTC (permalink / raw)
To: Nicholas A. Bellinger; +Cc: linux-kernel, target-devel, Joern Engel
Three have been checked for but were never set. Remove the dead code.
Also renumbers the remaining ones to a) get rid of the holes after the
removal and b) avoid a collision between TMR_FUNCTION_COMPLETE==0 and
the uninitialized case. If we failed to set a code, we should rather
fall into the default case then return success.
Signed-off-by: Joern Engel <joern@logfs.org>
---
drivers/target/iscsi/iscsi_target.c | 2 --
drivers/target/tcm_fc/tfc_cmd.c | 3 ---
include/target/target_core_base.h | 13 +++++--------
3 files changed, 5 insertions(+), 13 deletions(-)
diff --git a/drivers/target/iscsi/iscsi_target.c b/drivers/target/iscsi/iscsi_target.c
index 49346b3..25d5567 100644
--- a/drivers/target/iscsi/iscsi_target.c
+++ b/drivers/target/iscsi/iscsi_target.c
@@ -3136,8 +3136,6 @@ static u8 iscsit_convert_tcm_tmr_rsp(struct se_tmr_req *se_tmr)
return ISCSI_TMF_RSP_NO_LUN;
case TMR_TASK_MGMT_FUNCTION_NOT_SUPPORTED:
return ISCSI_TMF_RSP_NOT_SUPPORTED;
- case TMR_FUNCTION_AUTHORIZATION_FAILED:
- return ISCSI_TMF_RSP_AUTH_FAILED;
case TMR_FUNCTION_REJECTED:
default:
return ISCSI_TMF_RSP_REJECTED;
diff --git a/drivers/target/tcm_fc/tfc_cmd.c b/drivers/target/tcm_fc/tfc_cmd.c
index b406f17..7b6bb72 100644
--- a/drivers/target/tcm_fc/tfc_cmd.c
+++ b/drivers/target/tcm_fc/tfc_cmd.c
@@ -413,10 +413,7 @@ int ft_queue_tm_resp(struct se_cmd *se_cmd)
code = FCP_TMF_REJECTED;
break;
case TMR_TASK_DOES_NOT_EXIST:
- case TMR_TASK_STILL_ALLEGIANT:
- case TMR_TASK_FAILOVER_NOT_SUPPORTED:
case TMR_TASK_MGMT_FUNCTION_NOT_SUPPORTED:
- case TMR_FUNCTION_AUTHORIZATION_FAILED:
default:
code = FCP_TMF_FAILED;
break;
diff --git a/include/target/target_core_base.h b/include/target/target_core_base.h
index c4af592..4e7dd74 100644
--- a/include/target/target_core_base.h
+++ b/include/target/target_core_base.h
@@ -218,14 +218,11 @@ enum tcm_tmreq_table {
/* fabric independent task management response values */
enum tcm_tmrsp_table {
- TMR_FUNCTION_COMPLETE = 0,
- TMR_TASK_DOES_NOT_EXIST = 1,
- TMR_LUN_DOES_NOT_EXIST = 2,
- TMR_TASK_STILL_ALLEGIANT = 3,
- TMR_TASK_FAILOVER_NOT_SUPPORTED = 4,
- TMR_TASK_MGMT_FUNCTION_NOT_SUPPORTED = 5,
- TMR_FUNCTION_AUTHORIZATION_FAILED = 6,
- TMR_FUNCTION_REJECTED = 255,
+ TMR_FUNCTION_COMPLETE = 1,
+ TMR_TASK_DOES_NOT_EXIST = 2,
+ TMR_LUN_DOES_NOT_EXIST = 3,
+ TMR_TASK_MGMT_FUNCTION_NOT_SUPPORTED = 4,
+ TMR_FUNCTION_REJECTED = 5,
};
/*
--
1.7.10.4
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH 2/3] target: remove unused codes from enum tcm_tmrsp_table
2013-07-03 15:22 ` [PATCH 2/3] target: remove unused codes from enum tcm_tmrsp_table Joern Engel
@ 2013-07-07 0:33 ` Nicholas A. Bellinger
0 siblings, 0 replies; 7+ messages in thread
From: Nicholas A. Bellinger @ 2013-07-07 0:33 UTC (permalink / raw)
To: Joern Engel; +Cc: linux-kernel, target-devel
On Wed, 2013-07-03 at 11:22 -0400, Joern Engel wrote:
> Three have been checked for but were never set. Remove the dead code.
> Also renumbers the remaining ones to a) get rid of the holes after the
> removal and b) avoid a collision between TMR_FUNCTION_COMPLETE==0 and
> the uninitialized case. If we failed to set a code, we should rather
> fall into the default case then return success.
>
> Signed-off-by: Joern Engel <joern@logfs.org>
> ---
Appled to for-next.
Thanks Joern!
--nab
> drivers/target/iscsi/iscsi_target.c | 2 --
> drivers/target/tcm_fc/tfc_cmd.c | 3 ---
> include/target/target_core_base.h | 13 +++++--------
> 3 files changed, 5 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/target/iscsi/iscsi_target.c b/drivers/target/iscsi/iscsi_target.c
> index 49346b3..25d5567 100644
> --- a/drivers/target/iscsi/iscsi_target.c
> +++ b/drivers/target/iscsi/iscsi_target.c
> @@ -3136,8 +3136,6 @@ static u8 iscsit_convert_tcm_tmr_rsp(struct se_tmr_req *se_tmr)
> return ISCSI_TMF_RSP_NO_LUN;
> case TMR_TASK_MGMT_FUNCTION_NOT_SUPPORTED:
> return ISCSI_TMF_RSP_NOT_SUPPORTED;
> - case TMR_FUNCTION_AUTHORIZATION_FAILED:
> - return ISCSI_TMF_RSP_AUTH_FAILED;
> case TMR_FUNCTION_REJECTED:
> default:
> return ISCSI_TMF_RSP_REJECTED;
> diff --git a/drivers/target/tcm_fc/tfc_cmd.c b/drivers/target/tcm_fc/tfc_cmd.c
> index b406f17..7b6bb72 100644
> --- a/drivers/target/tcm_fc/tfc_cmd.c
> +++ b/drivers/target/tcm_fc/tfc_cmd.c
> @@ -413,10 +413,7 @@ int ft_queue_tm_resp(struct se_cmd *se_cmd)
> code = FCP_TMF_REJECTED;
> break;
> case TMR_TASK_DOES_NOT_EXIST:
> - case TMR_TASK_STILL_ALLEGIANT:
> - case TMR_TASK_FAILOVER_NOT_SUPPORTED:
> case TMR_TASK_MGMT_FUNCTION_NOT_SUPPORTED:
> - case TMR_FUNCTION_AUTHORIZATION_FAILED:
> default:
> code = FCP_TMF_FAILED;
> break;
> diff --git a/include/target/target_core_base.h b/include/target/target_core_base.h
> index c4af592..4e7dd74 100644
> --- a/include/target/target_core_base.h
> +++ b/include/target/target_core_base.h
> @@ -218,14 +218,11 @@ enum tcm_tmreq_table {
>
> /* fabric independent task management response values */
> enum tcm_tmrsp_table {
> - TMR_FUNCTION_COMPLETE = 0,
> - TMR_TASK_DOES_NOT_EXIST = 1,
> - TMR_LUN_DOES_NOT_EXIST = 2,
> - TMR_TASK_STILL_ALLEGIANT = 3,
> - TMR_TASK_FAILOVER_NOT_SUPPORTED = 4,
> - TMR_TASK_MGMT_FUNCTION_NOT_SUPPORTED = 5,
> - TMR_FUNCTION_AUTHORIZATION_FAILED = 6,
> - TMR_FUNCTION_REJECTED = 255,
> + TMR_FUNCTION_COMPLETE = 1,
> + TMR_TASK_DOES_NOT_EXIST = 2,
> + TMR_LUN_DOES_NOT_EXIST = 3,
> + TMR_TASK_MGMT_FUNCTION_NOT_SUPPORTED = 4,
> + TMR_FUNCTION_REJECTED = 5,
> };
>
> /*
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 3/3] target: make queue_tm_rsp() return void
2013-07-03 15:22 [PATCH 0/3] target: three cleanup patches Joern Engel
2013-07-03 15:22 ` [PATCH 1/3] target: remove iscsit_find_cmd_from_itt_or_dump() Joern Engel
2013-07-03 15:22 ` [PATCH 2/3] target: remove unused codes from enum tcm_tmrsp_table Joern Engel
@ 2013-07-03 15:22 ` Joern Engel
2013-07-07 0:31 ` Nicholas A. Bellinger
2 siblings, 1 reply; 7+ messages in thread
From: Joern Engel @ 2013-07-03 15:22 UTC (permalink / raw)
To: Nicholas A. Bellinger; +Cc: linux-kernel, target-devel, Joern Engel
The return value wasn't checked by any of the callers. Assuming this is
correct behaviour, we can simplify some code by not bothering to
generate it.
Signed-off-by: Joern Engel <joern@logfs.org>
---
drivers/infiniband/ulp/srpt/ib_srpt.c | 14 +++++---------
drivers/scsi/qla2xxx/tcm_qla2xxx.c | 4 +---
drivers/target/iscsi/iscsi_target_configfs.c | 3 +--
drivers/target/loopback/tcm_loop.c | 3 +--
drivers/target/sbp/sbp_target.c | 3 +--
drivers/target/tcm_fc/tcm_fc.h | 2 +-
drivers/target/tcm_fc/tfc_cmd.c | 5 ++---
drivers/usb/gadget/tcm_usb_gadget.c | 3 +--
drivers/vhost/tcm_vhost.c | 3 +--
include/target/target_core_fabric.h | 2 +-
10 files changed, 15 insertions(+), 27 deletions(-)
diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.c b/drivers/infiniband/ulp/srpt/ib_srpt.c
index c09d41b..2c61a28 100644
--- a/drivers/infiniband/ulp/srpt/ib_srpt.c
+++ b/drivers/infiniband/ulp/srpt/ib_srpt.c
@@ -2987,7 +2987,7 @@ static u8 tcm_to_srp_tsk_mgmt_status(const int tcm_mgmt_status)
* Callback function called by the TCM core. Must not block since it can be
* invoked on the context of the IB completion handler.
*/
-static int srpt_queue_response(struct se_cmd *cmd)
+static void srpt_queue_response(struct se_cmd *cmd)
{
struct srpt_rdma_ch *ch;
struct srpt_send_ioctx *ioctx;
@@ -2998,8 +2998,6 @@ static int srpt_queue_response(struct se_cmd *cmd)
int resp_len;
u8 srp_tm_status;
- ret = 0;
-
ioctx = container_of(cmd, struct srpt_send_ioctx, cmd);
ch = ioctx->ch;
BUG_ON(!ch);
@@ -3025,7 +3023,7 @@ static int srpt_queue_response(struct se_cmd *cmd)
|| WARN_ON_ONCE(state == SRPT_STATE_CMD_RSP_SENT))) {
atomic_inc(&ch->req_lim_delta);
srpt_abort_cmd(ioctx);
- goto out;
+ return;
}
dir = ioctx->cmd.data_direction;
@@ -3037,7 +3035,7 @@ static int srpt_queue_response(struct se_cmd *cmd)
if (ret) {
printk(KERN_ERR "xfer_data failed for tag %llu\n",
ioctx->tag);
- goto out;
+ return;
}
}
@@ -3058,9 +3056,6 @@ static int srpt_queue_response(struct se_cmd *cmd)
srpt_set_cmd_state(ioctx, SRPT_STATE_DONE);
target_put_sess_cmd(ioctx->ch->sess, &ioctx->cmd);
}
-
-out:
- return ret;
}
static int srpt_queue_status(struct se_cmd *cmd)
@@ -3073,7 +3068,8 @@ static int srpt_queue_status(struct se_cmd *cmd)
(SCF_TRANSPORT_TASK_SENSE | SCF_EMULATED_TASK_SENSE))
WARN_ON(cmd->scsi_status != SAM_STAT_CHECK_CONDITION);
ioctx->queue_status_only = true;
- return srpt_queue_response(cmd);
+ srpt_queue_response(cmd);
+ return 0;
}
static void srpt_refresh_port_work(struct work_struct *work)
diff --git a/drivers/scsi/qla2xxx/tcm_qla2xxx.c b/drivers/scsi/qla2xxx/tcm_qla2xxx.c
index 2313f06..9b75ec9 100644
--- a/drivers/scsi/qla2xxx/tcm_qla2xxx.c
+++ b/drivers/scsi/qla2xxx/tcm_qla2xxx.c
@@ -699,7 +699,7 @@ static int tcm_qla2xxx_queue_status(struct se_cmd *se_cmd)
return qlt_xmit_response(cmd, xmit_type, se_cmd->scsi_status);
}
-static int tcm_qla2xxx_queue_tm_rsp(struct se_cmd *se_cmd)
+static void tcm_qla2xxx_queue_tm_rsp(struct se_cmd *se_cmd)
{
struct se_tmr_req *se_tmr = se_cmd->se_tmr_req;
struct qla_tgt_mgmt_cmd *mcmd = container_of(se_cmd,
@@ -731,8 +731,6 @@ static int tcm_qla2xxx_queue_tm_rsp(struct se_cmd *se_cmd)
* CTIO response packet.
*/
qlt_xmit_tm_rsp(mcmd);
-
- return 0;
}
/* Local pointer to allocated TCM configfs fabric module */
diff --git a/drivers/target/iscsi/iscsi_target_configfs.c b/drivers/target/iscsi/iscsi_target_configfs.c
index 6bd5d01..0cfa96e 100644
--- a/drivers/target/iscsi/iscsi_target_configfs.c
+++ b/drivers/target/iscsi/iscsi_target_configfs.c
@@ -1582,13 +1582,12 @@ static int lio_queue_status(struct se_cmd *se_cmd)
return 0;
}
-static int lio_queue_tm_rsp(struct se_cmd *se_cmd)
+static void lio_queue_tm_rsp(struct se_cmd *se_cmd)
{
struct iscsi_cmd *cmd = container_of(se_cmd, struct iscsi_cmd, se_cmd);
cmd->i_state = ISTATE_SEND_TASKMGTRSP;
iscsit_add_cmd_to_response_queue(cmd, cmd->conn, cmd->i_state);
- return 0;
}
static char *lio_tpg_get_endpoint_wwn(struct se_portal_group *se_tpg)
diff --git a/drivers/target/loopback/tcm_loop.c b/drivers/target/loopback/tcm_loop.c
index 2d444b1..673e4fa 100644
--- a/drivers/target/loopback/tcm_loop.c
+++ b/drivers/target/loopback/tcm_loop.c
@@ -787,7 +787,7 @@ static int tcm_loop_queue_status(struct se_cmd *se_cmd)
return 0;
}
-static int tcm_loop_queue_tm_rsp(struct se_cmd *se_cmd)
+static void tcm_loop_queue_tm_rsp(struct se_cmd *se_cmd)
{
struct se_tmr_req *se_tmr = se_cmd->se_tmr_req;
struct tcm_loop_tmr *tl_tmr = se_tmr->fabric_tmr_ptr;
@@ -797,7 +797,6 @@ static int tcm_loop_queue_tm_rsp(struct se_cmd *se_cmd)
*/
atomic_set(&tl_tmr->tmr_complete, 1);
wake_up(&tl_tmr->tl_tmr_wait);
- return 0;
}
static char *tcm_loop_dump_proto_id(struct tcm_loop_hba *tl_hba)
diff --git a/drivers/target/sbp/sbp_target.c b/drivers/target/sbp/sbp_target.c
index d3536f5..e51b09a 100644
--- a/drivers/target/sbp/sbp_target.c
+++ b/drivers/target/sbp/sbp_target.c
@@ -1842,9 +1842,8 @@ static int sbp_queue_status(struct se_cmd *se_cmd)
return sbp_send_sense(req);
}
-static int sbp_queue_tm_rsp(struct se_cmd *se_cmd)
+static void sbp_queue_tm_rsp(struct se_cmd *se_cmd)
{
- return 0;
}
static int sbp_check_stop_free(struct se_cmd *se_cmd)
diff --git a/drivers/target/tcm_fc/tcm_fc.h b/drivers/target/tcm_fc/tcm_fc.h
index eea6935..0dd54a4 100644
--- a/drivers/target/tcm_fc/tcm_fc.h
+++ b/drivers/target/tcm_fc/tcm_fc.h
@@ -161,7 +161,7 @@ int ft_write_pending(struct se_cmd *);
int ft_write_pending_status(struct se_cmd *);
u32 ft_get_task_tag(struct se_cmd *);
int ft_get_cmd_state(struct se_cmd *);
-int ft_queue_tm_resp(struct se_cmd *);
+void ft_queue_tm_resp(struct se_cmd *);
/*
* other internal functions.
diff --git a/drivers/target/tcm_fc/tfc_cmd.c b/drivers/target/tcm_fc/tfc_cmd.c
index 7b6bb72..0e5a1cae 100644
--- a/drivers/target/tcm_fc/tfc_cmd.c
+++ b/drivers/target/tcm_fc/tfc_cmd.c
@@ -394,14 +394,14 @@ static void ft_send_tm(struct ft_cmd *cmd)
/*
* Send status from completed task management request.
*/
-int ft_queue_tm_resp(struct se_cmd *se_cmd)
+void ft_queue_tm_resp(struct se_cmd *se_cmd)
{
struct ft_cmd *cmd = container_of(se_cmd, struct ft_cmd, se_cmd);
struct se_tmr_req *tmr = se_cmd->se_tmr_req;
enum fcp_resp_rsp_codes code;
if (cmd->aborted)
- return 0;
+ return;
switch (tmr->response) {
case TMR_FUNCTION_COMPLETE:
code = FCP_TMF_CMPL;
@@ -421,7 +421,6 @@ int ft_queue_tm_resp(struct se_cmd *se_cmd)
pr_debug("tmr fn %d resp %d fcp code %d\n",
tmr->function, tmr->response, code);
ft_send_resp_code(cmd, code);
- return 0;
}
static void ft_send_work(struct work_struct *work);
diff --git a/drivers/usb/gadget/tcm_usb_gadget.c b/drivers/usb/gadget/tcm_usb_gadget.c
index 7cacd6a..0ff3339 100644
--- a/drivers/usb/gadget/tcm_usb_gadget.c
+++ b/drivers/usb/gadget/tcm_usb_gadget.c
@@ -1467,9 +1467,8 @@ static int usbg_get_cmd_state(struct se_cmd *se_cmd)
return 0;
}
-static int usbg_queue_tm_rsp(struct se_cmd *se_cmd)
+static void usbg_queue_tm_rsp(struct se_cmd *se_cmd)
{
- return 0;
}
static const char *usbg_check_wwn(const char *name)
diff --git a/drivers/vhost/tcm_vhost.c b/drivers/vhost/tcm_vhost.c
index 957a0b9..60f386d 100644
--- a/drivers/vhost/tcm_vhost.c
+++ b/drivers/vhost/tcm_vhost.c
@@ -344,9 +344,8 @@ static int tcm_vhost_queue_status(struct se_cmd *se_cmd)
return 0;
}
-static int tcm_vhost_queue_tm_rsp(struct se_cmd *se_cmd)
+static void tcm_vhost_queue_tm_rsp(struct se_cmd *se_cmd)
{
- return 0;
}
static void vhost_scsi_free_cmd(struct tcm_vhost_cmd *tv_cmd)
diff --git a/include/target/target_core_fabric.h b/include/target/target_core_fabric.h
index aaa1ee6..9708a04 100644
--- a/include/target/target_core_fabric.h
+++ b/include/target/target_core_fabric.h
@@ -61,7 +61,7 @@ struct target_core_fabric_ops {
int (*get_cmd_state)(struct se_cmd *);
int (*queue_data_in)(struct se_cmd *);
int (*queue_status)(struct se_cmd *);
- int (*queue_tm_rsp)(struct se_cmd *);
+ void (*queue_tm_rsp)(struct se_cmd *);
/*
* fabric module calls for target_core_fabric_configfs.c
*/
--
1.7.10.4
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH 3/3] target: make queue_tm_rsp() return void
2013-07-03 15:22 ` [PATCH 3/3] target: make queue_tm_rsp() return void Joern Engel
@ 2013-07-07 0:31 ` Nicholas A. Bellinger
0 siblings, 0 replies; 7+ messages in thread
From: Nicholas A. Bellinger @ 2013-07-07 0:31 UTC (permalink / raw)
To: Joern Engel; +Cc: linux-kernel, target-devel
On Wed, 2013-07-03 at 11:22 -0400, Joern Engel wrote:
> The return value wasn't checked by any of the callers. Assuming this is
> correct behaviour, we can simplify some code by not bothering to
> generate it.
>
> Signed-off-by: Joern Engel <joern@logfs.org>
> ---
> drivers/infiniband/ulp/srpt/ib_srpt.c | 14 +++++---------
> drivers/scsi/qla2xxx/tcm_qla2xxx.c | 4 +---
> drivers/target/iscsi/iscsi_target_configfs.c | 3 +--
> drivers/target/loopback/tcm_loop.c | 3 +--
> drivers/target/sbp/sbp_target.c | 3 +--
> drivers/target/tcm_fc/tcm_fc.h | 2 +-
> drivers/target/tcm_fc/tfc_cmd.c | 5 ++---
> drivers/usb/gadget/tcm_usb_gadget.c | 3 +--
> drivers/vhost/tcm_vhost.c | 3 +--
> include/target/target_core_fabric.h | 2 +-
> 10 files changed, 15 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.c b/drivers/infiniband/ulp/srpt/ib_srpt.c
> index c09d41b..2c61a28 100644
> --- a/drivers/infiniband/ulp/srpt/ib_srpt.c
> +++ b/drivers/infiniband/ulp/srpt/ib_srpt.c
> @@ -2987,7 +2987,7 @@ static u8 tcm_to_srp_tsk_mgmt_status(const int tcm_mgmt_status)
> * Callback function called by the TCM core. Must not block since it can be
> * invoked on the context of the IB completion handler.
> */
> -static int srpt_queue_response(struct se_cmd *cmd)
> +static void srpt_queue_response(struct se_cmd *cmd)
> {
> struct srpt_rdma_ch *ch;
> struct srpt_send_ioctx *ioctx;
> @@ -2998,8 +2998,6 @@ static int srpt_queue_response(struct se_cmd *cmd)
> int resp_len;
> u8 srp_tm_status;
>
> - ret = 0;
> -
> ioctx = container_of(cmd, struct srpt_send_ioctx, cmd);
> ch = ioctx->ch;
> BUG_ON(!ch);
> @@ -3025,7 +3023,7 @@ static int srpt_queue_response(struct se_cmd *cmd)
> || WARN_ON_ONCE(state == SRPT_STATE_CMD_RSP_SENT))) {
> atomic_inc(&ch->req_lim_delta);
> srpt_abort_cmd(ioctx);
> - goto out;
> + return;
> }
>
> dir = ioctx->cmd.data_direction;
> @@ -3037,7 +3035,7 @@ static int srpt_queue_response(struct se_cmd *cmd)
> if (ret) {
> printk(KERN_ERR "xfer_data failed for tag %llu\n",
> ioctx->tag);
> - goto out;
> + return;
> }
> }
>
> @@ -3058,9 +3056,6 @@ static int srpt_queue_response(struct se_cmd *cmd)
> srpt_set_cmd_state(ioctx, SRPT_STATE_DONE);
> target_put_sess_cmd(ioctx->ch->sess, &ioctx->cmd);
> }
> -
> -out:
> - return ret;
> }
>
> static int srpt_queue_status(struct se_cmd *cmd)
> @@ -3073,7 +3068,8 @@ static int srpt_queue_status(struct se_cmd *cmd)
> (SCF_TRANSPORT_TASK_SENSE | SCF_EMULATED_TASK_SENSE))
> WARN_ON(cmd->scsi_status != SAM_STAT_CHECK_CONDITION);
> ioctx->queue_status_only = true;
> - return srpt_queue_response(cmd);
> + srpt_queue_response(cmd);
> + return 0;
> }
>
> static void srpt_refresh_port_work(struct work_struct *work)
Hi Joern,
This one doesn't quite work for ib_srpt code, as srpt_queue_response()
is used by TFO->queue_data_in() and expects a return value here..
Squashing the following fix on top of your original patch into for-next
now.
Thanks!
diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.c b/drivers/infiniband/ulp/srpt/ib_srpt.c
index 59319c6..653ac6b 100644
--- a/drivers/infiniband/ulp/srpt/ib_srpt.c
+++ b/drivers/infiniband/ulp/srpt/ib_srpt.c
@@ -3082,6 +3082,17 @@ static void srpt_queue_response(struct se_cmd *cmd)
}
}
+static int srpt_queue_data_in(struct se_cmd *cmd)
+{
+ srpt_queue_response(cmd);
+ return 0;
+}
+
+static void srpt_queue_tm_rsp(struct se_cmd *cmd)
+{
+ srpt_queue_response(cmd);
+}
+
static int srpt_queue_status(struct se_cmd *cmd)
{
struct srpt_send_ioctx *ioctx;
@@ -3926,9 +3937,9 @@ static struct target_core_fabric_ops srpt_template = {
.set_default_node_attributes = srpt_set_default_node_attrs,
.get_task_tag = srpt_get_task_tag,
.get_cmd_state = srpt_get_tcm_cmd_state,
- .queue_data_in = srpt_queue_response,
+ .queue_data_in = srpt_queue_data_in,
.queue_status = srpt_queue_status,
- .queue_tm_rsp = srpt_queue_response,
+ .queue_tm_rsp = srpt_queue_tm_rsp,
/*
* Setup function pointers for generic logic in
* target_core_fabric_configfs.c
^ permalink raw reply related [flat|nested] 7+ messages in thread