All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mike Christie <michael.christie@oracle.com>
To: lduncan@suse.com, cleech@redhat.com, njavali@marvell.com,
	mrangankar@marvell.com, GR-QLogic-Storage-Upstream@marvell.com,
	martin.petersen@oracle.com, linux-scsi@vger.kernel.org,
	jejb@linux.ibm.com
Cc: Mike Christie <michael.christie@oracle.com>
Subject: [PATCH 12/13] scsi: iscsi: Try to avoid taking back_lock in xmit path
Date: Tue, 17 May 2022 17:24:47 -0500	[thread overview]
Message-ID: <20220517222448.25612-13-michael.christie@oracle.com> (raw)
In-Reply-To: <20220517222448.25612-1-michael.christie@oracle.com>

We need the back lock when freeing a task, so we hold it when calling
__iscsi_put_task from the completion path to make it easier and to avoid
having to retake it in that path. For iscsi_put_task we just grabbed it
while also doing the decrement on the refcount but it's only really needed
if the refcount is zero and we free the task. This modifies iscsi_put_task
to just take the lock when needed then has the xmit path use it. Normally
we will then not take the back lock from the xmit path. It will only be
rare cases where the network is so fast that we get a response right after
we send the header/data.

Reviewed-by: Lee Duncan <lduncan@suse.com>
Signed-off-by: Mike Christie <michael.christie@oracle.com>
---
 drivers/scsi/libiscsi.c | 30 ++++++++++++++----------------
 1 file changed, 14 insertions(+), 16 deletions(-)

diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c
index dee6e2d5c86e..2bba10464cfa 100644
--- a/drivers/scsi/libiscsi.c
+++ b/drivers/scsi/libiscsi.c
@@ -490,6 +490,12 @@ bool iscsi_get_task(struct iscsi_task *task)
 }
 EXPORT_SYMBOL_GPL(iscsi_get_task);
 
+/**
+ * __iscsi_put_task - drop the refcount on a task
+ * @task: iscsi_task to drop the refcount on
+ *
+ * The back_lock must be held when calling in case it frees the task.
+ */
 void __iscsi_put_task(struct iscsi_task *task)
 {
 	if (refcount_dec_and_test(&task->refcount))
@@ -501,10 +507,11 @@ void iscsi_put_task(struct iscsi_task *task)
 {
 	struct iscsi_session *session = task->conn->session;
 
-	/* regular RX path uses back_lock */
-	spin_lock_bh(&session->back_lock);
-	__iscsi_put_task(task);
-	spin_unlock_bh(&session->back_lock);
+	if (refcount_dec_and_test(&task->refcount)) {
+		spin_lock_bh(&session->back_lock);
+		iscsi_free_task(task);
+		spin_unlock_bh(&session->back_lock);
+	}
 }
 EXPORT_SYMBOL_GPL(iscsi_put_task);
 
@@ -1454,8 +1461,6 @@ static int iscsi_xmit_task(struct iscsi_conn *conn, struct iscsi_task *task,
 {
 	int rc;
 
-	spin_lock_bh(&conn->session->back_lock);
-
 	if (!conn->task) {
 		/*
 		 * Take a ref so we can access it after xmit_task().
@@ -1464,7 +1469,6 @@ static int iscsi_xmit_task(struct iscsi_conn *conn, struct iscsi_task *task,
 		 * stopped the xmit thread. WARN on move on.
 		 */
 		if (!iscsi_get_task(task)) {
-			spin_unlock_bh(&conn->session->back_lock);
 			WARN_ON_ONCE(1);
 			return 0;
 		}
@@ -1478,7 +1482,7 @@ static int iscsi_xmit_task(struct iscsi_conn *conn, struct iscsi_task *task,
 	 * case a bad target sends a cmd rsp before we have handled the task.
 	 */
 	if (was_requeue)
-		__iscsi_put_task(task);
+		iscsi_put_task(task);
 
 	/*
 	 * Do this after dropping the extra ref because if this was a requeue
@@ -1490,10 +1494,8 @@ static int iscsi_xmit_task(struct iscsi_conn *conn, struct iscsi_task *task,
 		 * task and get woken up again.
 		 */
 		conn->task = task;
-		spin_unlock_bh(&conn->session->back_lock);
 		return -ENODATA;
 	}
-	spin_unlock_bh(&conn->session->back_lock);
 
 	spin_unlock_bh(&conn->session->frwd_lock);
 	rc = conn->session->tt->xmit_task(task);
@@ -1501,10 +1503,7 @@ static int iscsi_xmit_task(struct iscsi_conn *conn, struct iscsi_task *task,
 	if (!rc) {
 		/* done with this task */
 		task->last_xfer = jiffies;
-	}
-	/* regular RX path uses back_lock */
-	spin_lock(&conn->session->back_lock);
-	if (rc) {
+	} else {
 		/*
 		 * get an extra ref that is released next time we access it
 		 * as conn->task above.
@@ -1513,8 +1512,7 @@ static int iscsi_xmit_task(struct iscsi_conn *conn, struct iscsi_task *task,
 		conn->task = task;
 	}
 
-	__iscsi_put_task(task);
-	spin_unlock(&conn->session->back_lock);
+	iscsi_put_task(task);
 	return rc;
 }
 
-- 
2.25.1


  parent reply	other threads:[~2022-05-17 22:26 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-17 22:24 [PATCH 00/13] iscsi fixes, perf improvements and cleanups Mike Christie
2022-05-17 22:24 ` [PATCH 01/13] scsi: iscsi: Fix HW conn removal use after free Mike Christie
2022-05-18 15:44   ` Lee Duncan
2022-05-17 22:24 ` [PATCH 02/13] scsi: iscsi: Add helper to remove a session from the kernel Mike Christie
2022-05-18 16:01   ` Lee Duncan
2022-05-17 22:24 ` [PATCH 03/13] scsi: qedi: Use QEDI_MODE_NORMAL for error handling Mike Christie
2022-05-18 16:04   ` Lee Duncan
2022-05-17 22:24 ` [PATCH 04/13] scsi: iscsi: Fix session removal on shutdown Mike Christie
2022-05-18 21:42   ` Lee Duncan
2022-05-17 22:24 ` [PATCH 05/13] scsi: iscsi: Rename iscsi_conn_queue_work Mike Christie
2022-05-17 22:24 ` [PATCH 06/13] scsi: iscsi: Add recv workqueue helpers Mike Christie
2022-05-17 22:24 ` [PATCH 07/13] scsi: iscsi: Run recv path from workqueue Mike Christie
2022-05-17 22:24 ` [PATCH 08/13] scsi: iscsi_tcp: Tell net when there's more data Mike Christie
2022-05-17 22:24 ` [PATCH 09/13] scsi: iscsi_tcp: Drop target_alloc use Mike Christie
2022-05-17 22:24 ` [PATCH 10/13] scsi: iscsi: remove unneeded task state check Mike Christie
2022-05-17 22:24 ` [PATCH 11/13] scsi: iscsi: Remove iscsi_get_task back_lock requirement Mike Christie
2022-05-17 22:24 ` Mike Christie [this message]
2022-05-17 22:24 ` [PATCH 13/13] scsi: libiscsi: improve conn_send_pdu API Mike Christie

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220517222448.25612-13-michael.christie@oracle.com \
    --to=michael.christie@oracle.com \
    --cc=GR-QLogic-Storage-Upstream@marvell.com \
    --cc=cleech@redhat.com \
    --cc=jejb@linux.ibm.com \
    --cc=lduncan@suse.com \
    --cc=linux-scsi@vger.kernel.org \
    --cc=martin.petersen@oracle.com \
    --cc=mrangankar@marvell.com \
    --cc=njavali@marvell.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.