[RFC PATCH 0/4] scsi/iscsi: Send iscsi data from kblockd

[RFC PATCH 0/4] scsi/iscsi: Send iscsi data from kblockd

[Mike Christie]

The following patches allow iscsi to transmit the initial iscsi PDU and
data from the kblockd context similar to how nvme/tcp does it. This has
the benefit that in a lot of our setups, we run
number_of_sessions == CPUs, so the apps performing IO share the CPU with
the iscsi layer. Right now, a lot of times we end up going from kblockd
to the iscsi workqueue which just adds an extra hop. By running from the
kblockd workqueue we see improvements of 20-30% in IOPs and throughput.

I made this a RFC and cc'd Ming and Bart because I wanted to get some
extra feed back on the first patch:

scsi: Allow drivers to set BLK_MQ_F_BLOCKING

because you guys know that code. For example the iscsi layer doesn't use
scsi_host_block so we won't run into problems there. But I wanted to make
sure there were not other possible issues.

The following patches were made over this iscsi patchset

https://lore.kernel.org/linux-scsi/20220308002747.122682-1-michael.christie@oracle.com/T/#t

the scsi patch applies over Linus or Martin's tree.

[RFC PATCH 1/4] scsi: Allow drivers to set BLK_MQ_F_BLOCKING

[Mike Christie]

The software iscsi driver's queuecommand can block and taking the extra
hop from kblockd to its workqueue results in a performance hit. Allowing
it to set BLK_MQ_F_BLOCKING and transmit from that context directly
results in a 20-30% improvement in IOPs for workloads like:

fio --filename=/dev/sdb --direct=1 --rw=randrw --bs=4k --ioengine=libaio
--iodepth=128  --numjobs=1

and for all write workloads.

when using the none scheduler and the app and iscsi bound to the same
CPUs. Throughput tests show similar gains.

This patch adds a new scsi_host_template field so drivers can tell scsi-ml
that they can block so scsi-ml can setup the tag set with the
BLK_MQ_F_BLOCKING flag.

Signed-off-by: Mike Christie <michael.christie@oracle.com>
---
 drivers/scsi/scsi_lib.c  | 6 ++++--
 include/scsi/scsi_host.h | 4 ++++
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 0a70aa763a96..a5dbeb9994ae 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -1989,6 +1989,8 @@ int scsi_mq_setup_tags(struct Scsi_Host *shost)
 	tag_set->driver_data = shost;
 	if (shost->host_tagset)
 		tag_set->flags |= BLK_MQ_F_TAG_HCTX_SHARED;
+	if (shost->hostt->queuecommand_blocks)
+		tag_set->flags |= BLK_MQ_F_BLOCKING;
 
 	return blk_mq_alloc_tag_set(tag_set);
 }
@@ -2952,8 +2954,8 @@ scsi_host_block(struct Scsi_Host *shost)
 	}
 
 	/*
-	 * SCSI never enables blk-mq's BLK_MQ_F_BLOCKING flag so
-	 * calling synchronize_rcu() once is enough.
+	 * Drivers that use this helper enable blk-mq's BLK_MQ_F_BLOCKING flag
+	 * so calling synchronize_rcu() once is enough.
 	 */
 	WARN_ON_ONCE(shost->tag_set.flags & BLK_MQ_F_BLOCKING);
 
diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h
index 72e1a347baa6..0d106dc9309d 100644
--- a/include/scsi/scsi_host.h
+++ b/include/scsi/scsi_host.h
@@ -75,6 +75,10 @@ struct scsi_host_template {
 	 */
 	int (* queuecommand)(struct Scsi_Host *, struct scsi_cmnd *);
 
+	/*
+	 * Set To true if the queuecommand function can block.
+	 */
+	bool queuecommand_blocks;
 	/*
 	 * The commit_rqs function is used to trigger a hardware
 	 * doorbell after some requests have been queued with
-- 
2.25.1

[RFC PATCH 2/4] scsi: iscsi: Tell drivers when we must not block

[Mike Christie]

When in the next patches we xmit from kblockd we will want to perform
non-blocking IO so we don't affect other sessions while waiting for memory.
This adds an arg to the task and pdu xmit functions so libiscsi can tell
the drivers that support transmitting from both queuecommand and the
iscsi_q wq when they can't block.

Signed-off-by: Mike Christie <michael.christie@oracle.com>
---
 drivers/infiniband/ulp/iser/iscsi_iser.c |  2 +-
 drivers/scsi/be2iscsi/be_main.c          |  2 +-
 drivers/scsi/bnx2i/bnx2i_iscsi.c         |  3 ++-
 drivers/scsi/cxgbi/libcxgbi.c            |  2 +-
 drivers/scsi/cxgbi/libcxgbi.h            |  2 +-
 drivers/scsi/iscsi_tcp.c                 | 16 +++++++++++-----
 drivers/scsi/libiscsi.c                  | 16 ++++++++--------
 drivers/scsi/libiscsi_tcp.c              |  5 +++--
 drivers/scsi/qedi/qedi_iscsi.c           |  2 +-
 drivers/scsi/qla4xxx/ql4_os.c            |  4 ++--
 include/scsi/libiscsi_tcp.h              |  2 +-
 include/scsi/scsi_transport_iscsi.h      |  4 ++--
 12 files changed, 34 insertions(+), 26 deletions(-)

diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.c b/drivers/infiniband/ulp/iser/iscsi_iser.c
index 07e47021a71f..244a4540dbf6 100644
--- a/drivers/infiniband/ulp/iser/iscsi_iser.c
+++ b/drivers/infiniband/ulp/iser/iscsi_iser.c
@@ -317,7 +317,7 @@ static int iscsi_iser_task_xmit_unsol_data(struct iscsi_conn *conn,
  *
  * Return: zero on success or escalates $error on failure.
  */
-static int iscsi_iser_task_xmit(struct iscsi_task *task)
+static int iscsi_iser_task_xmit(struct iscsi_task *task, bool dontwait)
 {
 	struct iscsi_conn *conn = task->conn;
 	struct iscsi_iser_task *iser_task = task->dd_data;
diff --git a/drivers/scsi/be2iscsi/be_main.c b/drivers/scsi/be2iscsi/be_main.c
index 36fddce2786d..fbd0c6981097 100644
--- a/drivers/scsi/be2iscsi/be_main.c
+++ b/drivers/scsi/be2iscsi/be_main.c
@@ -4745,7 +4745,7 @@ static int beiscsi_mtask(struct iscsi_task *task)
 	return 0;
 }
 
-static int beiscsi_task_xmit(struct iscsi_task *task)
+static int beiscsi_task_xmit(struct iscsi_task *task, bool dontwait)
 {
 	struct beiscsi_io_task *io_task = task->dd_data;
 	struct scsi_cmnd *sc = task->sc;
diff --git a/drivers/scsi/bnx2i/bnx2i_iscsi.c b/drivers/scsi/bnx2i/bnx2i_iscsi.c
index a592ca8602f9..d887157c1c02 100644
--- a/drivers/scsi/bnx2i/bnx2i_iscsi.c
+++ b/drivers/scsi/bnx2i/bnx2i_iscsi.c
@@ -1213,10 +1213,11 @@ bnx2i_mtask_xmit(struct iscsi_conn *conn, struct iscsi_task *task)
 /**
  * bnx2i_task_xmit - transmit iscsi command to chip for further processing
  * @task:	transport layer command structure pointer
+ * @dontwait:	true if the driver should not block
  *
  * maps SG buffers and send request to chip/firmware in the form of SQ WQE
  */
-static int bnx2i_task_xmit(struct iscsi_task *task)
+static int bnx2i_task_xmit(struct iscsi_task *task, bool dontwait)
 {
 	struct iscsi_conn *conn = task->conn;
 	struct iscsi_session *session = conn->session;
diff --git a/drivers/scsi/cxgbi/libcxgbi.c b/drivers/scsi/cxgbi/libcxgbi.c
index 411b0d386fad..6fc6cf1a0090 100644
--- a/drivers/scsi/cxgbi/libcxgbi.c
+++ b/drivers/scsi/cxgbi/libcxgbi.c
@@ -2358,7 +2358,7 @@ static int cxgbi_sock_send_skb(struct cxgbi_sock *csk, struct sk_buff *skb)
 	return len;
 }
 
-int cxgbi_conn_xmit_pdu(struct iscsi_task *task)
+int cxgbi_conn_xmit_pdu(struct iscsi_task *task, bool dontwait)
 {
 	struct iscsi_tcp_conn *tcp_conn = task->conn->dd_data;
 	struct cxgbi_conn *cconn = tcp_conn->dd_data;
diff --git a/drivers/scsi/cxgbi/libcxgbi.h b/drivers/scsi/cxgbi/libcxgbi.h
index 3687b5c0cf90..a852dc31171a 100644
--- a/drivers/scsi/cxgbi/libcxgbi.h
+++ b/drivers/scsi/cxgbi/libcxgbi.h
@@ -604,7 +604,7 @@ void cxgbi_conn_tx_open(struct cxgbi_sock *);
 void cxgbi_conn_pdu_ready(struct cxgbi_sock *);
 int cxgbi_conn_alloc_pdu(struct iscsi_task *, u8);
 int cxgbi_conn_init_pdu(struct iscsi_task *, unsigned int , unsigned int);
-int cxgbi_conn_xmit_pdu(struct iscsi_task *);
+int cxgbi_conn_xmit_pdu(struct iscsi_task *, bool);
 
 void cxgbi_cleanup_task(struct iscsi_task *task);
 
diff --git a/drivers/scsi/iscsi_tcp.c b/drivers/scsi/iscsi_tcp.c
index 974245eab605..c2627505011d 100644
--- a/drivers/scsi/iscsi_tcp.c
+++ b/drivers/scsi/iscsi_tcp.c
@@ -279,6 +279,7 @@ iscsi_sw_tcp_conn_restore_callbacks(struct iscsi_conn *conn)
  * iscsi_sw_tcp_xmit_segment - transmit segment
  * @tcp_conn: the iSCSI TCP connection
  * @segment: the buffer to transmnit
+ * @dontwait: true if we should use MSG_DONTWAIT
  *
  * This function transmits as much of the buffer as
  * the network layer will accept, and returns the number of
@@ -289,7 +290,8 @@ iscsi_sw_tcp_conn_restore_callbacks(struct iscsi_conn *conn)
  * it will retrieve the hash value and send it as well.
  */
 static int iscsi_sw_tcp_xmit_segment(struct iscsi_tcp_conn *tcp_conn,
-				     struct iscsi_segment *segment)
+				     struct iscsi_segment *segment,
+				     bool dontwait)
 {
 	struct iscsi_sw_tcp_conn *tcp_sw_conn = tcp_conn->dd_data;
 	struct socket *sk = tcp_sw_conn->sock;
@@ -308,6 +310,9 @@ static int iscsi_sw_tcp_xmit_segment(struct iscsi_tcp_conn *tcp_conn,
 		if (segment->total_copied + segment->size < segment->total_size)
 			flags |= MSG_MORE | MSG_SENDPAGE_NOTLAST;
 
+		if (dontwait)
+			flags |= MSG_DONTWAIT;
+
 		/* Use sendpage if we can; else fall back to sendmsg */
 		if (!segment->data) {
 			sg = segment->sg;
@@ -350,8 +355,9 @@ static int iscsi_sw_tcp_xmit_segment(struct iscsi_tcp_conn *tcp_conn,
 /**
  * iscsi_sw_tcp_xmit - TCP transmit
  * @conn: iscsi connection
+ * @dontwait: true if we should perform nonblocking IO
  **/
-static int iscsi_sw_tcp_xmit(struct iscsi_conn *conn)
+static int iscsi_sw_tcp_xmit(struct iscsi_conn *conn, bool dontwait)
 {
 	struct iscsi_tcp_conn *tcp_conn = conn->dd_data;
 	struct iscsi_sw_tcp_conn *tcp_sw_conn = tcp_conn->dd_data;
@@ -360,7 +366,7 @@ static int iscsi_sw_tcp_xmit(struct iscsi_conn *conn)
 	int rc = 0;
 
 	while (1) {
-		rc = iscsi_sw_tcp_xmit_segment(tcp_conn, segment);
+		rc = iscsi_sw_tcp_xmit_segment(tcp_conn, segment, dontwait);
 		/*
 		 * We may not have been able to send data because the conn
 		 * is getting stopped. libiscsi will know so propagate err
@@ -411,7 +417,7 @@ static inline int iscsi_sw_tcp_xmit_qlen(struct iscsi_conn *conn)
 	return segment->total_copied - segment->total_size;
 }
 
-static int iscsi_sw_tcp_pdu_xmit(struct iscsi_task *task)
+static int iscsi_sw_tcp_pdu_xmit(struct iscsi_task *task, bool dontwait)
 {
 	struct iscsi_conn *conn = task->conn;
 	unsigned int noreclaim_flag;
@@ -428,7 +434,7 @@ static int iscsi_sw_tcp_pdu_xmit(struct iscsi_task *task)
 	noreclaim_flag = memalloc_noreclaim_save();
 
 	while (iscsi_sw_tcp_xmit_qlen(conn)) {
-		rc = iscsi_sw_tcp_xmit(conn);
+		rc = iscsi_sw_tcp_xmit(conn, dontwait);
 		if (rc == 0) {
 			rc = -EAGAIN;
 			break;
diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c
index fcf5c30614ba..63e0d97df50f 100644
--- a/drivers/scsi/libiscsi.c
+++ b/drivers/scsi/libiscsi.c
@@ -812,7 +812,7 @@ static int iscsi_send_mgmt_task(struct iscsi_task *task)
 		if (rc)
 			return rc;
 
-		rc = session->tt->xmit_task(task);
+		rc = session->tt->xmit_task(task, false);
 		if (rc)
 			return rc;
 	} else {
@@ -1498,7 +1498,7 @@ static int iscsi_check_cmdsn_window_closed(struct iscsi_conn *conn)
 }
 
 static int iscsi_xmit_task(struct iscsi_conn *conn, struct iscsi_task *task,
-			   bool was_requeue)
+			   bool was_requeue, bool dontwait)
 {
 	int rc;
 
@@ -1539,7 +1539,7 @@ static int iscsi_xmit_task(struct iscsi_conn *conn, struct iscsi_task *task,
 	}
 
 	spin_unlock_bh(&conn->session->frwd_lock);
-	rc = conn->session->tt->xmit_task(task);
+	rc = conn->session->tt->xmit_task(task, dontwait);
 	spin_lock_bh(&conn->session->frwd_lock);
 	if (!rc) {
 		/* done with this task */
@@ -1608,7 +1608,7 @@ static int iscsi_data_xmit(struct iscsi_conn *conn)
 	}
 
 	if (conn->task) {
-		rc = iscsi_xmit_task(conn, conn->task, false);
+		rc = iscsi_xmit_task(conn, conn->task, false, false);
 	        if (rc)
 		        goto done;
 	}
@@ -1630,7 +1630,7 @@ static int iscsi_data_xmit(struct iscsi_conn *conn)
 			spin_unlock_bh(&conn->session->back_lock);
 			continue;
 		}
-		rc = iscsi_xmit_task(conn, task, false);
+		rc = iscsi_xmit_task(conn, task, false, false);
 		if (rc)
 			goto done;
 	}
@@ -1652,7 +1652,7 @@ static int iscsi_data_xmit(struct iscsi_conn *conn)
 				fail_scsi_task(task, DID_ABORT);
 			continue;
 		}
-		rc = iscsi_xmit_task(conn, task, false);
+		rc = iscsi_xmit_task(conn, task, false, false);
 		if (rc)
 			goto done;
 		/*
@@ -1678,7 +1678,7 @@ static int iscsi_data_xmit(struct iscsi_conn *conn)
 			break;
 
 		list_del_init(&task->running);
-		rc = iscsi_xmit_task(conn, task, true);
+		rc = iscsi_xmit_task(conn, task, true, false);
 		if (rc)
 			goto done;
 		if (!list_empty(&conn->mgmtqueue))
@@ -1843,7 +1843,7 @@ int iscsi_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *sc)
 				goto prepd_fault;
 			}
 		}
-		if (session->tt->xmit_task(task)) {
+		if (session->tt->xmit_task(task, true)) {
 			session->cmdsn--;
 			reason = FAILURE_SESSION_NOT_READY;
 			goto prepd_reject;
diff --git a/drivers/scsi/libiscsi_tcp.c b/drivers/scsi/libiscsi_tcp.c
index defe08142b75..2c783862963d 100644
--- a/drivers/scsi/libiscsi_tcp.c
+++ b/drivers/scsi/libiscsi_tcp.c
@@ -1060,12 +1060,13 @@ static struct iscsi_r2t_info *iscsi_tcp_get_curr_r2t(struct iscsi_task *task)
 /**
  * iscsi_tcp_task_xmit - xmit normal PDU task
  * @task: iscsi command task
+ * @dontwait: true if the driver should not wait for wmem space
  *
  * We're expected to return 0 when everything was transmitted successfully,
  * -EAGAIN if there's still data in the queue, or != 0 for any other kind
  * of error.
  */
-int iscsi_tcp_task_xmit(struct iscsi_task *task)
+int iscsi_tcp_task_xmit(struct iscsi_task *task, bool dontwait)
 {
 	struct iscsi_conn *conn = task->conn;
 	struct iscsi_session *session = conn->session;
@@ -1074,7 +1075,7 @@ int iscsi_tcp_task_xmit(struct iscsi_task *task)
 
 flush:
 	/* Flush any pending data first. */
-	rc = session->tt->xmit_pdu(task);
+	rc = session->tt->xmit_pdu(task, dontwait);
 	if (rc < 0)
 		return rc;
 
diff --git a/drivers/scsi/qedi/qedi_iscsi.c b/drivers/scsi/qedi/qedi_iscsi.c
index 282ecb4e39bb..80a9bd4ef65e 100644
--- a/drivers/scsi/qedi/qedi_iscsi.c
+++ b/drivers/scsi/qedi/qedi_iscsi.c
@@ -827,7 +827,7 @@ static int qedi_mtask_xmit(struct iscsi_conn *conn, struct iscsi_task *task)
 	return qedi_iscsi_send_generic_request(task);
 }
 
-static int qedi_task_xmit(struct iscsi_task *task)
+static int qedi_task_xmit(struct iscsi_task *task, bool dontwait)
 {
 	struct iscsi_conn *conn = task->conn;
 	struct qedi_conn *qedi_conn = conn->dd_data;
diff --git a/drivers/scsi/qla4xxx/ql4_os.c b/drivers/scsi/qla4xxx/ql4_os.c
index 955d8cb675f1..60f8c10c000d 100644
--- a/drivers/scsi/qla4xxx/ql4_os.c
+++ b/drivers/scsi/qla4xxx/ql4_os.c
@@ -137,7 +137,7 @@ qla4xxx_session_create(struct iscsi_endpoint *ep, uint16_t cmds_max,
 static void qla4xxx_session_destroy(struct iscsi_cls_session *sess);
 static void qla4xxx_task_work(struct work_struct *wdata);
 static int qla4xxx_alloc_pdu(struct iscsi_task *, uint8_t);
-static int qla4xxx_task_xmit(struct iscsi_task *);
+static int qla4xxx_task_xmit(struct iscsi_task *, bool);
 static void qla4xxx_task_cleanup(struct iscsi_task *);
 static void qla4xxx_fail_session(struct iscsi_cls_session *cls_session);
 static void qla4xxx_conn_get_stats(struct iscsi_cls_conn *cls_conn,
@@ -3477,7 +3477,7 @@ static void qla4xxx_task_cleanup(struct iscsi_task *task)
 	return;
 }
 
-static int qla4xxx_task_xmit(struct iscsi_task *task)
+static int qla4xxx_task_xmit(struct iscsi_task *task, bool dontwait)
 {
 	struct scsi_cmnd *sc = task->sc;
 	struct iscsi_session *sess = task->conn->session;
diff --git a/include/scsi/libiscsi_tcp.h b/include/scsi/libiscsi_tcp.h
index 7c8ba9d7378b..9fb97cbfa05c 100644
--- a/include/scsi/libiscsi_tcp.h
+++ b/include/scsi/libiscsi_tcp.h
@@ -88,7 +88,7 @@ extern int iscsi_tcp_recv_skb(struct iscsi_conn *conn, struct sk_buff *skb,
 			      unsigned int offset, bool offloaded, int *status);
 extern void iscsi_tcp_cleanup_task(struct iscsi_task *task);
 extern int iscsi_tcp_task_init(struct iscsi_task *task);
-extern int iscsi_tcp_task_xmit(struct iscsi_task *task);
+extern int iscsi_tcp_task_xmit(struct iscsi_task *task, bool dontwait);
 
 /* segment helpers */
 extern int iscsi_tcp_recv_segment_is_hdr(struct iscsi_tcp_conn *tcp_conn);
diff --git a/include/scsi/scsi_transport_iscsi.h b/include/scsi/scsi_transport_iscsi.h
index 7a0d24d3b916..d3b37d28e11a 100644
--- a/include/scsi/scsi_transport_iscsi.h
+++ b/include/scsi/scsi_transport_iscsi.h
@@ -108,11 +108,11 @@ struct iscsi_transport {
 			   struct iscsi_stats *stats);
 
 	int (*init_task) (struct iscsi_task *task);
-	int (*xmit_task) (struct iscsi_task *task);
+	int (*xmit_task) (struct iscsi_task *task, bool dontwait);
 	void (*cleanup_task) (struct iscsi_task *task);
 
 	int (*alloc_pdu) (struct iscsi_task *task, uint8_t opcode);
-	int (*xmit_pdu) (struct iscsi_task *task);
+	int (*xmit_pdu) (struct iscsi_task *task, bool dontwait);
 	int (*init_pdu) (struct iscsi_task *task, unsigned int offset,
 			 unsigned int count);
 	void (*parse_pdu_itt) (struct iscsi_conn *conn, itt_t itt,
-- 
2.25.1

[RFC PATCH 3/4] scsi: iscsi: Support transmit from queuecommand

[Mike Christie]

This patch alllow drivers like iscsi_tcp to transmit the task from the
queuecommand. For the case where we were going to run from kblockd and the
app and iscsi share CPUs, we can skip the extra workqueue runs and it can
result in up to 30% increase in 4K IOPs.

Signed-off-by: Mike Christie <michael.christie@oracle.com>
---
 drivers/scsi/libiscsi.c | 52 +++++++++++++++++++++++++++++++++++------
 include/scsi/libiscsi.h | 11 +++++++--
 2 files changed, 54 insertions(+), 9 deletions(-)

diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c
index 63e0d97df50f..5a2953260a94 100644
--- a/drivers/scsi/libiscsi.c
+++ b/drivers/scsi/libiscsi.c
@@ -1700,9 +1700,13 @@ static void iscsi_xmitworker(struct work_struct *work)
 	/*
 	 * serialize Xmit worker on a per-connection basis.
 	 */
+	mutex_lock(&conn->xmit_mutex);
+
 	do {
 		rc = iscsi_data_xmit(conn);
 	} while (rc >= 0 || rc == -EAGAIN);
+
+	mutex_unlock(&conn->xmit_mutex);
 }
 
 static inline struct iscsi_task *iscsi_alloc_task(struct iscsi_conn *conn,
@@ -1832,9 +1836,16 @@ int iscsi_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *sc)
 		goto reject;
 	}
 
-	if (!ihost->workq) {
+	session->queued_cmdsn++;
+
+	if (!ihost->workq ||
+	    (ihost->xmit_from_qc && !conn->task &&
+	     list_empty(&conn->cmdqueue) && mutex_trylock(&conn->xmit_mutex))) {
 		reason = iscsi_prep_scsi_cmd_pdu(task);
 		if (reason) {
+			if (ihost->workq)
+				mutex_unlock(&conn->xmit_mutex);
+
 			if (reason == -ENOMEM ||  reason == -EACCES) {
 				reason = FAILURE_OOM;
 				goto prepd_reject;
@@ -1843,21 +1854,27 @@ int iscsi_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *sc)
 				goto prepd_fault;
 			}
 		}
-		if (session->tt->xmit_task(task, true)) {
-			session->cmdsn--;
-			reason = FAILURE_SESSION_NOT_READY;
-			goto prepd_reject;
+
+		if (!ihost->workq) {
+			if (session->tt->xmit_task(task, true)) {
+				session->cmdsn--;
+				reason = FAILURE_SESSION_NOT_READY;
+				goto prepd_reject;
+			}
+		} else {
+			iscsi_xmit_task(conn, task, false, true);
+			mutex_unlock(&conn->xmit_mutex);
 		}
 	} else {
 		list_add_tail(&task->running, &conn->cmdqueue);
 		iscsi_conn_queue_xmit(conn);
 	}
 
-	session->queued_cmdsn++;
 	spin_unlock_bh(&session->frwd_lock);
 	return 0;
 
 prepd_reject:
+	session->queued_cmdsn--;
 	spin_lock_bh(&session->back_lock);
 	iscsi_complete_task(task, ISCSI_TASK_REQUEUE_SCSIQ);
 	spin_unlock_bh(&session->back_lock);
@@ -1868,6 +1885,7 @@ int iscsi_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *sc)
 	return SCSI_MLQUEUE_TARGET_BUSY;
 
 prepd_fault:
+	session->queued_cmdsn--;
 	spin_lock_bh(&session->back_lock);
 	iscsi_complete_task(task, ISCSI_TASK_REQUEUE_SCSIQ);
 	spin_unlock_bh(&session->back_lock);
@@ -2558,11 +2576,17 @@ int iscsi_eh_device_reset(struct scsi_cmnd *sc)
 
 	iscsi_suspend_tx(conn);
 
+	/*
+	 * If this is called from a SG ioctl the host may not be fully stopped.
+	 * Take the xmit_mutex in case we are xmitting from iscsi_queuecommand.
+	 */
+	mutex_lock(&conn->xmit_mutex);
 	spin_lock_bh(&session->frwd_lock);
 	memset(hdr, 0, sizeof(*hdr));
 	fail_scsi_tasks(conn, sc->device->lun, DID_ERROR);
 	session->tmf_state = TMF_INITIAL;
 	spin_unlock_bh(&session->frwd_lock);
+	mutex_unlock(&conn->xmit_mutex);
 
 	iscsi_start_tx(conn);
 	goto done;
@@ -2720,11 +2744,17 @@ static int iscsi_eh_target_reset(struct scsi_cmnd *sc)
 
 	iscsi_suspend_tx(conn);
 
+	/*
+	 * If this is called from a SG ioctl the host may not be fully stopped.
+	 * Take the xmit_mutex in case we are xmitting from iscsi_queuecommand.
+	 */
+	mutex_lock(&conn->xmit_mutex);
 	spin_lock_bh(&session->frwd_lock);
 	memset(hdr, 0, sizeof(*hdr));
 	fail_scsi_tasks(conn, -1, DID_ERROR);
 	session->tmf_state = TMF_INITIAL;
 	spin_unlock_bh(&session->frwd_lock);
+	mutex_unlock(&conn->xmit_mutex);
 
 	iscsi_start_tx(conn);
 	goto done;
@@ -2909,6 +2939,8 @@ struct Scsi_Host *iscsi_host_alloc(struct scsi_host_template *sht,
 			2, shost->host_no);
 		if (!ihost->workq)
 			goto free_host;
+	} else {
+		ihost->xmit_from_qc = true;
 	}
 
 	spin_lock_init(&ihost->lock);
@@ -3165,6 +3197,7 @@ iscsi_conn_setup(struct iscsi_cls_session *cls_session, int dd_size,
 	INIT_LIST_HEAD(&conn->cmdqueue);
 	INIT_LIST_HEAD(&conn->requeue);
 	INIT_WORK(&conn->xmitwork, iscsi_xmitworker);
+	mutex_init(&conn->xmit_mutex);
 
 	/* allocate login_task used for the login/text sequences */
 	spin_lock_bh(&session->frwd_lock);
@@ -3395,13 +3428,18 @@ void iscsi_conn_stop(struct iscsi_cls_conn *cls_conn, int flag)
 	}
 
 	/*
-	 * flush queues.
+	 * When flushing the queues we don't wait for the block to complete
+	 * above because it can be expensive when there are lots of devices.
+	 * To make sure there is not an xmit from iscsi_queuecommand running
+	 * take the xmit_mutex.
 	 */
+	mutex_lock(&conn->xmit_mutex);
 	spin_lock_bh(&session->frwd_lock);
 	fail_scsi_tasks(conn, -1, DID_TRANSPORT_DISRUPTED);
 	fail_mgmt_tasks(session, conn);
 	memset(&session->tmhdr, 0, sizeof(session->tmhdr));
 	spin_unlock_bh(&session->frwd_lock);
+	mutex_unlock(&conn->xmit_mutex);
 	mutex_unlock(&session->eh_mutex);
 }
 EXPORT_SYMBOL_GPL(iscsi_conn_stop);
diff --git a/include/scsi/libiscsi.h b/include/scsi/libiscsi.h
index 412722f44747..1f2accf2bc1b 100644
--- a/include/scsi/libiscsi.h
+++ b/include/scsi/libiscsi.h
@@ -198,6 +198,12 @@ struct iscsi_conn {
 	struct list_head	cmdqueue;	/* data-path cmd queue */
 	struct list_head	requeue;	/* tasks needing another run */
 	struct work_struct	xmitwork;	/* per-conn. xmit workqueue */
+	/*
+	 * This must be held while calling the xmit_task callout if it can
+	 * called from the iscsi_q workqueue. It must be taken after the
+	 * eh_mutex if both mutex's are needed.
+	 */
+	struct mutex		xmit_mutex;
 	/* recv */
 	struct work_struct	recvwork;
 	unsigned long		flags;		/* ISCSI_CONN_FLAGs */
@@ -267,8 +273,8 @@ struct iscsi_session {
 	struct iscsi_cls_session *cls_session;
 	/*
 	 * Syncs up the scsi eh thread with the iscsi eh thread when sending
-	 * task management functions. This must be taken before the session
-	 * and recv lock.
+	 * task management functions. This must be taken before the conn's
+	 * xmit_mutex.
 	 */
 	struct mutex		eh_mutex;
 	/* abort */
@@ -370,6 +376,7 @@ struct iscsi_host {
 	int			num_sessions;
 	int			state;
 
+	bool			xmit_from_qc;
 	struct workqueue_struct	*workq;
 };
 
-- 
2.25.1

[RFC PATCH 4/4] scsi: iscsi_tcp: Allow user to control if transmit from queuecommand

[Mike Christie]

Transmitting from the queuecommand is nice when your app and iscsi threads
have to run on the same CPU. But, for the case where you want higher
throughput/IOPs it's sometimes better to hog multiple CPUs with the app on
one CPU and the xmit/recv paths an another. To allow both configs this
adds a modparam.

Signed-off-by: Mike Christie <michael.christie@oracle.com>
---
 drivers/scsi/iscsi_tcp.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/scsi/iscsi_tcp.c b/drivers/scsi/iscsi_tcp.c
index c2627505011d..c48707b53746 100644
--- a/drivers/scsi/iscsi_tcp.c
+++ b/drivers/scsi/iscsi_tcp.c
@@ -56,6 +56,10 @@ static bool iscsi_use_recv_wq;
 module_param_named(use_recv_wq, iscsi_use_recv_wq, bool, 0644);
 MODULE_PARM_DESC(use_recv_wq, "Set to true to read iSCSI data/headers from the iscsi_q workqueue. The default is false which will perform reads from the network softirq context.");
 
+static bool iscsi_xmit_from_qc;
+module_param_named(xmit_from_queuecommand, iscsi_xmit_from_qc, bool, 0644);
+MODULE_PARM_DESC(xmit_from_queuecommand, "Set to true to try to xmit the task from the queuecommand callout. The default is false wihch will xmit the task from the iscsi_q workqueue.");
+
 static int iscsi_sw_tcp_dbg;
 module_param_named(debug_iscsi_tcp, iscsi_sw_tcp_dbg, int,
 		   S_IRUGO | S_IWUSR);
@@ -909,6 +913,7 @@ iscsi_sw_tcp_session_create(struct iscsi_endpoint *ep, uint16_t cmds_max,
 	struct iscsi_cls_session *cls_session;
 	struct iscsi_session *session;
 	struct iscsi_sw_tcp_host *tcp_sw_host;
+	struct iscsi_host *ihost;
 	struct Scsi_Host *shost;
 	int rc;
 
@@ -928,6 +933,12 @@ iscsi_sw_tcp_session_create(struct iscsi_endpoint *ep, uint16_t cmds_max,
 	shost->max_channel = 0;
 	shost->max_cmd_len = SCSI_MAX_VARLEN_CDB_SIZE;
 
+	if (iscsi_xmit_from_qc) {
+		shost->hostt->queuecommand_blocks = true;
+		ihost = shost_priv(shost);
+		ihost->xmit_from_qc = true;
+	}
+
 	rc = iscsi_host_get_max_scsi_cmds(shost, cmds_max);
 	if (rc < 0)
 		goto free_host;
-- 
2.25.1

Re: [RFC PATCH 1/4] scsi: Allow drivers to set BLK_MQ_F_BLOCKING

[Bart Van Assche]

On 3/7/22 16:39, Mike Christie wrote:
> The software iscsi driver's queuecommand can block and taking the extra
> hop from kblockd to its workqueue results in a performance hit. Allowing
> it to set BLK_MQ_F_BLOCKING and transmit from that context directly
> results in a 20-30% improvement in IOPs for workloads like: [...]

That's impressive!

> @@ -2952,8 +2954,8 @@ scsi_host_block(struct Scsi_Host *shost)
>   	}
>   
>   	/*
> -	 * SCSI never enables blk-mq's BLK_MQ_F_BLOCKING flag so
> -	 * calling synchronize_rcu() once is enough.
> +	 * Drivers that use this helper enable blk-mq's BLK_MQ_F_BLOCKING flag
> +	 * so calling synchronize_rcu() once is enough.
>   	 */
>   	WARN_ON_ONCE(shost->tag_set.flags & BLK_MQ_F_BLOCKING);

s/enable/do not set/ ?

> diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h
> index 72e1a347baa6..0d106dc9309d 100644
> --- a/include/scsi/scsi_host.h
> +++ b/include/scsi/scsi_host.h
> @@ -75,6 +75,10 @@ struct scsi_host_template {
>   	 */
>   	int (* queuecommand)(struct Scsi_Host *, struct scsi_cmnd *);
>   
> +	/*
> +	 * Set To true if the queuecommand function can block.
> +	 */
> +	bool queuecommand_blocks;
>   	/*
>   	 * The commit_rqs function is used to trigger a hardware
>   	 * doorbell after some requests have been queued with

I'm not sure what the best name is for this new flag. Some function 
names refer to sleeping (e.g. might_sleep()) while the flag 
BLK_MQ_F_BLOCKING has "blocking" in its name. Although I do not have a 
strong opinion about this: has it been considered to use a name like 
queuecommand_sleeps or queuecommand_may_sleep instead of 
queuecommand_blocks?

Thanks,

Bart.

Re: [RFC PATCH 2/4] scsi: iscsi: Tell drivers when we must not block

[Bart Van Assche]

On 3/7/22 16:39, Mike Christie wrote:
> -static int iscsi_iser_task_xmit(struct iscsi_task *task)
> +static int iscsi_iser_task_xmit(struct iscsi_task *task, bool dontwait)

Arguments with "not" in their name easily lead to double negations. Has 
it been considered to change "bool dontwait" into "bool may_sleep" and 
to inverse the argument passed to this and similar functions?

Thanks,

Bart.

Re: [RFC PATCH 4/4] scsi: iscsi_tcp: Allow user to control if transmit from queuecommand

[Bart Van Assche]

On 3/7/22 16:39, Mike Christie wrote:
> +static bool iscsi_xmit_from_qc;
> +module_param_named(xmit_from_queuecommand, iscsi_xmit_from_qc, bool, 0644);
> +MODULE_PARM_DESC(xmit_from_queuecommand, "Set to true to try to xmit the task from the queuecommand callout. The default is false wihch will xmit the task from the iscsi_q workqueue.");

s/wihch/which/ ?

It may be hard for users to get the value of this parameter right. Has 
it been considered to make the iSCSI initiator select the proper mode 
depending on the load? I think the DPDK and SPDK software supports this. 
This is supported via user-level multithreading and by scaling to more 
CPU cores if required to achieve full performance.

Thanks,

Bart.

RE: [RFC PATCH 1/4] scsi: Allow drivers to set BLK_MQ_F_BLOCKING

[Daejun Park]

Hi Mike, 

>@@ -2952,8 +2954,8 @@ scsi_host_block(struct Scsi_Host *shost)
>         }
> 
>         /*
>-         * SCSI never enables blk-mq's BLK_MQ_F_BLOCKING flag so
>-         * calling synchronize_rcu() once is enough.
>+         * Drivers that use this helper enable blk-mq's BLK_MQ_F_BLOCKING flag
>+         * so calling synchronize_rcu() once is enough.
>          */
>         WARN_ON_ONCE(shost->tag_set.flags & BLK_MQ_F_BLOCKING);
Should we keep this line?

And I think scsi_host_block() should call synchronize_srcu() rather than
synchronize_rcu(), if the BLK_MQ_F_BLOCKING flag is used.

Thanks,
Daejun

Re: [RFC PATCH 2/4] scsi: iscsi: Tell drivers when we must not block

[Mike Christie]

On 3/7/22 10:59 PM, Bart Van Assche wrote:
> On 3/7/22 16:39, Mike Christie wrote:
>> -static int iscsi_iser_task_xmit(struct iscsi_task *task)
>> +static int iscsi_iser_task_xmit(struct iscsi_task *task, bool dontwait)
> 
> Arguments with "not" in their name easily lead to double negations. Has it been considered to change "bool dontwait" into "bool may_sleep" and to inverse the argument passed to this and similar functions?

Yeah, I'm not tied to any naming. I just copied the name from the
network layer's MSG_DONTWAIT flag.

I was going to suggest something with "block" in the name but I
I saw in your other comment you might like something with sleep.

may_sleep is fine with me. For the queuecommand related field,
I'll do a sleep variant too.

Re: [RFC PATCH 1/4] scsi: Allow drivers to set BLK_MQ_F_BLOCKING

[Mike Christie]

On 3/8/22 1:00 AM, Daejun Park wrote:
> Hi Mike, 
> 
>> @@ -2952,8 +2954,8 @@ scsi_host_block(struct Scsi_Host *shost)
>>         }
>>
>>         /*
>> -         * SCSI never enables blk-mq's BLK_MQ_F_BLOCKING flag so
>> -         * calling synchronize_rcu() once is enough.
>> +         * Drivers that use this helper enable blk-mq's BLK_MQ_F_BLOCKING flag
>> +         * so calling synchronize_rcu() once is enough.
>>          */
>>         WARN_ON_ONCE(shost->tag_set.flags & BLK_MQ_F_BLOCKING);
> Should we keep this line?
> 
> And I think scsi_host_block() should call synchronize_srcu() rather than
> synchronize_rcu(), if the BLK_MQ_F_BLOCKING flag is used.
>

Sorry, I messed up the comment.

The comment should have been:

Drivers that set the blk-mq BLK_MQ_F_BLOCKING do not use this function
so calling synchronize_rcu() once is enough.

--------------------------

The iscsi drivers that use BLK_MQ_F_BLOCKING use scsi_target_block which
ends up calling blk_mq_wait_quiesce_done. That function will do the right
thing wrt rcu/srcu based on if the BLK_MQ_F_BLOCKING flag was set when
the scsi host was added.

So in the above comment I meant to express drivers that use BLK_MQ_F_BLOCKING
shouldn't call scsi_host_block and right now the WARN call is valid.

However, I could do something like you are suggesting and do a:

if (shost->hostt->queuecommand_may_sleep_blocks)
	synchronize_srcu()
else
	synchronize_rcu()

However, I don't have any way to test it, so I was a little reluctant
to add that.

Re: [RFC PATCH 4/4] scsi: iscsi_tcp: Allow user to control if transmit from queuecommand

[Mike Christie]

On 3/7/22 11:10 PM, Bart Van Assche wrote:
> On 3/7/22 16:39, Mike Christie wrote:
>> +static bool iscsi_xmit_from_qc;
>> +module_param_named(xmit_from_queuecommand, iscsi_xmit_from_qc, bool, 0644);
>> +MODULE_PARM_DESC(xmit_from_queuecommand, "Set to true to try to xmit the task from the queuecommand callout. The default is false wihch will xmit the task from the iscsi_q workqueue.");
> 
> s/wihch/which/ ?
> 
> It may be hard for users to get the value of this parameter right. Has it been considered to make the iSCSI initiator select the proper mode depending on the load? I think the DPDK and SPDK software supports this. This is supported via user-level multithreading and by scaling to more CPU cores if required to achieve full performance.

The problem is guessing what the user is doing and wants.
Users just do a wide range of things for whatever reasons:

1. No tuning at all. Let iscsi, network and app run wild on all CPUs.

2. Pin iscsi to specific CPUs. Let the app run wild. Or pin the
networking and let iscsi and app run wild.

3. Pin iscsi and network to a specific CPU. Let app run wild.

4. Pin everything.

They all have their benefits drawbacks like if you put the network
and iscsi on the same CPU (or put them on different ones), and run
the app on a different one you get the highest performance for a lot
of workloads. Abusing the CPUs overcome the loss of caching. In this
case, you might not want to use xmit_from_queuecommand because we
could be transmitting from the CPU the app is on and it might have
wanted those CPU cycle for it's own work.

However, you might have to put everything on the same CPUs because they
can't interfere with other workloads. In that case you take the perf
hit. In this case xmit_from_queuecommand=true works really well for you.

Note, if you start with xmit_from_queuecommand=true and if the user is
doing a workload that doesn't allow us to transmit from the queucommand
then we do drop back to the old style of doing things. We also do that
for the case where SCSI WRITEs have to be broken up.

So I didn't make it dynamic or change the default so I wouldn't mess
up existing users.

Re: [RFC PATCH 1/4] scsi: Allow drivers to set BLK_MQ_F_BLOCKING

[Ming Lei]

On Mon, Mar 07, 2022 at 06:39:54PM -0600, Mike Christie wrote:
> The software iscsi driver's queuecommand can block and taking the extra
> hop from kblockd to its workqueue results in a performance hit. Allowing
> it to set BLK_MQ_F_BLOCKING and transmit from that context directly
> results in a 20-30% improvement in IOPs for workloads like:
> 
> fio --filename=/dev/sdb --direct=1 --rw=randrw --bs=4k --ioengine=libaio
> --iodepth=128  --numjobs=1
> 
> and for all write workloads.

This single patch shouldn't make any difference for iscsi, so please
make it as last one if performance improvement data is provided
in commit log.

Also is there performance effect for other worloads? such as multiple
jobs? iscsi is SQ hardware, so if driver is blocked in ->queuecommand()
via BLK_MQ_F_BLOCKING, other contexts can't submit IO to scsi ML any more.


thanks,
Ming

Re: [RFC PATCH 1/4] scsi: Allow drivers to set BLK_MQ_F_BLOCKING

[Mike Christie]

On 3/8/22 6:53 PM, Ming Lei wrote:
> On Mon, Mar 07, 2022 at 06:39:54PM -0600, Mike Christie wrote:
>> The software iscsi driver's queuecommand can block and taking the extra
>> hop from kblockd to its workqueue results in a performance hit. Allowing
>> it to set BLK_MQ_F_BLOCKING and transmit from that context directly
>> results in a 20-30% improvement in IOPs for workloads like:
>>
>> fio --filename=/dev/sdb --direct=1 --rw=randrw --bs=4k --ioengine=libaio
>> --iodepth=128  --numjobs=1
>>
>> and for all write workloads.
> 
> This single patch shouldn't make any difference for iscsi, so please
> make it as last one if performance improvement data is provided
> in commit log.

Ok.

> 
> Also is there performance effect for other worloads? such as multiple
> jobs? iscsi is SQ hardware, so if driver is blocked in ->queuecommand()
> via BLK_MQ_F_BLOCKING, other contexts can't submit IO to scsi ML any more.

If you mean multiple jobs running on the same connection/session then
they are all serialized now. A connection can only do 1 cmd at a time.
There's a big mutex around it in the network layer, so multiple jobs
just suck no matter what.

If you mean multiple jobs from different connection/sessions, then the
iscsi code with this patchset blocks only because the network layer
takes a mutex for a short time. We configure it to not block for things
like socket space, memory allocations, we do zero copy IO normally, etc
so it's quick.

We also can do up to workqueues max_active limit worth of calls so
other things can normally send IO. We haven't found a need to increase
it yet.

Re: [RFC PATCH 1/4] scsi: Allow drivers to set BLK_MQ_F_BLOCKING

[Ming Lei]

On Tue, Mar 08, 2022 at 07:17:13PM -0600, Mike Christie wrote:
> On 3/8/22 6:53 PM, Ming Lei wrote:
> > On Mon, Mar 07, 2022 at 06:39:54PM -0600, Mike Christie wrote:
> >> The software iscsi driver's queuecommand can block and taking the extra
> >> hop from kblockd to its workqueue results in a performance hit. Allowing
> >> it to set BLK_MQ_F_BLOCKING and transmit from that context directly
> >> results in a 20-30% improvement in IOPs for workloads like:
> >>
> >> fio --filename=/dev/sdb --direct=1 --rw=randrw --bs=4k --ioengine=libaio
> >> --iodepth=128  --numjobs=1
> >>
> >> and for all write workloads.
> > 
> > This single patch shouldn't make any difference for iscsi, so please
> > make it as last one if performance improvement data is provided
> > in commit log.
> 
> Ok.
> 
> > 
> > Also is there performance effect for other worloads? such as multiple
> > jobs? iscsi is SQ hardware, so if driver is blocked in ->queuecommand()
> > via BLK_MQ_F_BLOCKING, other contexts can't submit IO to scsi ML any more.
> 
> If you mean multiple jobs running on the same connection/session then
> they are all serialized now. A connection can only do 1 cmd at a time.
> There's a big mutex around it in the network layer, so multiple jobs
> just suck no matter what.

I guess one block device can only bind to one isci connection, given the
1 cmd per connection limit, so looks multiple jobs is fine.

> 
> If you mean multiple jobs from different connection/sessions, then the
> iscsi code with this patchset blocks only because the network layer
> takes a mutex for a short time. We configure it to not block for things
> like socket space, memory allocations, we do zero copy IO normally, etc
> so it's quick.
> 
> We also can do up to workqueues max_active limit worth of calls so
> other things can normally send IO. We haven't found a need to increase
> it yet.
 
I meant that hctx->run_work is required for blk-mq to dispatch IO, iscsi is
SQ HBA, so there is only single work_struct. If one context is blocked in
->queue_rq or ->queuecommand, other contexts can't submit IO to driver any
more.


Thanks,
Ming

Re: [RFC PATCH 1/4] scsi: Allow drivers to set BLK_MQ_F_BLOCKING

[Mike Christie]

On 3/8/22 7:43 PM, Ming Lei wrote:
> On Tue, Mar 08, 2022 at 07:17:13PM -0600, Mike Christie wrote:
>> On 3/8/22 6:53 PM, Ming Lei wrote:
>>> On Mon, Mar 07, 2022 at 06:39:54PM -0600, Mike Christie wrote:
>>>> The software iscsi driver's queuecommand can block and taking the extra
>>>> hop from kblockd to its workqueue results in a performance hit. Allowing
>>>> it to set BLK_MQ_F_BLOCKING and transmit from that context directly
>>>> results in a 20-30% improvement in IOPs for workloads like:
>>>>
>>>> fio --filename=/dev/sdb --direct=1 --rw=randrw --bs=4k --ioengine=libaio
>>>> --iodepth=128  --numjobs=1
>>>>
>>>> and for all write workloads.
>>>
>>> This single patch shouldn't make any difference for iscsi, so please
>>> make it as last one if performance improvement data is provided
>>> in commit log.
>>
>> Ok.
>>
>>>
>>> Also is there performance effect for other worloads? such as multiple
>>> jobs? iscsi is SQ hardware, so if driver is blocked in ->queuecommand()
>>> via BLK_MQ_F_BLOCKING, other contexts can't submit IO to scsi ML any more.
>>
>> If you mean multiple jobs running on the same connection/session then
>> they are all serialized now. A connection can only do 1 cmd at a time.
>> There's a big mutex around it in the network layer, so multiple jobs
>> just suck no matter what.
> 
> I guess one block device can only bind to one isci connection, given the
> 1 cmd per connection limit, so looks multiple jobs is fine.
> 
>>
>> If you mean multiple jobs from different connection/sessions, then the
>> iscsi code with this patchset blocks only because the network layer
>> takes a mutex for a short time. We configure it to not block for things
>> like socket space, memory allocations, we do zero copy IO normally, etc
>> so it's quick.
>>
>> We also can do up to workqueues max_active limit worth of calls so
>> other things can normally send IO. We haven't found a need to increase
>> it yet.
>  
> I meant that hctx->run_work is required for blk-mq to dispatch IO, iscsi is
> SQ HBA, so there is only single work_struct. If one context is blocked in
> ->queue_rq or ->queuecommand, other contexts can't submit IO to driver any
> more.

I see what you mean. With the current code, we have the same issue already.
We have 1 work_struct per connection/session and one connection/session
per scsi_host.

Basically, the iscsi protocol and socket layer only allow us to send the 1
command per connection at a time (you can't have 2 threads doing
sendmsg/sendpage). It's why nvme/tcp is a lot better. It makes N tcp
connections and each hwctx can use a different one.

Re: [RFC PATCH 0/4] scsi/iscsi: Send iscsi data from kblockd

[Christoph Hellwig]

The subject seems a bit misleading, I'd expect to see
BLK_MQ_F_BLOCKING in the subject.

Note that you'll also need the series from Ming to support dm-multipath
on top devices that set BLK_MQ_F_BLOCKING.

Re: [RFC PATCH 0/4] scsi/iscsi: Send iscsi data from kblockd

[Ming Lei]

On Tue, Mar 15, 2022 at 01:26:40AM -0700, Christoph Hellwig wrote:
> The subject seems a bit misleading, I'd expect to see
> BLK_MQ_F_BLOCKING in the subject.
> 
> Note that you'll also need the series from Ming to support dm-multipath
> on top devices that set BLK_MQ_F_BLOCKING.

Indeed.

Mike, please feel free to fold the following patches into your next
post:

https://lore.kernel.org/linux-block/YcP4FMG9an5ReIiV@T590/#r


Thanks, 
Ming