MFC r274843, r274845:

Move icl_pdu_get_data() and xpt_done() out of initiator's session lock.

During heavy reads data copying in icl_pdu_get_data() may consume large
percent of CPU time.  Moving it out of the lock significantly reduces
lock hold time and respectively lock congestion on read operations.

1 files changed, 69 insertions, 52 deletions

diff --git a/sys/dev/iscsi/iscsi.c b/sys/dev/iscsi/iscsi.c
index 86ba82b..415680f 100644
--- a/sys/dev/iscsi/iscsi.c
+++ b/sys/dev/iscsi/iscsi.c
@@ -144,6 +144,7 @@ static uma_zone_t iscsi_outstanding_zone;
 #define ISCSI_SESSION_LOCK(X)		mtx_lock(&X->is_lock)
 #define ISCSI_SESSION_UNLOCK(X)		mtx_unlock(&X->is_lock)
 #define ISCSI_SESSION_LOCK_ASSERT(X)	mtx_assert(&X->is_lock, MA_OWNED)
+#define ISCSI_SESSION_LOCK_ASSERT_NOT(X) mtx_assert(&X->is_lock, MA_NOTOWNED)
 
 static int	iscsi_ioctl(struct cdev *dev, u_long cmd, caddr_t arg,
 		    int mode, struct thread *td);
@@ -714,37 +715,46 @@ iscsi_receive_callback(struct icl_pdu *response)
 	switch (response->ip_bhs->bhs_opcode) {
 	case ISCSI_BHS_OPCODE_NOP_IN:
 		iscsi_pdu_handle_nop_in(response);
+		ISCSI_SESSION_UNLOCK(is);
 		break;
 	case ISCSI_BHS_OPCODE_SCSI_RESPONSE:
 		iscsi_pdu_handle_scsi_response(response);
+		/* Session lock dropped inside. */
+		ISCSI_SESSION_LOCK_ASSERT_NOT(is);
 		break;
 	case ISCSI_BHS_OPCODE_TASK_RESPONSE:
 		iscsi_pdu_handle_task_response(response);
+		ISCSI_SESSION_UNLOCK(is);
 		break;
 	case ISCSI_BHS_OPCODE_SCSI_DATA_IN:
 		iscsi_pdu_handle_data_in(response);
+		/* Session lock dropped inside. */
+		ISCSI_SESSION_LOCK_ASSERT_NOT(is);
 		break;
 	case ISCSI_BHS_OPCODE_LOGOUT_RESPONSE:
 		iscsi_pdu_handle_logout_response(response);
+		ISCSI_SESSION_UNLOCK(is);
 		break;
 	case ISCSI_BHS_OPCODE_R2T:
 		iscsi_pdu_handle_r2t(response);
+		ISCSI_SESSION_UNLOCK(is);
 		break;
 	case ISCSI_BHS_OPCODE_ASYNC_MESSAGE:
 		iscsi_pdu_handle_async_message(response);
+		ISCSI_SESSION_UNLOCK(is);
 		break;
 	case ISCSI_BHS_OPCODE_REJECT:
 		iscsi_pdu_handle_reject(response);
+		ISCSI_SESSION_UNLOCK(is);
 		break;
 	default:
 		ISCSI_SESSION_WARN(is, "received PDU with unsupported "
 		    "opcode 0x%x; reconnecting",
 		    response->ip_bhs->bhs_opcode);
 		iscsi_session_reconnect(is);
+		ISCSI_SESSION_UNLOCK(is);
 		icl_pdu_free(response);
 	}
-
-	ISCSI_SESSION_UNLOCK(is);
 }
 
 static void
@@ -833,8 +843,9 @@ iscsi_pdu_handle_scsi_response(struct icl_pdu *response)
 	struct iscsi_bhs_scsi_response *bhssr;
 	struct iscsi_outstanding *io;
 	struct iscsi_session *is;
+	union ccb *ccb;
 	struct ccb_scsiio *csio;
-	size_t data_segment_len;
+	size_t data_segment_len, received;
 	uint16_t sense_len;
 
 	is = PDU_SESSION(response);
@@ -845,46 +856,44 @@ iscsi_pdu_handle_scsi_response(struct icl_pdu *response)
 		ISCSI_SESSION_WARN(is, "bad itt 0x%x", bhssr->bhssr_initiator_task_tag);
 		icl_pdu_free(response);
 		iscsi_session_reconnect(is);
+		ISCSI_SESSION_UNLOCK(is);
 		return;
 	}
 
+	ccb = io->io_ccb;
+	received = io->io_received;
+	iscsi_outstanding_remove(is, io);
+	ISCSI_SESSION_UNLOCK(is);
+
 	if (bhssr->bhssr_response != BHSSR_RESPONSE_COMMAND_COMPLETED) {
 		ISCSI_SESSION_WARN(is, "service response 0x%x", bhssr->bhssr_response);
- 		if ((io->io_ccb->ccb_h.status & CAM_DEV_QFRZN) == 0) {
- 			xpt_freeze_devq(io->io_ccb->ccb_h.path, 1);
+ 		if ((ccb->ccb_h.status & CAM_DEV_QFRZN) == 0) {
+ 			xpt_freeze_devq(ccb->ccb_h.path, 1);
 			ISCSI_SESSION_DEBUG(is, "freezing devq");
 		}
- 		io->io_ccb->ccb_h.status = CAM_REQ_CMP_ERR | CAM_DEV_QFRZN;
+ 		ccb->ccb_h.status = CAM_REQ_CMP_ERR | CAM_DEV_QFRZN;
 	} else if (bhssr->bhssr_status == 0) {
-		io->io_ccb->ccb_h.status = CAM_REQ_CMP;
+		ccb->ccb_h.status = CAM_REQ_CMP;
 	} else {
- 		if ((io->io_ccb->ccb_h.status & CAM_DEV_QFRZN) == 0) {
- 			xpt_freeze_devq(io->io_ccb->ccb_h.path, 1);
+ 		if ((ccb->ccb_h.status & CAM_DEV_QFRZN) == 0) {
+ 			xpt_freeze_devq(ccb->ccb_h.path, 1);
 			ISCSI_SESSION_DEBUG(is, "freezing devq");
 		}
- 		io->io_ccb->ccb_h.status = CAM_SCSI_STATUS_ERROR | CAM_DEV_QFRZN;
-		io->io_ccb->csio.scsi_status = bhssr->bhssr_status;
+ 		ccb->ccb_h.status = CAM_SCSI_STATUS_ERROR | CAM_DEV_QFRZN;
+		ccb->csio.scsi_status = bhssr->bhssr_status;
 	}
 
-	if (bhssr->bhssr_flags & BHSSR_FLAGS_RESIDUAL_OVERFLOW) {
-		ISCSI_SESSION_WARN(is, "target indicated residual overflow");
-		icl_pdu_free(response);
-		iscsi_session_reconnect(is);
-		return;
-	}
-
-	csio = &io->io_ccb->csio;
-
+	csio = &ccb->csio;
 	data_segment_len = icl_pdu_data_segment_length(response);
 	if (data_segment_len > 0) {
 		if (data_segment_len < sizeof(sense_len)) {
 			ISCSI_SESSION_WARN(is, "truncated data segment (%zd bytes)",
 			    data_segment_len);
-			if ((io->io_ccb->ccb_h.status & CAM_DEV_QFRZN) == 0) {
-				xpt_freeze_devq(io->io_ccb->ccb_h.path, 1);
+			if ((ccb->ccb_h.status & CAM_DEV_QFRZN) == 0) {
+				xpt_freeze_devq(ccb->ccb_h.path, 1);
 				ISCSI_SESSION_DEBUG(is, "freezing devq");
 			}
-			io->io_ccb->ccb_h.status = CAM_REQ_CMP_ERR | CAM_DEV_QFRZN;
+			ccb->ccb_h.status = CAM_REQ_CMP_ERR | CAM_DEV_QFRZN;
 			goto out;
 		}
 		icl_pdu_get_data(response, 0, &sense_len, sizeof(sense_len));
@@ -897,11 +906,11 @@ iscsi_pdu_handle_scsi_response(struct icl_pdu *response)
 			ISCSI_SESSION_WARN(is, "truncated data segment "
 			    "(%zd bytes, should be %zd)",
 			    data_segment_len, sizeof(sense_len) + sense_len);
-			if ((io->io_ccb->ccb_h.status & CAM_DEV_QFRZN) == 0) {
-				xpt_freeze_devq(io->io_ccb->ccb_h.path, 1);
+			if ((ccb->ccb_h.status & CAM_DEV_QFRZN) == 0) {
+				xpt_freeze_devq(ccb->ccb_h.path, 1);
 				ISCSI_SESSION_DEBUG(is, "freezing devq");
 			}
-			io->io_ccb->ccb_h.status = CAM_REQ_CMP_ERR | CAM_DEV_QFRZN;
+			ccb->ccb_h.status = CAM_REQ_CMP_ERR | CAM_DEV_QFRZN;
 			goto out;
 		} else if (sizeof(sense_len) + sense_len < data_segment_len)
 			ISCSI_SESSION_WARN(is, "oversize data segment "
@@ -914,7 +923,7 @@ iscsi_pdu_handle_scsi_response(struct icl_pdu *response)
 		}
 		icl_pdu_get_data(response, sizeof(sense_len), &csio->sense_data, sense_len);
 		csio->sense_resid = csio->sense_len - sense_len;
-		io->io_ccb->ccb_h.status |= CAM_AUTOSNS_VALID;
+		ccb->ccb_h.status |= CAM_AUTOSNS_VALID;
 	}
 
 out:
@@ -922,21 +931,19 @@ out:
 		csio->resid = ntohl(bhssr->bhssr_residual_count);
 
 	if ((csio->ccb_h.flags & CAM_DIR_MASK) == CAM_DIR_IN) {
-		KASSERT(io->io_received <= csio->dxfer_len,
-		    ("io->io_received > csio->dxfer_len"));
-		if (io->io_received < csio->dxfer_len) {
-			if (csio->resid != csio->dxfer_len - io->io_received) {
+		KASSERT(received <= csio->dxfer_len,
+		    ("received > csio->dxfer_len"));
+		if (received < csio->dxfer_len) {
+			if (csio->resid != csio->dxfer_len - received) {
 				ISCSI_SESSION_WARN(is, "underflow mismatch: "
 				    "target indicates %d, we calculated %zd",
-				    csio->resid,
-				    csio->dxfer_len - io->io_received);
+				    csio->resid, csio->dxfer_len - received);
 			}
-			csio->resid = csio->dxfer_len - io->io_received;
+			csio->resid = csio->dxfer_len - received;
 		}
 	}
 
-	xpt_done(io->io_ccb);
-	iscsi_outstanding_remove(is, io);
+	xpt_done(ccb);
 	icl_pdu_free(response);
 }
 
@@ -978,8 +985,9 @@ iscsi_pdu_handle_data_in(struct icl_pdu *response)
 	struct iscsi_bhs_data_in *bhsdi;
 	struct iscsi_outstanding *io;
 	struct iscsi_session *is;
+	union ccb *ccb;
 	struct ccb_scsiio *csio;
-	size_t data_segment_len;
+	size_t data_segment_len, received, oreceived;
 	
 	is = PDU_SESSION(response);
 	bhsdi = (struct iscsi_bhs_data_in *)response->ip_bhs;
@@ -988,6 +996,7 @@ iscsi_pdu_handle_data_in(struct icl_pdu *response)
 		ISCSI_SESSION_WARN(is, "bad itt 0x%x", bhsdi->bhsdi_initiator_task_tag);
 		icl_pdu_free(response);
 		iscsi_session_reconnect(is);
+		ISCSI_SESSION_UNLOCK(is);
 		return;
 	}
 
@@ -998,6 +1007,7 @@ iscsi_pdu_handle_data_in(struct icl_pdu *response)
 		 * but initiators and targets MUST be able to properly receive
 		 * 0 length data segments."
 		 */
+		ISCSI_SESSION_UNLOCK(is);
 		icl_pdu_free(response);
 		return;
 	}
@@ -1012,10 +1022,12 @@ iscsi_pdu_handle_data_in(struct icl_pdu *response)
 		    io->io_received, (size_t)ntohl(bhsdi->bhsdi_buffer_offset));
 		icl_pdu_free(response);
 		iscsi_session_reconnect(is);
+		ISCSI_SESSION_UNLOCK(is);
 		return;
 	}
 
-	csio = &io->io_ccb->csio;
+	ccb = io->io_ccb;
+	csio = &ccb->csio;
 
 	if (io->io_received + data_segment_len > csio->dxfer_len) {
 		ISCSI_SESSION_WARN(is, "oversize data segment (%zd bytes "
@@ -1023,11 +1035,18 @@ iscsi_pdu_handle_data_in(struct icl_pdu *response)
 		    data_segment_len, io->io_received, csio->dxfer_len);
 		icl_pdu_free(response);
 		iscsi_session_reconnect(is);
+		ISCSI_SESSION_UNLOCK(is);
 		return;
 	}
 
-	icl_pdu_get_data(response, 0, csio->data_ptr + io->io_received, data_segment_len);
+	oreceived = io->io_received;
 	io->io_received += data_segment_len;
+	received = io->io_received;
+	if ((bhsdi->bhsdi_flags & BHSDI_FLAGS_S) != 0)
+		iscsi_outstanding_remove(is, io);
+	ISCSI_SESSION_UNLOCK(is);
+
+	icl_pdu_get_data(response, 0, csio->data_ptr + oreceived, data_segment_len);
 
 	/*
 	 * XXX: Check DataSN.
@@ -1043,33 +1062,31 @@ iscsi_pdu_handle_data_in(struct icl_pdu *response)
 
 	//ISCSI_SESSION_DEBUG(is, "got S flag; status 0x%x", bhsdi->bhsdi_status);
 	if (bhsdi->bhsdi_status == 0) {
-		io->io_ccb->ccb_h.status = CAM_REQ_CMP;
+		ccb->ccb_h.status = CAM_REQ_CMP;
 	} else {
-		if ((io->io_ccb->ccb_h.status & CAM_DEV_QFRZN) == 0) {
-			xpt_freeze_devq(io->io_ccb->ccb_h.path, 1);
+		if ((ccb->ccb_h.status & CAM_DEV_QFRZN) == 0) {
+			xpt_freeze_devq(ccb->ccb_h.path, 1);
 			ISCSI_SESSION_DEBUG(is, "freezing devq");
 		}
-		io->io_ccb->ccb_h.status = CAM_SCSI_STATUS_ERROR | CAM_DEV_QFRZN;
+		ccb->ccb_h.status = CAM_SCSI_STATUS_ERROR | CAM_DEV_QFRZN;
 		csio->scsi_status = bhsdi->bhsdi_status;
 	}
 
 	if ((csio->ccb_h.flags & CAM_DIR_MASK) == CAM_DIR_IN) {
-		KASSERT(io->io_received <= csio->dxfer_len,
-		    ("io->io_received > csio->dxfer_len"));
-		if (io->io_received < csio->dxfer_len) {
+		KASSERT(received <= csio->dxfer_len,
+		    ("received > csio->dxfer_len"));
+		if (received < csio->dxfer_len) {
 			csio->resid = ntohl(bhsdi->bhsdi_residual_count);
-			if (csio->resid != csio->dxfer_len - io->io_received) {
+			if (csio->resid != csio->dxfer_len - received) {
 				ISCSI_SESSION_WARN(is, "underflow mismatch: "
 				    "target indicates %d, we calculated %zd",
-				    csio->resid,
-				    csio->dxfer_len - io->io_received);
+				    csio->resid, csio->dxfer_len - received);
 			}
-			csio->resid = csio->dxfer_len - io->io_received;
+			csio->resid = csio->dxfer_len - received;
 		}
 	}
 
-	xpt_done(io->io_ccb);
-	iscsi_outstanding_remove(is, io);
+	xpt_done(ccb);
 	icl_pdu_free(response);
 }