Re: [PATCH 12/15] IB/srpt: Eliminate srpt_find_channel()

[Sagi Grimberg]

Looks good,

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

[PATCH 12/15] IB/srpt: Eliminate srpt_find_channel()

[Bart Van Assche]

In the CM REQ message handler, store the channel pointer in
cm_id->context such that the function srpt_find_channel() is no
longer needed. Additionally, make the CM event messages more
informative.

Signed-off-by: Bart Van Assche 
Cc: Christoph Hellwig 
---
 drivers/infiniband/ulp/srpt/ib_srpt.c | 113 +-
 1 file changed, 43 insertions(+), 70 deletions(-)

diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.c 
b/drivers/infiniband/ulp/srpt/ib_srpt.c
index 2ae3c1b..6ec130d 100644
--- a/drivers/infiniband/ulp/srpt/ib_srpt.c
+++ b/drivers/infiniband/ulp/srpt/ib_srpt.c
@@ -1893,25 +1893,14 @@ static int srpt_shutdown_session(struct se_session 
*se_sess)
  * ib_destroy_cm_id(), which locks the cm_id spinlock and hence waits until
  * this function has finished).
  */
-static void srpt_drain_channel(struct ib_cm_id *cm_id)
+static void srpt_drain_channel(struct srpt_rdma_ch *ch)
 {
-   struct srpt_device *sdev;
-   struct srpt_rdma_ch *ch;
int ret;
bool do_reset = false;
 
WARN_ON_ONCE(irqs_disabled());
 
-   sdev = cm_id->context;
-   BUG_ON(!sdev);
-   spin_lock_irq(>spinlock);
-   list_for_each_entry(ch, >rch_list, list) {
-   if (ch->cm_id == cm_id) {
-   do_reset = srpt_set_ch_state(ch, CH_DRAINING);
-   break;
-   }
-   }
-   spin_unlock_irq(>spinlock);
+   do_reset = srpt_set_ch_state(ch, CH_DRAINING);
 
if (do_reset) {
if (ch->sess)
@@ -1925,34 +1914,6 @@ static void srpt_drain_channel(struct ib_cm_id *cm_id)
 }
 
 /**
- * srpt_find_channel() - Look up an RDMA channel.
- * @cm_id: Pointer to the CM ID of the channel to be looked up.
- *
- * Return NULL if no matching RDMA channel has been found.
- */
-static struct srpt_rdma_ch *srpt_find_channel(struct srpt_device *sdev,
- struct ib_cm_id *cm_id)
-{
-   struct srpt_rdma_ch *ch;
-   bool found;
-
-   WARN_ON_ONCE(irqs_disabled());
-   BUG_ON(!sdev);
-
-   found = false;
-   spin_lock_irq(>spinlock);
-   list_for_each_entry(ch, >rch_list, list) {
-   if (ch->cm_id == cm_id) {
-   found = true;
-   break;
-   }
-   }
-   spin_unlock_irq(>spinlock);
-
-   return found ? ch : NULL;
-}
-
-/**
  * srpt_release_channel() - Release channel resources.
  *
  * Schedules the actual release because:
@@ -2160,6 +2121,7 @@ static int srpt_cm_req_recv(struct ib_cm_id *cm_id,
memcpy(ch->t_port_id, req->target_port_id, 16);
ch->sport = >port[param->port - 1];
ch->cm_id = cm_id;
+   cm_id->context = ch;
/*
 * Avoid QUEUE_FULL conditions by limiting the number of buffers used
 * for the SRP protocol to the command queue size.
@@ -2304,10 +2266,23 @@ out:
return ret;
 }
 
-static void srpt_cm_rej_recv(struct ib_cm_id *cm_id)
+static void srpt_cm_rej_recv(struct srpt_rdma_ch *ch,
+enum ib_cm_rej_reason reason,
+const u8 *private_data,
+u8 private_data_len)
 {
-   pr_info("Received IB REJ for cm_id %p.\n", cm_id);
-   srpt_drain_channel(cm_id);
+   char *priv = kmalloc(private_data_len * 3 + 1, GFP_KERNEL);
+   int i;
+
+   if (priv) {
+   priv[0] = '\0';
+   for (i = 0; i < private_data_len; i++)
+   sprintf(priv + 3 * i, "%02x ", private_data[i]);
+   }
+   pr_info("Received CM REJ for ch %s-%d; reason %d; private data %s.\n",
+   ch->sess_name, ch->qp->qp_num, reason, priv ? : "(?)");
+   kfree(priv);
+   srpt_drain_channel(ch);
 }
 
 /**
@@ -2316,14 +2291,10 @@ static void srpt_cm_rej_recv(struct ib_cm_id *cm_id)
  * An IB_CM_RTU_RECEIVED message indicates that the connection is established
  * and that the recipient may begin transmitting (RTU = ready to use).
  */
-static void srpt_cm_rtu_recv(struct ib_cm_id *cm_id)
+static void srpt_cm_rtu_recv(struct srpt_rdma_ch *ch)
 {
-   struct srpt_rdma_ch *ch;
int ret;
 
-   ch = srpt_find_channel(cm_id->context, cm_id);
-   BUG_ON(!ch);
-
if (srpt_set_ch_state(ch, CH_LIVE)) {
struct srpt_recv_ioctx *ioctx, *ioctx_tmp;
 
@@ -2339,31 +2310,30 @@ static void srpt_cm_rtu_recv(struct ib_cm_id *cm_id)
}
 }
 
-static void srpt_cm_timewait_exit(struct ib_cm_id *cm_id)
+static void srpt_cm_timewait_exit(struct srpt_rdma_ch *ch)
 {
-   pr_info("Received IB TimeWait exit for cm_id %p.\n", cm_id);
-   srpt_drain_channel(cm_id);
+   pr_info("Received CM TimeWait exit for ch %s-%d.\n", ch->sess_name,
+   ch->qp->qp_num);
+   srpt_drain_channel(ch);
 }
 
-static void srpt_cm_rep_error(struct ib_cm_id *cm_id)
+static void srpt_cm_rep_error(struct srpt_rdma_ch *ch)
 {
-  

Re: [PATCH 12/15] IB/srpt: Eliminate srpt_find_channel()

[Christoph Hellwig]

On Tue, Jan 05, 2016 at 03:26:22PM +0100, Bart Van Assche wrote:
> In the CM REQ message handler, store the channel pointer in
> cm_id->context such that the function srpt_find_channel() is no
> longer needed. Additionally, make the CM event messages more
> informative.

Looks fine,

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