All of lore.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 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.