Commit 6f0fae3d authored by Sagi Grimberg's avatar Sagi Grimberg Committed by Nicholas Bellinger

iser-target: Use single CQ for TX and RX

Using TX and RX CQs attached to the same vector might
create a throttling effect coming from the serial processing
of a work-queue. Use one CQ instead, it will do better in interrupt
processing and it provides a simpler code. Also, We get rid of
redundant isert_rx_wq.

Next we can remove the atomic post_send_buf_count from the IO path.
Signed-off-by: default avatarSagi Grimberg <sagig@mellanox.com>
Signed-off-by: default avatarNicholas Bellinger <nab@linux-iscsi.org>
parent 4a295bae
...@@ -35,10 +35,10 @@ ...@@ -35,10 +35,10 @@
#define ISERT_MAX_CONN 8 #define ISERT_MAX_CONN 8
#define ISER_MAX_RX_CQ_LEN (ISERT_QP_MAX_RECV_DTOS * ISERT_MAX_CONN) #define ISER_MAX_RX_CQ_LEN (ISERT_QP_MAX_RECV_DTOS * ISERT_MAX_CONN)
#define ISER_MAX_TX_CQ_LEN (ISERT_QP_MAX_REQ_DTOS * ISERT_MAX_CONN) #define ISER_MAX_TX_CQ_LEN (ISERT_QP_MAX_REQ_DTOS * ISERT_MAX_CONN)
#define ISER_MAX_CQ_LEN (ISER_MAX_RX_CQ_LEN + ISER_MAX_TX_CQ_LEN)
static DEFINE_MUTEX(device_list_mutex); static DEFINE_MUTEX(device_list_mutex);
static LIST_HEAD(device_list); static LIST_HEAD(device_list);
static struct workqueue_struct *isert_rx_wq;
static struct workqueue_struct *isert_comp_wq; static struct workqueue_struct *isert_comp_wq;
static struct workqueue_struct *isert_release_wq; static struct workqueue_struct *isert_release_wq;
...@@ -124,8 +124,8 @@ isert_conn_setup_qp(struct isert_conn *isert_conn, struct rdma_cm_id *cma_id) ...@@ -124,8 +124,8 @@ isert_conn_setup_qp(struct isert_conn *isert_conn, struct rdma_cm_id *cma_id)
memset(&attr, 0, sizeof(struct ib_qp_init_attr)); memset(&attr, 0, sizeof(struct ib_qp_init_attr));
attr.event_handler = isert_qp_event_callback; attr.event_handler = isert_qp_event_callback;
attr.qp_context = isert_conn; attr.qp_context = isert_conn;
attr.send_cq = comp->tx_cq; attr.send_cq = comp->cq;
attr.recv_cq = comp->rx_cq; attr.recv_cq = comp->cq;
attr.cap.max_send_wr = ISERT_QP_MAX_REQ_DTOS; attr.cap.max_send_wr = ISERT_QP_MAX_REQ_DTOS;
attr.cap.max_recv_wr = ISERT_QP_MAX_RECV_DTOS; attr.cap.max_recv_wr = ISERT_QP_MAX_RECV_DTOS;
/* /*
...@@ -237,10 +237,8 @@ isert_free_rx_descriptors(struct isert_conn *isert_conn) ...@@ -237,10 +237,8 @@ isert_free_rx_descriptors(struct isert_conn *isert_conn)
isert_conn->conn_rx_descs = NULL; isert_conn->conn_rx_descs = NULL;
} }
static void isert_cq_tx_work(struct work_struct *); static void isert_cq_work(struct work_struct *);
static void isert_cq_tx_callback(struct ib_cq *, void *); static void isert_cq_callback(struct ib_cq *, void *);
static void isert_cq_rx_work(struct work_struct *);
static void isert_cq_rx_callback(struct ib_cq *, void *);
static int static int
isert_create_device_ib_res(struct isert_device *device) isert_create_device_ib_res(struct isert_device *device)
...@@ -248,15 +246,14 @@ isert_create_device_ib_res(struct isert_device *device) ...@@ -248,15 +246,14 @@ isert_create_device_ib_res(struct isert_device *device)
struct ib_device *ib_dev = device->ib_device; struct ib_device *ib_dev = device->ib_device;
struct ib_device_attr *dev_attr; struct ib_device_attr *dev_attr;
int ret = 0, i; int ret = 0, i;
int max_rx_cqe, max_tx_cqe; int max_cqe;
dev_attr = &device->dev_attr; dev_attr = &device->dev_attr;
ret = isert_query_device(ib_dev, dev_attr); ret = isert_query_device(ib_dev, dev_attr);
if (ret) if (ret)
return ret; return ret;
max_rx_cqe = min(ISER_MAX_RX_CQ_LEN, dev_attr->max_cqe); max_cqe = min(ISER_MAX_CQ_LEN, dev_attr->max_cqe);
max_tx_cqe = min(ISER_MAX_TX_CQ_LEN, dev_attr->max_cqe);
/* asign function handlers */ /* asign function handlers */
if (dev_attr->device_cap_flags & IB_DEVICE_MEM_MGT_EXTENSIONS && if (dev_attr->device_cap_flags & IB_DEVICE_MEM_MGT_EXTENSIONS &&
...@@ -293,35 +290,19 @@ isert_create_device_ib_res(struct isert_device *device) ...@@ -293,35 +290,19 @@ isert_create_device_ib_res(struct isert_device *device)
struct isert_comp *comp = &device->comps[i]; struct isert_comp *comp = &device->comps[i];
comp->device = device; comp->device = device;
INIT_WORK(&comp->rx_work, isert_cq_rx_work); INIT_WORK(&comp->work, isert_cq_work);
comp->rx_cq = ib_create_cq(device->ib_device, comp->cq = ib_create_cq(device->ib_device,
isert_cq_rx_callback, isert_cq_callback,
isert_cq_event_callback, isert_cq_event_callback,
(void *)comp, (void *)comp,
max_rx_cqe, i); max_cqe, i);
if (IS_ERR(comp->rx_cq)) { if (IS_ERR(comp->cq)) {
ret = PTR_ERR(comp->rx_cq); ret = PTR_ERR(comp->cq);
comp->rx_cq = NULL; comp->cq = NULL;
goto out_cq; goto out_cq;
} }
INIT_WORK(&comp->tx_work, isert_cq_tx_work); ret = ib_req_notify_cq(comp->cq, IB_CQ_NEXT_COMP);
comp->tx_cq = ib_create_cq(device->ib_device,
isert_cq_tx_callback,
isert_cq_event_callback,
(void *)comp,
max_tx_cqe, i);
if (IS_ERR(comp->tx_cq)) {
ret = PTR_ERR(comp->tx_cq);
comp->tx_cq = NULL;
goto out_cq;
}
ret = ib_req_notify_cq(comp->rx_cq, IB_CQ_NEXT_COMP);
if (ret)
goto out_cq;
ret = ib_req_notify_cq(comp->tx_cq, IB_CQ_NEXT_COMP);
if (ret) if (ret)
goto out_cq; goto out_cq;
} }
...@@ -332,13 +313,9 @@ isert_create_device_ib_res(struct isert_device *device) ...@@ -332,13 +313,9 @@ isert_create_device_ib_res(struct isert_device *device)
for (i = 0; i < device->comps_used; i++) { for (i = 0; i < device->comps_used; i++) {
struct isert_comp *comp = &device->comps[i]; struct isert_comp *comp = &device->comps[i];
if (comp->rx_cq) { if (comp->cq) {
cancel_work_sync(&comp->rx_work); cancel_work_sync(&comp->work);
ib_destroy_cq(comp->rx_cq); ib_destroy_cq(comp->cq);
}
if (comp->tx_cq) {
cancel_work_sync(&comp->tx_work);
ib_destroy_cq(comp->tx_cq);
} }
} }
kfree(device->comps); kfree(device->comps);
...@@ -356,12 +333,9 @@ isert_free_device_ib_res(struct isert_device *device) ...@@ -356,12 +333,9 @@ isert_free_device_ib_res(struct isert_device *device)
for (i = 0; i < device->comps_used; i++) { for (i = 0; i < device->comps_used; i++) {
struct isert_comp *comp = &device->comps[i]; struct isert_comp *comp = &device->comps[i];
cancel_work_sync(&comp->rx_work); cancel_work_sync(&comp->work);
cancel_work_sync(&comp->tx_work); ib_destroy_cq(comp->cq);
ib_destroy_cq(comp->rx_cq); comp->cq = NULL;
ib_destroy_cq(comp->tx_cq);
comp->rx_cq = NULL;
comp->tx_cq = NULL;
} }
kfree(device->comps); kfree(device->comps);
} }
...@@ -2013,14 +1987,39 @@ isert_send_completion(struct iser_tx_desc *tx_desc, ...@@ -2013,14 +1987,39 @@ isert_send_completion(struct iser_tx_desc *tx_desc,
} }
} }
/**
* is_isert_tx_desc() - Indicate if the completion wr_id
* is a TX descriptor or not.
* @isert_conn: iser connection
* @wr_id: completion WR identifier
*
* Since we cannot rely on wc opcode in FLUSH errors
* we must work around it by checking if the wr_id address
* falls in the iser connection rx_descs buffer. If so
* it is an RX descriptor, otherwize it is a TX.
*/
static inline bool
is_isert_tx_desc(struct isert_conn *isert_conn, void *wr_id)
{
void *start = isert_conn->conn_rx_descs;
int len = ISERT_QP_MAX_RECV_DTOS * sizeof(*isert_conn->conn_rx_descs);
if (wr_id >= start && wr_id < start + len)
return false;
return true;
}
static void static void
isert_cq_comp_err(void *desc, struct isert_conn *isert_conn, bool tx) isert_cq_comp_err(struct isert_conn *isert_conn, struct ib_wc *wc)
{ {
if (tx) { if (is_isert_tx_desc(isert_conn, (void *)wc->wr_id)) {
struct ib_device *ib_dev = isert_conn->conn_cm_id->device; struct ib_device *ib_dev = isert_conn->conn_cm_id->device;
struct isert_cmd *isert_cmd; struct isert_cmd *isert_cmd;
struct iser_tx_desc *desc;
isert_cmd = ((struct iser_tx_desc *)desc)->isert_cmd; desc = (struct iser_tx_desc *)(uintptr_t)wc->wr_id;
isert_cmd = desc->isert_cmd;
if (!isert_cmd) if (!isert_cmd)
isert_unmap_tx_desc(desc, ib_dev); isert_unmap_tx_desc(desc, ib_dev);
else else
...@@ -2049,80 +2048,52 @@ isert_cq_comp_err(void *desc, struct isert_conn *isert_conn, bool tx) ...@@ -2049,80 +2048,52 @@ isert_cq_comp_err(void *desc, struct isert_conn *isert_conn, bool tx)
} }
static void static void
isert_cq_tx_work(struct work_struct *work) isert_handle_wc(struct ib_wc *wc)
{ {
struct isert_comp *comp = container_of(work, struct isert_comp,
tx_work);
struct ib_cq *cq = comp->tx_cq;
struct isert_conn *isert_conn; struct isert_conn *isert_conn;
struct iser_tx_desc *tx_desc; struct iser_tx_desc *tx_desc;
struct ib_wc wc; struct iser_rx_desc *rx_desc;
while (ib_poll_cq(cq, 1, &wc) == 1) {
isert_conn = wc.qp->qp_context;
tx_desc = (struct iser_tx_desc *)(uintptr_t)wc.wr_id;
if (wc.status == IB_WC_SUCCESS) { isert_conn = wc->qp->qp_context;
if (likely(wc->status == IB_WC_SUCCESS)) {
if (wc->opcode == IB_WC_RECV) {
rx_desc = (struct iser_rx_desc *)(uintptr_t)wc->wr_id;
isert_rx_completion(rx_desc, isert_conn, wc->byte_len);
} else {
tx_desc = (struct iser_tx_desc *)(uintptr_t)wc->wr_id;
isert_send_completion(tx_desc, isert_conn); isert_send_completion(tx_desc, isert_conn);
}
} else { } else {
pr_debug("TX wc.status != IB_WC_SUCCESS >>>>>>>>>>>>>>\n"); if (wc->status != IB_WC_WR_FLUSH_ERR)
pr_debug("TX wc.status: 0x%08x\n", wc.status); pr_err("wr id %llx status %d vend_err %x\n",
pr_debug("TX wc.vendor_err: 0x%08x\n", wc.vendor_err); wc->wr_id, wc->status, wc->vendor_err);
else
pr_debug("flush error: wr id %llx\n", wc->wr_id);
if (wc.wr_id != ISER_FASTREG_LI_WRID) if (wc->wr_id != ISER_FASTREG_LI_WRID)
isert_cq_comp_err(tx_desc, isert_conn, true); isert_cq_comp_err(isert_conn, wc);
}
} }
ib_req_notify_cq(cq, IB_CQ_NEXT_COMP);
} }
static void static void
isert_cq_tx_callback(struct ib_cq *cq, void *context) isert_cq_work(struct work_struct *work)
{
struct isert_comp *comp = context;
queue_work(isert_comp_wq, &comp->tx_work);
}
static void
isert_cq_rx_work(struct work_struct *work)
{ {
struct isert_comp *comp = container_of(work, struct isert_comp, struct isert_comp *comp = container_of(work, struct isert_comp,
rx_work); work);
struct ib_cq *cq = comp->rx_cq;
struct isert_conn *isert_conn;
struct iser_rx_desc *rx_desc;
struct ib_wc wc; struct ib_wc wc;
u32 xfer_len;
while (ib_poll_cq(cq, 1, &wc) == 1) {
isert_conn = wc.qp->qp_context;
rx_desc = (struct iser_rx_desc *)(uintptr_t)wc.wr_id;
if (wc.status == IB_WC_SUCCESS) { while (ib_poll_cq(comp->cq, 1, &wc) == 1)
xfer_len = wc.byte_len; isert_handle_wc(&wc);
isert_rx_completion(rx_desc, isert_conn, xfer_len);
} else {
pr_debug("RX wc.status != IB_WC_SUCCESS >>>>>>>>>>>>>>\n");
if (wc.status != IB_WC_WR_FLUSH_ERR) {
pr_debug("RX wc.status: 0x%08x\n", wc.status);
pr_debug("RX wc.vendor_err: 0x%08x\n",
wc.vendor_err);
}
isert_cq_comp_err(rx_desc, isert_conn, false);
}
}
ib_req_notify_cq(cq, IB_CQ_NEXT_COMP); ib_req_notify_cq(comp->cq, IB_CQ_NEXT_COMP);
} }
static void static void
isert_cq_rx_callback(struct ib_cq *cq, void *context) isert_cq_callback(struct ib_cq *cq, void *context)
{ {
struct isert_comp *comp = context; struct isert_comp *comp = context;
queue_work(isert_rx_wq, &comp->rx_work); queue_work(isert_comp_wq, &comp->work);
} }
static int static int
...@@ -3363,17 +3334,11 @@ static int __init isert_init(void) ...@@ -3363,17 +3334,11 @@ static int __init isert_init(void)
{ {
int ret; int ret;
isert_rx_wq = alloc_workqueue("isert_rx_wq", 0, 0);
if (!isert_rx_wq) {
pr_err("Unable to allocate isert_rx_wq\n");
return -ENOMEM;
}
isert_comp_wq = alloc_workqueue("isert_comp_wq", 0, 0); isert_comp_wq = alloc_workqueue("isert_comp_wq", 0, 0);
if (!isert_comp_wq) { if (!isert_comp_wq) {
pr_err("Unable to allocate isert_comp_wq\n"); pr_err("Unable to allocate isert_comp_wq\n");
ret = -ENOMEM; ret = -ENOMEM;
goto destroy_rx_wq; return -ENOMEM;
} }
isert_release_wq = alloc_workqueue("isert_release_wq", WQ_UNBOUND, isert_release_wq = alloc_workqueue("isert_release_wq", WQ_UNBOUND,
...@@ -3391,8 +3356,7 @@ static int __init isert_init(void) ...@@ -3391,8 +3356,7 @@ static int __init isert_init(void)
destroy_comp_wq: destroy_comp_wq:
destroy_workqueue(isert_comp_wq); destroy_workqueue(isert_comp_wq);
destroy_rx_wq:
destroy_workqueue(isert_rx_wq);
return ret; return ret;
} }
...@@ -3401,7 +3365,6 @@ static void __exit isert_exit(void) ...@@ -3401,7 +3365,6 @@ static void __exit isert_exit(void)
flush_scheduled_work(); flush_scheduled_work();
destroy_workqueue(isert_release_wq); destroy_workqueue(isert_release_wq);
destroy_workqueue(isert_comp_wq); destroy_workqueue(isert_comp_wq);
destroy_workqueue(isert_rx_wq);
iscsit_unregister_transport(&iser_target_transport); iscsit_unregister_transport(&iser_target_transport);
pr_debug("iSER_TARGET[0] - Released iser_target_transport\n"); pr_debug("iSER_TARGET[0] - Released iser_target_transport\n");
} }
......
...@@ -163,20 +163,16 @@ struct isert_conn { ...@@ -163,20 +163,16 @@ struct isert_conn {
* struct isert_comp - iSER completion context * struct isert_comp - iSER completion context
* *
* @device: pointer to device handle * @device: pointer to device handle
* @rx_cq: RX completion queue * @cq: completion queue
* @tx_cq: TX completion queue
* @active_qps: Number of active QPs attached * @active_qps: Number of active QPs attached
* to completion context * to completion context
* @rx_work: RX work handle * @work: completion work handle
* @tx_work: TX work handle
*/ */
struct isert_comp { struct isert_comp {
struct isert_device *device; struct isert_device *device;
struct ib_cq *rx_cq; struct ib_cq *cq;
struct ib_cq *tx_cq;
int active_qps; int active_qps;
struct work_struct rx_work; struct work_struct work;
struct work_struct tx_work;
}; };
struct isert_device { struct isert_device {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment