Re: [PATCH 10/12] IB/srpt: convert to the generic RDMA READ/WRITE API

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 04/11/2016 02:32 PM, Christoph Hellwig wrote:
  static int srpt_get_desc_tbl(struct srpt_send_ioctx *ioctx,
-			     struct srp_cmd *srp_cmd,
-			     enum dma_data_direction *dir, u64 *data_len)
+		struct srp_cmd *srp_cmd, enum dma_data_direction *dir,
+		struct scatterlist **sg, unsigned *sg_cnt, u64 *data_len)
  {
[ ... ]

-		db = idb->desc_list;
-		memcpy(ioctx->rbufs, db, ioctx->n_rbuf * sizeof(*db));
  		*data_len = be32_to_cpu(idb->len);
+		return srpt_alloc_rw_ctxs(ioctx, idb->desc_list, nbufs,
+				sg, sg_cnt);
+	} else {
+		*data_len = 0;
+		return 0;
  	}
-out:
-	return ret;
  }

srpt_get_desc_tbl() only has one caller. Have you considered to move srpt_alloc_rw_ctxs() from this function to the caller of srpt_get_desc_tbl()?

-	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);
+	rc = srpt_get_desc_tbl(send_ioctx, srp_cmd, &dir, &sg, &sg_cnt,
+			&data_len);
+	if (rc) {
+		if (rc != -EAGAIN) {
+			pr_err("0x%llx: parsing SRP descriptor table failed.\n",
+			       srp_cmd->tag);
+		} else {
+			printk_ratelimited("out of MRs for 0x%llx\n", srp_cmd->tag);
+		}
  		goto release_ioctx;
  	}

Sorry but releasing an I/O context if srpt_alloc_rw_ctxs() returns -EAGAIN looks wrong to me. If this happens the I/O context should be added to the wait list without releasing it. Additionally, srpt_recv_done() will have to be modified such that newly received commands are added to the wait list if this list is not empty to prevent that starvation of a postponed request occurs due to new incoming requests.

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



[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Photo]     [Yosemite News]     [Yosemite Photos]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux