public inbox for linux-scsi@vger.kernel.org
 help / color / mirror / Atom feed
From: John Garry <john.garry@huawei.com>
To: <jejb@linux.ibm.com>, <martin.petersen@oracle.com>,
	<artur.paszkiewicz@intel.com>, <jinpu.wang@cloud.ionos.com>,
	<damien.lemoal@opensource.wdc.com>
Cc: <chenxiang66@hisilicon.com>, <linux-scsi@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>, <linuxarm@huawei.com>,
	<hch@lst.de>, "John Garry" <john.garry@huawei.com>
Subject: [PATCH 1/2] scsi: isci: Drop SAS_TASK_AT_INITIATOR check in isci_task_abort_task()
Date: Thu, 10 Feb 2022 18:43:23 +0800	[thread overview]
Message-ID: <1644489804-85730-2-git-send-email-john.garry@huawei.com> (raw)
In-Reply-To: <1644489804-85730-1-git-send-email-john.garry@huawei.com>

In the queue path, move around when we assign sas_task->lldd_task such
that this pointer and the SAS_TASK_AT_INITIATOR flag are set atomically.
It is also not required to clear SAS_TASK_AT_INITIATOR in
isci_task_execute_task() error path as it is also cleared immediately
after in isci_task_refuse() call.

Now the following items may be considered:
- SAS_TASK_STATE_DONE and SAS_TASK_AT_INITIATOR are mutually exclusive
  apart from possibly when SAS_TASK_STATE_DONE is set in
  sas_scsi_find_task(), but that is after .lldd_abort_task, i.e. the
  considered callback, is called.
- If isci_task_refuse() is called in the queue path, then
  sas_task->lldd_task and SAS_TASK_AT_INITIATOR are cleared atomically
  in isci_task_refuse().
- In the completion path, SAS_TASK_STATE_DONE is set and
  SAS_TASK_AT_INITIATOR is cleared atomically before the sas_task.lldd_task
  is cleared later.

So in isci_task_abort_task() if SAS_TASK_STATE_DONE is not set and
sas_task.lldd_task is still set, then SAS_TASK_AT_INITIATOR must be set -
so we can drop this check on SAS_TASK_AT_INITIATOR.

Signed-off-by: John Garry <john.garry@huawei.com>
---
 drivers/scsi/isci/request.c | 12 ++++--------
 drivers/scsi/isci/request.h |  5 ++++-
 drivers/scsi/isci/task.c    | 13 ++++++-------
 3 files changed, 14 insertions(+), 16 deletions(-)

diff --git a/drivers/scsi/isci/request.c b/drivers/scsi/isci/request.c
index fcaa84a3c210..b48ec64f745a 100644
--- a/drivers/scsi/isci/request.c
+++ b/drivers/scsi/isci/request.c
@@ -3406,9 +3406,9 @@ static struct isci_request *isci_request_from_tag(struct isci_host *ihost, u16 t
 	return ireq;
 }
 
-static struct isci_request *isci_io_request_from_tag(struct isci_host *ihost,
-						     struct sas_task *task,
-						     u16 tag)
+struct isci_request *isci_io_request_from_tag(struct isci_host *ihost,
+					      struct sas_task *task,
+					      u16 tag)
 {
 	struct isci_request *ireq;
 
@@ -3434,16 +3434,12 @@ struct isci_request *isci_tmf_request_from_tag(struct isci_host *ihost,
 }
 
 int isci_request_execute(struct isci_host *ihost, struct isci_remote_device *idev,
-			 struct sas_task *task, u16 tag)
+			 struct sas_task *task, struct isci_request * ireq)
 {
 	enum sci_status status;
-	struct isci_request *ireq;
 	unsigned long flags;
 	int ret = 0;
 
-	/* do common allocation and init of request object. */
-	ireq = isci_io_request_from_tag(ihost, task, tag);
-
 	status = isci_io_request_build(ihost, ireq, idev);
 	if (status != SCI_SUCCESS) {
 		dev_dbg(&ihost->pdev->dev,
diff --git a/drivers/scsi/isci/request.h b/drivers/scsi/isci/request.h
index aff95317fcf4..40b71b3fd03e 100644
--- a/drivers/scsi/isci/request.h
+++ b/drivers/scsi/isci/request.h
@@ -291,7 +291,10 @@ struct isci_request *isci_tmf_request_from_tag(struct isci_host *ihost,
 					       struct isci_tmf *isci_tmf,
 					       u16 tag);
 int isci_request_execute(struct isci_host *ihost, struct isci_remote_device *idev,
-			 struct sas_task *task, u16 tag);
+			 struct sas_task *task, struct isci_request * ireq);
+struct isci_request *isci_io_request_from_tag(struct isci_host *ihost,
+					      struct sas_task *task,
+					      u16 tag);
 enum sci_status
 sci_task_request_construct(struct isci_host *ihost,
 			    struct isci_remote_device *idev,
diff --git a/drivers/scsi/isci/task.c b/drivers/scsi/isci/task.c
index 3fd88d72a0c0..14738702d4c9 100644
--- a/drivers/scsi/isci/task.c
+++ b/drivers/scsi/isci/task.c
@@ -162,18 +162,18 @@ int isci_task_execute_task(struct sas_task *task, gfp_t gfp_flags)
 					 SAS_TASK_UNDELIVERED,
 					 SAS_SAM_STAT_TASK_ABORTED);
 		} else {
+			struct isci_request *ireq;
+
 			task->task_state_flags |= SAS_TASK_AT_INITIATOR;
+			/* do common allocation and init of request object. */
+			ireq = isci_io_request_from_tag(ihost, task, tag);
 			spin_unlock_irqrestore(&task->task_state_lock, flags);
 
 			/* build and send the request. */
-			status = isci_request_execute(ihost, idev, task, tag);
+			/* do common allocation and init of request object. */
+			status = isci_request_execute(ihost, idev, task, ireq);
 
 			if (status != SCI_SUCCESS) {
-				spin_lock_irqsave(&task->task_state_lock, flags);
-				/* Did not really start this command. */
-				task->task_state_flags &= ~SAS_TASK_AT_INITIATOR;
-				spin_unlock_irqrestore(&task->task_state_lock, flags);
-
 				if (test_bit(IDEV_GONE, &idev->flags)) {
 					/* Indicate that the device
 					 * is gone.
@@ -498,7 +498,6 @@ int isci_task_abort_task(struct sas_task *task)
 
 	/* If task is already done, the request isn't valid */
 	if (!(task->task_state_flags & SAS_TASK_STATE_DONE) &&
-	    (task->task_state_flags & SAS_TASK_AT_INITIATOR) &&
 	    old_request) {
 		idev = isci_get_device(task->dev->lldd_dev);
 		target_done_already = test_bit(IREQ_COMPLETE_IN_TARGET,
-- 
2.26.2


  reply	other threads:[~2022-02-10 10:48 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-02-10 10:43 [PATCH 0/2] scsi: libsas: Drop SAS_TASK_AT_INITIATOR John Garry
2022-02-10 10:43 ` John Garry [this message]
2022-02-10 10:43 ` [PATCH 2/2] " John Garry
2022-02-10 11:46   ` Damien Le Moal
2022-02-11 21:57 ` [PATCH 0/2] " Martin K. Petersen
2022-02-15  3:19 ` Martin K. Petersen

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=1644489804-85730-2-git-send-email-john.garry@huawei.com \
    --to=john.garry@huawei.com \
    --cc=artur.paszkiewicz@intel.com \
    --cc=chenxiang66@hisilicon.com \
    --cc=damien.lemoal@opensource.wdc.com \
    --cc=hch@lst.de \
    --cc=jejb@linux.ibm.com \
    --cc=jinpu.wang@cloud.ionos.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=linuxarm@huawei.com \
    --cc=martin.petersen@oracle.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox