| Message ID | 568BD249.1000100@sandisk.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
On Tue, Jan 05, 2016 at 03:25:13PM +0100, Bart Van Assche wrote: > The target core function that should be called if target_submit_cmd() > fails is target_put_sess_cmd(). Additionally, change the return type > of srpt_handle_cmd() from int into void. I actually ran into this bug a long time ago with a modified srpt driver and forgot to send a similar fix.. Looks good: Reviewed-by: Christoph Hellwig <hch@lst.de> Minor nitpick below: > + send_ioctx->state = SRPT_STATE_DONE; > + target_put_sess_cmd(cmd); > + return; > } no need for that return statement. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 05/01/2016 16:25, Bart Van Assche wrote: > The target core function that should be called if target_submit_cmd() > fails is target_put_sess_cmd(). Additionally, change the return type > of srpt_handle_cmd() from int into void. > > Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com> > Cc: Christoph Hellwig <hch@lst.de> > --- > drivers/infiniband/ulp/srpt/ib_srpt.c | 24 ++++++++++++------------ > 1 file changed, 12 insertions(+), 12 deletions(-) > > diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.c b/drivers/infiniband/ulp/srpt/ib_srpt.c > index 1857d17..395bc1b 100644 > --- a/drivers/infiniband/ulp/srpt/ib_srpt.c > +++ b/drivers/infiniband/ulp/srpt/ib_srpt.c > @@ -1482,15 +1482,14 @@ static int srpt_check_stop_free(struct se_cmd *cmd) > /** > * srpt_handle_cmd() - Process SRP_CMD. > */ > -static int srpt_handle_cmd(struct srpt_rdma_ch *ch, > - struct srpt_recv_ioctx *recv_ioctx, > - struct srpt_send_ioctx *send_ioctx) > +static void srpt_handle_cmd(struct srpt_rdma_ch *ch, > + struct srpt_recv_ioctx *recv_ioctx, > + struct srpt_send_ioctx *send_ioctx) > { > struct se_cmd *cmd; > struct srp_cmd *srp_cmd; > u64 data_len; > enum dma_data_direction dir; > - sense_reason_t ret; > int rc; > > BUG_ON(!send_ioctx); > @@ -1518,8 +1517,7 @@ static int srpt_handle_cmd(struct srpt_rdma_ch *ch, > if (srpt_get_desc_tbl(send_ioctx, srp_cmd, &dir, &data_len)) { > pr_err("0x%llx: parsing SRP descriptor table failed.\n", > srp_cmd->tag); > - ret = TCM_INVALID_CDB_FIELD; > - goto send_sense; > + goto put; Do you still need to call target_put_sess_cmd() in this failure? -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 01/06/2016 03:31 PM, Sagi Grimberg wrote: > On 05/01/2016 16:25, Bart Van Assche wrote: >> @@ -1518,8 +1517,7 @@ static int srpt_handle_cmd(struct srpt_rdma_ch *ch, >> if (srpt_get_desc_tbl(send_ioctx, srp_cmd, &dir, &data_len)) { >> pr_err("0x%llx: parsing SRP descriptor table failed.\n", >> srp_cmd->tag); >> - ret = TCM_INVALID_CDB_FIELD; >> - goto send_sense; >> + goto put; > > Do you still need to call target_put_sess_cmd() in this failure? Good catch. I will update this patch before I repost this patch series. Bart. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.c b/drivers/infiniband/ulp/srpt/ib_srpt.c index 1857d17..395bc1b 100644 --- a/drivers/infiniband/ulp/srpt/ib_srpt.c +++ b/drivers/infiniband/ulp/srpt/ib_srpt.c @@ -1482,15 +1482,14 @@ static int srpt_check_stop_free(struct se_cmd *cmd) /** * srpt_handle_cmd() - Process SRP_CMD. */ -static int srpt_handle_cmd(struct srpt_rdma_ch *ch, - struct srpt_recv_ioctx *recv_ioctx, - struct srpt_send_ioctx *send_ioctx) +static void srpt_handle_cmd(struct srpt_rdma_ch *ch, + struct srpt_recv_ioctx *recv_ioctx, + struct srpt_send_ioctx *send_ioctx) { struct se_cmd *cmd; struct srp_cmd *srp_cmd; u64 data_len; enum dma_data_direction dir; - sense_reason_t ret; int rc; BUG_ON(!send_ioctx); @@ -1518,8 +1517,7 @@ static int srpt_handle_cmd(struct srpt_rdma_ch *ch, if (srpt_get_desc_tbl(send_ioctx, srp_cmd, &dir, &data_len)) { pr_err("0x%llx: parsing SRP descriptor table failed.\n", srp_cmd->tag); - ret = TCM_INVALID_CDB_FIELD; - goto send_sense; + goto put; } rc = target_submit_cmd(cmd, ch->sess, srp_cmd->cdb, @@ -1527,14 +1525,16 @@ static int srpt_handle_cmd(struct srpt_rdma_ch *ch, scsilun_to_int(&srp_cmd->lun), data_len, TCM_SIMPLE_TAG, dir, TARGET_SCF_ACK_KREF); if (rc != 0) { - ret = TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE; - goto send_sense; + pr_debug("target_submit_cmd() returned %d for tag %#llx\n", rc, + srp_cmd->tag); + goto put; } - return 0; + return; -send_sense: - transport_send_check_condition_and_sense(cmd, ret, 0); - return -1; +put: + send_ioctx->state = SRPT_STATE_DONE; + target_put_sess_cmd(cmd); + return; } static int srp_tmr_to_tcm(int fn)
The target core function that should be called if target_submit_cmd() fails is target_put_sess_cmd(). Additionally, change the return type of srpt_handle_cmd() from int into void. Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com> Cc: Christoph Hellwig <hch@lst.de> --- drivers/infiniband/ulp/srpt/ib_srpt.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-)