On Sun, 2012-07-08 at 15:58 -0400, Christoph Hellwig wrote:
> plain text document attachment (srpt-use-target_execute_cmd)
> srpt_handle_rdma_comp is called from kthread context and thus can execute
> target_execute_cmd directly. srpt_abort_cmd sets the CMD_T_LUN_STOP
> flag directly, and thus the abuse of transport_generic_handle_data can be
> replaced with an opencoded variant of that code path. I'm still not happy
> about a fabric driver poking into target core internals like this, but
> let's defer the bigger architecture changes for now.
>
Mmmmm..
This patch is OK for the moment, but there is actually a patch in
lio-core (that's not been forward ported to target-pending just yet)
that converts ib_srpt to use target_submit_cmd(), and gets rid of the
legacy srpt_send_ioctx->kref usage cruft. (Bart CC'ed)
I'll look at digging this patch out and getting it rebased against
for-next for some review, most likely as as v3.7 item unless someone
really want's to own it for-3.6.
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
>
> ---
> drivers/infiniband/ulp/srpt/ib_srpt.c | 15 ++++++++++-----
> 1 file changed, 10 insertions(+), 5 deletions(-)
>
> Index: lio-core/drivers/infiniband/ulp/srpt/ib_srpt.c
> ===================================================================
> --- lio-core.orig/drivers/infiniband/ulp/srpt/ib_srpt.c 2012-07-01 14:29:18.684261928 +0200
> +++ lio-core/drivers/infiniband/ulp/srpt/ib_srpt.c 2012-07-01 14:29:32.087595181 +0200
> @@ -1377,10 +1377,14 @@ static int srpt_abort_cmd(struct srpt_se
> break;
> case SRPT_STATE_NEED_DATA:
> /* DMA_TO_DEVICE (write) - RDMA read error. */
> +
> + /* XXX(hch): this is a horrible layering violation.. */
> spin_lock_irqsave(&ioctx->cmd.t_state_lock, flags);
> ioctx->cmd.transport_state |= CMD_T_LUN_STOP;
> + ioctx->cmd.transport_state &= ~CMD_T_ACTIVE;
> spin_unlock_irqrestore(&ioctx->cmd.t_state_lock, flags);
> - transport_generic_handle_data(&ioctx->cmd);
> +
> + complete(&ioctx->cmd.transport_lun_stop_comp);
> break;
> case SRPT_STATE_CMD_RSP_SENT:
> /*
> @@ -1463,9 +1467,10 @@ static void srpt_handle_send_comp(struct
> /**
> * srpt_handle_rdma_comp() - Process an IB RDMA completion notification.
> *
> - * Note: transport_generic_handle_data() is asynchronous so unmapping the
> - * data that has been transferred via IB RDMA must be postponed until the
> - * check_stop_free() callback.
> + * XXX: what is now target_execute_cmd used to be asynchronous, and unmapping
> + * the data that has been transferred via IB RDMA had to be postponed until the
> + * check_stop_free() callback. None of this is nessecary anymore and needs to
> + * be cleaned up.
> */
> static void srpt_handle_rdma_comp(struct srpt_rdma_ch *ch,
> struct srpt_send_ioctx *ioctx,
> @@ -1477,7 +1482,7 @@ static void srpt_handle_rdma_comp(struct
> if (opcode == SRPT_RDMA_READ_LAST) {
> if (srpt_test_and_set_cmd_state(ioctx, SRPT_STATE_NEED_DATA,
> SRPT_STATE_DATA_IN))
> - transport_generic_handle_data(&ioctx->cmd);
> + target_execute_cmd(&ioctx->cmd);
> else
> printk(KERN_ERR "%s[%d]: wrong state = %d\n", __func__,
> __LINE__, srpt_get_cmd_state(ioctx));
>
> --
> To unsubscribe from this list: send the line "unsubscribe target-devel" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe target-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
