Re: [PATCH 2/4] IB/isert: convert to new CQ API

Max Gurtovoy
2016-02-16T18:04:40Z

Hi,

> @@ -977,7 +959,10 @@ isert_post_recvm(struct isert_conn *isert_conn, u32 count)
>
> for (rx_wr = isert_conn->rx_wr, i = 0; i < count; i++, rx_wr++) {
> rx_desc = &isert_conn->rx_descs[i];
> - rx_wr->wr_id = (uintptr_t)rx_desc;
> +
> + rx_desc->rx_cqe.done = isert_recv_done;

perhaps we can do it once in isert_alloc_rx_descriptors ?

> +
> + rx_wr->wr_cqe = &rx_desc->rx_cqe;
> rx_wr->sg_list = &rx_desc->rx_sg;
> rx_wr->num_sge = 1;
> rx_wr->next = rx_wr + 1;
> @@ -1002,7 +987,9 @@ isert_post_recv(struct isert_conn *isert_conn, struct iser_rx_desc *rx_desc)
> struct ib_recv_wr *rx_wr_failed, rx_wr;
> int ret;
>
> - rx_wr.wr_id = (uintptr_t)rx_desc;
> + rx_desc->rx_cqe.done = isert_recv_done;

same here.

> +
> + rx_wr.wr_cqe = &rx_desc->rx_cqe;
> rx_wr.sg_list = &rx_desc->rx_sg;
> rx_wr.num_sge = 1;
> rx_wr.next = NULL;
> @@ -1018,7 +1005,7 @@ isert_post_recv(struct isert_conn *isert_conn, struct iser_rx_desc *rx_desc)

> +static void
> +isert_recv_done(struct ib_cq *cq, struct ib_wc *wc)
> {
> + struct isert_conn *isert_conn = wc->qp->qp_context;
> + struct ib_device *ib_dev = isert_conn->cm_id->device;
> + struct iser_rx_desc *rx_desc =
> + container_of(wc->wr_cqe, struct iser_rx_desc, rx_cqe);

maybe we can define it as:
static inline struct iser_rx_desc *
isert_rx(struct ib_cqe *rx_cqe)
{
return container_of(rx_cqe, struct iser_rx_desc, rx_cqe);
}
in the .h file ?

> + struct iscsi_hdr *hdr = &rx_desc->iscsi_header;
> struct iser_ctrl *iser_ctrl = &rx_desc->iser_header;
> uint64_t read_va = 0, write_va = 0;

> + if (unlikely(wc->status != IB_WC_SUCCESS)) {
> + isert_print_wc(wc);
> + if (!--isert_conn->post_recv_buf_count)

maybe we can do isert_conn->post_recv_buf_count-- once at the beginning
of the function?

> + iscsit_cause_connection_reinstatement(isert_conn->conn, 0);
> + return;
> + }
> +
> + ib_dma_sync_single_for_cpu(ib_dev, rx_desc->dma_addr,
> + ISER_RX_PAYLOAD_SIZE, DMA_FROM_DEVICE);
> +
> + isert_dbg("DMA: 0x%llx, iSCSI opcode: 0x%02x, ITT: 0x%08x, flags: 0x%02x dlen: %d\n",
> + rx_desc->dma_addr, hdr->opcode, hdr->itt, hdr->flags,
> + (int)(wc->byte_len - ISER_HEADERS_LEN));
> +
> switch (iser_ctrl->flags & 0xF0) {
> case ISCSI_CTRL:
> if (iser_ctrl->flags & ISER_RSV) {
> @@ -1584,55 +1609,44 @@ isert_rx_do_work(struct iser_rx_desc *rx_desc, struct isert_conn *isert_conn)
>
> isert_rx_opcode(isert_conn, rx_desc,
> read_stag, read_va, write_stag, write_va);
> +
> + ib_dma_sync_single_for_device(ib_dev, rx_desc->dma_addr,
> + ISER_RX_PAYLOAD_SIZE, DMA_FROM_DEVICE);
> +
> + isert_conn->post_recv_buf_count--;
> }
>
> static void
> -isert_rcv_completion(struct iser_rx_desc *desc,
> - struct isert_conn *isert_conn,
> - u32 xfer_len)
> +isert_login_recv_done(struct ib_cq *cq, struct ib_wc *wc)
> {
> + struct isert_conn *isert_conn = wc->qp->qp_context;
> struct ib_device *ib_dev = isert_conn->cm_id->device;
> - struct iscsi_hdr *hdr;
> - u64 rx_dma;
> - int rx_buflen;
> -
> - if (desc == isert_conn->login_req_buf) {
> - rx_dma = isert_conn->login_req_dma;
> - rx_buflen = ISER_RX_LOGIN_SIZE;
> - isert_dbg("login_buf: Using rx_dma: 0x%llx, rx_buflen: %d\n",
> - rx_dma, rx_buflen);
> - } else {
> - rx_dma = desc->dma_addr;
> - rx_buflen = ISER_RX_PAYLOAD_SIZE;
> - isert_dbg("req_buf: Using rx_dma: 0x%llx, rx_buflen: %d\n",
> - rx_dma, rx_buflen);
> +
> + if (unlikely(wc->status != IB_WC_SUCCESS)) {
> + isert_print_wc(wc);
> + if (!--isert_conn->post_recv_buf_count)

maybe we can do isert_conn->post_recv_buf_count-- once at the beginning
of the function?

> + iscsit_cause_connection_reinstatement(isert_conn->conn, 0);
> + return;
> }
>
> - ib_dma_sync_single_for_cpu(ib_dev, rx_dma, rx_buflen, DMA_FROM_DEVICE);
> + ib_dma_sync_single_for_cpu(ib_dev, isert_conn->login_req_dma,
> + ISER_RX_LOGIN_SIZE, DMA_FROM_DEVICE);
>
> - hdr = &desc->iscsi_header;
> - isert_dbg("iSCSI opcode: 0x%02x, ITT: 0x%08x, flags: 0x%02x dlen: %d\n",
> - hdr->opcode, hdr->itt, hdr->flags,
> - (int)(xfer_len - ISER_HEADERS_LEN));
> + isert_conn->login_req_len = wc->byte_len - ISER_HEADERS_LEN;
>
> - if (desc == isert_conn->login_req_buf) {
> - isert_conn->login_req_len = xfer_len - ISER_HEADERS_LEN;
> - if (isert_conn->conn) {
> - struct iscsi_login *login = isert_conn->conn->conn_login;
> + if (isert_conn->conn) {
> + struct iscsi_login *login = isert_conn->conn->conn_login;
>
> - if (login && !login->first_request)
> - isert_rx_login_req(isert_conn);
> - }
> - mutex_lock(&isert_conn->mutex);
> - complete(&isert_conn->login_req_comp);
> - mutex_unlock(&isert_conn->mutex);
> - } else {
> - isert_rx_do_work(desc, isert_conn);
> + if (login && !login->first_request)
> + isert_rx_login_req(isert_conn);
> }
>
> - ib_dma_sync_single_for_device(ib_dev, rx_dma, rx_buflen,
> - DMA_FROM_DEVICE);
> + mutex_lock(&isert_conn->mutex);
> + complete(&isert_conn->login_req_comp);
> + mutex_unlock(&isert_conn->mutex);
>
> + ib_dma_sync_single_for_device(ib_dev, isert_conn->login_req_dma,
> + ISER_RX_LOGIN_SIZE, DMA_FROM_DEVICE);
> isert_conn->post_recv_buf_count--;
> }

> +isert_rdma_write_done(struct ib_cq *cq, struct ib_wc *wc)
> {
> - struct isert_rdma_wr *wr = &isert_cmd->rdma_wr;
> - struct iscsi_cmd *cmd = isert_cmd->iscsi_cmd;
> - struct se_cmd *se_cmd = &cmd->se_cmd;
> - struct isert_conn *isert_conn = isert_cmd->conn;
> + struct isert_conn *isert_conn = wc->qp->qp_context;
> struct isert_device *device = isert_conn->device;
> + struct iser_tx_desc *desc =
> + container_of(wc->wr_cqe, struct iser_tx_desc, tx_cqe);

maybe we can define it as:
static inline struct iser_tx_desc *
isert_tx(struct ib_cqe *tx_cqe)
{
return container_of(tx_cqe, struct iser_tx_desc, tx_cqe);
}

> + struct isert_cmd *isert_cmd = desc->isert_cmd;
> + struct isert_rdma_wr *wr = &isert_cmd->rdma_wr;
> + struct se_cmd *cmd = &isert_cmd->iscsi_cmd->se_cmd;
> int ret = 0;

> +isert_rdma_read_done(struct ib_cq *cq, struct ib_wc *wc)
> {
> + struct isert_conn *isert_conn = wc->qp->qp_context;
> + struct isert_device *device = isert_conn->device;
> + struct iser_tx_desc *desc =
> + container_of(wc->wr_cqe, struct iser_tx_desc, tx_cqe);

and use it here.

> + struct isert_cmd *isert_cmd = desc->isert_cmd;
> struct isert_rdma_wr *wr = &isert_cmd->rdma_wr;
> struct iscsi_cmd *cmd = isert_cmd->iscsi_cmd;
> struct se_cmd *se_cmd = &cmd->se_cmd;

> +isert_send_done(struct ib_cq *cq, struct ib_wc *wc)
> {
> + struct isert_conn *isert_conn = wc->qp->qp_context;
> struct ib_device *ib_dev = isert_conn->cm_id->device;
> + struct iser_tx_desc *tx_desc =
> + container_of(wc->wr_cqe, struct iser_tx_desc, tx_cqe);

and here.

> struct isert_cmd *isert_cmd = tx_desc->isert_cmd;
> - struct isert_rdma_wr *wr;
>

thanks,
Max.