* [PATCH 01/11] isci: Lookup device references through requests in completions.
2011-10-27 22:04 [PATCH 00/11] isci fixes: hotplug Dan Williams
@ 2011-10-27 22:04 ` Dan Williams
2011-10-27 22:04 ` [PATCH 02/11] isci: Immediately fail I/O to removed devices Dan Williams
` (9 subsequent siblings)
10 siblings, 0 replies; 13+ messages in thread
From: Dan Williams @ 2011-10-27 22:04 UTC (permalink / raw)
To: linux-scsi; +Cc: Jeff Skirvin
From: Jeff Skirvin <jeffrey.d.skirvin@intel.com>
The LLDD needs to obtain a reference to the device through the request
itself and not through the domain_device, because the
domain_device.lldd_dev is set to NULL early in the lldd_dev_gone call.
This relies on the fact that the isci_remote_device object is keeping a
seperate reference count of outstanding requests. TODO: unify the
request count tracking with the isci_remote_device kref.
The failure signature of this condition looks like the following
log, where the important bits are the call to lldd_dev_gone followed
by a crash in isci_terminate_request_core:
[ 229.151541] isci 0000:0b:00.0: isci_remote_device_gone: domain_device = ffff8801492d4800, isci_device = ffff880143c657d0, isci_port = ffff880143c63658
[ 229.166007] isci 0000:0b:00.0: isci_remote_device_stop: isci_device = ffff880143c657d0
[ 229.175317] isci 0000:0b:00.0: isci_terminate_pending_requests: idev=ffff880143c657d0 request=ffff88014741f000; task=ffff8801470f46c0 old_state=2
[ 229.189702] isci 0000:0b:00.0: isci_terminate_request_core: device = ffff880143c657d0; request = ffff88014741f000
[ 229.201339] isci 0000:0b:00.0: isci_terminate_request_core: before completion wait (ffff88014741f000/ffff880149715ad0)
[ 229.213414] isci 0000:0b:00.0: sci_controller_process_completions: completion queue entry:0x8000a0e9
[ 229.214401] BUG: unable to handle kernel NULL pointer dereference at 0000000000000228
[ 229.214401] IP:jdskirvi-testlbo [<ffffffffa00a58be>] sci_request_completed_state_enter+0x50/0xafb [isci]
[ 229.214401] PGD 13d19e067 PUD 13d104067 PMD 0
[ 229.214401] Oops: 0000 [#1] SMP
[ 229.214401] CPU 0 x kernel: [ 226
[ 229.214401] Modules linked in: ipv6 dm_multipath uinput nouveau snd_hda_codec_realtek snd_hda_intel ttm drm_kms_helper drm snd_hda_codec snd_hwdep snd_pcm snd_timer i2c_algo_bit isci snd libsas ioatdma mxm_wmi iTCO_wdt soundcore snd_page_alloc scsi_transport_sas iTCO_vendor_support wmi dca video i2c_i801 i2c_core [last unloaded: speedstep_lib]
[ 229.214401]
[ 229.214401] Pid: 5, comm: kworker/u:0 Not tainted 3.0.0-isci-11.7.29+ #30.353196] Buffer Intel Corporation Stoakley/Pearlcity Workstation
[ 229.214401] RIP: 0010:[<ffffffffa00a58be>] I/O error on dev [<ffffffffa00a58be>] sci_request_completed_state_enter+0x50/0xafb [isci]
[ 229.214401] RSP: 0018:ffff88014fc03d20 EFLAGS: 00010046
[ 229.214401] RAX: 0000000000000000 RBX: ffff88014741f000 RCX: 0000000000000000
[ 229.214401] RDX: ffffffffa00b2c90 RSI: 0000000000000017 RDI: ffff88014741f0a0
[ 229.214401] RBP: ffff88014fc03d90 R08: 0000000000000018 R09: 0000000000000000
[ 229.214401] R10: 0000000000000000 R11: ffffffff81a17d98 R12: 000000000000001d
[ 229.214401] R13: ffff8801470f46c0 R14: 0000000000000000 R15: 0000000000008000
[ 229.214401] FS: 0000000000000000(0000) GS:ffff88014fc00000(0000) knlGS:0000000000000000
[ 229.214401] CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
[ 229.214401] CR2: 0000000000000228 CR3: 000000013ceaa000 CR4: 00000000000406f0
[ 229.214401] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[ 229.214401] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
[ 229.214401] Process kworker/u:0 (pid: 5, threadinfo ffff880149714000, task ffff880149718000)
[ 229.214401] Call Trace:
[ 229.214401] <IRQ>
[ 229.214401] [<ffffffffa00aa6ce>] sci_change_state+0x4a/0x4f [isci]
[ 229.214401] [<ffffffffa00a4ca6>] sci_io_request_tc_completion+0x79c/0x7a0 [isci]
[ 229.214401] [<ffffffffa00acf35>] sci_controller_process_completions+0x14f/0x396 [isci]
[ 229.214401] [<ffffffffa00abbda>] ? spin_lock_irq+0xe/0x10 [isci]
[ 229.214401] [<ffffffffa00ad2cf>] isci_host_completion_routine+0x71/0x2be [isci]
[ 229.214401] [<ffffffff8107c6b3>] ? mark_held_locks+0x52/0x70
[ 229.214401] [<ffffffff810538e8>] tasklet_action+0x90/0xf1
[ 229.214401] [<ffffffff81054050>] __do_softirq+0xe5/0x1bf
[ 229.214401] [<ffffffff8106d9d1>] ? hrtimer_interrupt+0x129/0x1bb
[ 229.214401] [<ffffffff814ff69c>] call_softirq+0x1c/0x30
[ 229.214401] [<ffffffff8100bb67>] do_softirq+0x4b/0xa3
[ 229.214401] [<ffffffff81053d84>] irq_exit+0x53/0xb4
[ 229.214401] [<ffffffff814fffe7>] smp_apic_timer_interrupt+0x83/0x91
[ 229.214401] [<ffffffff814fee53>] apic_timer_interrupt+0x13/0x20
[ 229.214401] <EOI>
[ 229.214401] [<ffffffff814f7ad4>] ? retint_restore_args+0x13/0x13
[ 229.214401] [<ffffffff8107af29>] ? trace_hardirqs_off+0xd/0xf
[ 229.214401] [<ffffffff8104ea71>] ? vprintk+0x40b/0x452
[ 229.214401] [<ffffffff814f4b5a>] printk+0x41/0x47
[ 229.214401] [<ffffffff81314484>] __dev_printk+0x78/0x7a
[ 229.214401] [<ffffffff8131471e>] dev_printk+0x45/0x47
[ 229.214401] [<ffffffffa00ae2a3>] isci_terminate_request_core+0x15d/0x317 [isci]
[ 229.214401] [<ffffffffa00af1ad>] isci_terminate_pending_requests+0x1a4/0x204 [isci]
[ 229.214401] [<ffffffffa00229f6>] ? sas_phye_oob_error+0xc3/0xc3 [libsas]
[ 229.214401] [<ffffffffa00a7d9e>] isci_remote_device_nuke_requests+0xa6/0xff [isci]
[ 229.214401] [<ffffffffa00a811a>] isci_remote_device_stop+0x7c/0x166 [isci]
[ 229.214401] [<ffffffffa00229f6>] ? sas_phye_oob_error+0xc3/0xc3 [libsas]
[ 229.214401] [<ffffffffa00a827a>] isci_remote_device_gone+0x76/0x7e [isci]
[ 229.214401] [<ffffffffa002363e>] sas_notify_lldd_dev_gone+0x34/0x36 [libsas]
[ 229.214401] [<ffffffffa0023945>] sas_unregister_dev+0x57/0x9c [libsas]
[ 229.214401] [<ffffffffa00239c0>] sas_unregister_domain_devices+0x36/0x65 [libsas]
[ 229.214401] [<ffffffffa0022cb8>] sas_deform_port+0x72/0x1ac [libsas]
[ 229.214401] [<ffffffffa00229f6>] ? sas_phye_oob_error+0xc3/0xc3 [libsas]
[ 229.214401] [<ffffffffa0022a34>] sas_phye_loss_of_signal+0x3e/0x42 [libsas]
Signed-off-by: Jeff Skirvin <jeffrey.d.skirvin@intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
drivers/scsi/isci/request.c | 7 +++----
1 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/scsi/isci/request.c b/drivers/scsi/isci/request.c
index 565a9f0..bfc7379 100644
--- a/drivers/scsi/isci/request.c
+++ b/drivers/scsi/isci/request.c
@@ -2728,9 +2728,9 @@ static void isci_request_io_request_complete(struct isci_host *ihost,
struct sas_task *task = isci_request_access_task(request);
struct ssp_response_iu *resp_iu;
unsigned long task_flags;
- struct isci_remote_device *idev = isci_lookup_device(task->dev);
- enum service_response response = SAS_TASK_UNDELIVERED;
- enum exec_status status = SAS_ABORTED_TASK;
+ struct isci_remote_device *idev = request->target_device;
+ enum service_response response = SAS_TASK_UNDELIVERED;
+ enum exec_status status = SAS_ABORTED_TASK;
enum isci_request_status request_status;
enum isci_completion_selection complete_to_host
= isci_perform_normal_io_completion;
@@ -3061,7 +3061,6 @@ static void isci_request_io_request_complete(struct isci_host *ihost,
/* complete the io request to the core. */
sci_controller_complete_io(ihost, request->target_device, request);
- isci_put_device(idev);
/* set terminated handle so it cannot be completed or
* terminated again, and to cause any calls into abort
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 02/11] isci: Immediately fail I/O to removed devices.
2011-10-27 22:04 [PATCH 00/11] isci fixes: hotplug Dan Williams
2011-10-27 22:04 ` [PATCH 01/11] isci: Lookup device references through requests in completions Dan Williams
@ 2011-10-27 22:04 ` Dan Williams
2011-10-27 22:05 ` [PATCH 03/11] isci: Fix tag leak in tasks and terminated requests Dan Williams
` (8 subsequent siblings)
10 siblings, 0 replies; 13+ messages in thread
From: Dan Williams @ 2011-10-27 22:04 UTC (permalink / raw)
To: linux-scsi; +Cc: Jeff Skirvin
From: Jeff Skirvin <jeffrey.d.skirvin@intel.com>
In the case where an I/O fails to start in isci_request_execute,
only allow retries if the device is not already gone.
Signed-off-by: Jeff Skirvin <jeffrey.d.skirvin@intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
drivers/scsi/isci/task.c | 31 +++++++++++++++++++++----------
1 files changed, 21 insertions(+), 10 deletions(-)
diff --git a/drivers/scsi/isci/task.c b/drivers/scsi/isci/task.c
index e2d9418..e8cf17b 100644
--- a/drivers/scsi/isci/task.c
+++ b/drivers/scsi/isci/task.c
@@ -212,16 +212,27 @@ int isci_task_execute_task(struct sas_task *task, int num, gfp_t gfp_flags)
task->task_state_flags &= ~SAS_TASK_AT_INITIATOR;
spin_unlock_irqrestore(&task->task_state_lock, flags);
- /* Indicate QUEUE_FULL so that the scsi
- * midlayer retries. if the request
- * failed for remote device reasons,
- * it gets returned as
- * SAS_TASK_UNDELIVERED next time
- * through.
- */
- isci_task_refuse(ihost, task,
- SAS_TASK_COMPLETE,
- SAS_QUEUE_FULL);
+ if (test_bit(IDEV_GONE, &idev->flags)) {
+
+ /* Indicate that the device
+ * is gone.
+ */
+ isci_task_refuse(ihost, task,
+ SAS_TASK_UNDELIVERED,
+ SAS_DEVICE_UNKNOWN);
+ } else {
+ /* Indicate QUEUE_FULL so that
+ * the scsi midlayer retries.
+ * If the request failed for
+ * remote device reasons, it
+ * gets returned as
+ * SAS_TASK_UNDELIVERED next
+ * time through.
+ */
+ isci_task_refuse(ihost, task,
+ SAS_TASK_COMPLETE,
+ SAS_QUEUE_FULL);
+ }
}
}
}
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 03/11] isci: Fix tag leak in tasks and terminated requests.
2011-10-27 22:04 [PATCH 00/11] isci fixes: hotplug Dan Williams
2011-10-27 22:04 ` [PATCH 01/11] isci: Lookup device references through requests in completions Dan Williams
2011-10-27 22:04 ` [PATCH 02/11] isci: Immediately fail I/O to removed devices Dan Williams
@ 2011-10-27 22:05 ` Dan Williams
2011-10-27 22:05 ` [PATCH 04/11] isci: Handle task request timeouts correctly Dan Williams
` (7 subsequent siblings)
10 siblings, 0 replies; 13+ messages in thread
From: Dan Williams @ 2011-10-27 22:05 UTC (permalink / raw)
To: linux-scsi; +Cc: Jeff Skirvin
From: Jeff Skirvin <jeffrey.d.skirvin@intel.com>
Make sure terminated requests and completed task tags are freed.
Signed-off-by: Jeff Skirvin <jeffrey.d.skirvin@intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
drivers/scsi/isci/task.c | 51 ++++++++++++++++++++++++++++------------------
1 files changed, 31 insertions(+), 20 deletions(-)
diff --git a/drivers/scsi/isci/task.c b/drivers/scsi/isci/task.c
index e8cf17b..7180b04 100644
--- a/drivers/scsi/isci/task.c
+++ b/drivers/scsi/isci/task.c
@@ -552,11 +552,27 @@ static void isci_request_cleanup_completed_loiterer(
if (isci_request != NULL) {
spin_lock_irqsave(&isci_host->scic_lock, flags);
+ isci_free_tag(isci_host, isci_request->io_tag);
+ isci_request_change_state(isci_request, unallocated);
list_del_init(&isci_request->dev_node);
spin_unlock_irqrestore(&isci_host->scic_lock, flags);
}
}
+static int isci_request_is_dealloc_managed(enum isci_request_status stat)
+{
+ switch (stat) {
+ case aborted:
+ case aborting:
+ case terminating:
+ case completed:
+ case dead:
+ return true;
+ default:
+ return false;
+ }
+}
+
/**
* isci_terminate_request_core() - This function will terminate the given
* request, and wait for it to complete. This function must only be called
@@ -574,7 +590,6 @@ static void isci_terminate_request_core(struct isci_host *ihost,
enum sci_status status = SCI_SUCCESS;
bool was_terminated = false;
bool needs_cleanup_handling = false;
- enum isci_request_status request_status;
unsigned long flags;
unsigned long termination_completed = 1;
struct completion *io_request_completion;
@@ -702,23 +717,11 @@ static void isci_terminate_request_core(struct isci_host *ihost,
* needs to be detached and freed here.
*/
spin_lock_irqsave(&isci_request->state_lock, flags);
- request_status = isci_request->status;
-
- if ((isci_request->ttype == io_task) /* TMFs are in their own thread */
- && ((request_status == aborted)
- || (request_status == aborting)
- || (request_status == terminating)
- || (request_status == completed)
- || (request_status == dead)
- )
- ) {
-
- /* The completion routine won't free a request in
- * the aborted/aborting/etc. states, so we do
- * it here.
- */
- needs_cleanup_handling = true;
- }
+
+ needs_cleanup_handling
+ = isci_request_is_dealloc_managed(
+ isci_request->status);
+
spin_unlock_irqrestore(&isci_request->state_lock, flags);
}
@@ -1329,8 +1332,16 @@ isci_task_request_complete(struct isci_host *ihost,
*/
set_bit(IREQ_TERMINATED, &ireq->flags);
- isci_request_change_state(ireq, unallocated);
- list_del_init(&ireq->dev_node);
+ /* As soon as something is in the terminate path, deallocation is
+ * managed there. Note that the final non-managed state of a task
+ * request is "completed".
+ */
+ if ((ireq->status == completed) ||
+ !isci_request_is_dealloc_managed(ireq->status)) {
+ isci_request_change_state(ireq, unallocated);
+ isci_free_tag(ihost, ireq->io_tag);
+ list_del_init(&ireq->dev_node);
+ }
/* The task management part completes last. */
complete(tmf_complete);
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 04/11] isci: Handle task request timeouts correctly.
2011-10-27 22:04 [PATCH 00/11] isci fixes: hotplug Dan Williams
` (2 preceding siblings ...)
2011-10-27 22:05 ` [PATCH 03/11] isci: Fix tag leak in tasks and terminated requests Dan Williams
@ 2011-10-27 22:05 ` Dan Williams
2011-10-27 22:05 ` [PATCH 05/11] isci: No task_done callbacks in error handler paths Dan Williams
` (6 subsequent siblings)
10 siblings, 0 replies; 13+ messages in thread
From: Dan Williams @ 2011-10-27 22:05 UTC (permalink / raw)
To: linux-scsi; +Cc: Jeff Skirvin
From: Jeff Skirvin <jeffrey.d.skirvin@intel.com>
In the case where "task" requests timeout (note that this class of
requests can also include SATA/STP soft reset FIS transmissions),
handle the case where the task was being managed by some call to
terminate the task request by completing both the tmf and the aborting
process.
Signed-off-by: Jeff Skirvin <jeffrey.d.skirvin@intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
drivers/scsi/isci/task.c | 149 +++++++++++++++++++++++++++++++++-------------
drivers/scsi/isci/task.h | 2 +
2 files changed, 109 insertions(+), 42 deletions(-)
diff --git a/drivers/scsi/isci/task.c b/drivers/scsi/isci/task.c
index 7180b04..4175f17 100644
--- a/drivers/scsi/isci/task.c
+++ b/drivers/scsi/isci/task.c
@@ -338,6 +338,61 @@ static struct isci_request *isci_task_request_build(struct isci_host *ihost,
return ireq;
}
+/**
+* isci_request_mark_zombie() - This function must be called with scic_lock held.
+*/
+static void isci_request_mark_zombie(struct isci_host *ihost, struct isci_request *ireq)
+{
+ struct completion *tmf_completion = NULL;
+ struct completion *req_completion;
+
+ /* Set the request state to "dead". */
+ ireq->status = dead;
+
+ req_completion = ireq->io_request_completion;
+ ireq->io_request_completion = NULL;
+
+ if (ireq->ttype == io_task) {
+
+ /* Break links with the sas_task - the callback is done
+ * elsewhere.
+ */
+ struct sas_task *task = isci_request_access_task(ireq);
+
+ if (task)
+ task->lldd_task = NULL;
+
+ ireq->ttype_ptr.io_task_ptr = NULL;
+ } else {
+ /* Break links with the TMF request. */
+ struct isci_tmf *tmf = isci_request_access_tmf(ireq);
+
+ /* In the case where a task request is dying,
+ * the thread waiting on the complete will sit and
+ * timeout unless we wake it now. Since the TMF
+ * has a default error status, complete it here
+ * to wake the waiting thread.
+ */
+ if (tmf) {
+ tmf_completion = tmf->complete;
+ tmf->complete = NULL;
+ }
+ ireq->ttype_ptr.tmf_task_ptr = NULL;
+ dev_dbg(&ihost->pdev->dev, "%s: tmf_code %d, managed tag %#x\n",
+ __func__, tmf->tmf_code, tmf->io_tag);
+ }
+
+ dev_warn(&ihost->pdev->dev, "task context unrecoverable (tag: %#x)\n",
+ ireq->io_tag);
+
+ /* Don't force waiting threads to timeout. */
+ if (req_completion)
+ complete(req_completion);
+
+ if (tmf_completion != NULL)
+ complete(tmf_completion);
+}
+
static int isci_task_execute_tmf(struct isci_host *ihost,
struct isci_remote_device *idev,
struct isci_tmf *tmf, unsigned long timeout_ms)
@@ -375,6 +430,7 @@ static int isci_task_execute_tmf(struct isci_host *ihost,
/* Assign the pointer to the TMF's completion kernel wait structure. */
tmf->complete = &completion;
+ tmf->status = SCI_FAILURE_TIMEOUT;
ireq = isci_task_request_build(ihost, idev, tag, tmf);
if (!ireq)
@@ -410,18 +466,35 @@ static int isci_task_execute_tmf(struct isci_host *ihost,
msecs_to_jiffies(timeout_ms));
if (timeleft == 0) {
+ /* The TMF did not complete - this could be because
+ * of an unplug. Terminate the TMF request now.
+ */
spin_lock_irqsave(&ihost->scic_lock, flags);
if (tmf->cb_state_func != NULL)
- tmf->cb_state_func(isci_tmf_timed_out, tmf, tmf->cb_data);
+ tmf->cb_state_func(isci_tmf_timed_out, tmf,
+ tmf->cb_data);
- sci_controller_terminate_request(ihost,
- idev,
- ireq);
+ sci_controller_terminate_request(ihost, idev, ireq);
spin_unlock_irqrestore(&ihost->scic_lock, flags);
- wait_for_completion(tmf->complete);
+ timeleft = wait_for_completion_timeout(
+ &completion,
+ msecs_to_jiffies(ISCI_TERMINATION_TIMEOUT_MSEC));
+
+ if (!timeleft) {
+ /* Strange condition - the termination of the TMF
+ * request timed-out.
+ */
+ spin_lock_irqsave(&ihost->scic_lock, flags);
+
+ /* If the TMF status has not changed, kill it. */
+ if (tmf->status == SCI_FAILURE_TIMEOUT)
+ isci_request_mark_zombie(ihost, ireq);
+
+ spin_unlock_irqrestore(&ihost->scic_lock, flags);
+ }
}
isci_print_tmf(tmf);
@@ -645,42 +718,27 @@ static void isci_terminate_request_core(struct isci_host *ihost,
__func__, isci_request, io_request_completion);
/* Wait here for the request to complete. */
- #define TERMINATION_TIMEOUT_MSEC 500
termination_completed
= wait_for_completion_timeout(
io_request_completion,
- msecs_to_jiffies(TERMINATION_TIMEOUT_MSEC));
+ msecs_to_jiffies(ISCI_TERMINATION_TIMEOUT_MSEC));
if (!termination_completed) {
/* The request to terminate has timed out. */
- spin_lock_irqsave(&ihost->scic_lock,
- flags);
+ spin_lock_irqsave(&ihost->scic_lock, flags);
/* Check for state changes. */
- if (!test_bit(IREQ_TERMINATED, &isci_request->flags)) {
+ if (!test_bit(IREQ_TERMINATED,
+ &isci_request->flags)) {
/* The best we can do is to have the
* request die a silent death if it
* ever really completes.
- *
- * Set the request state to "dead",
- * and clear the task pointer so that
- * an actual completion event callback
- * doesn't do anything.
*/
- isci_request->status = dead;
- isci_request->io_request_completion
- = NULL;
-
- if (isci_request->ttype == io_task) {
-
- /* Break links with the
- * sas_task.
- */
- isci_request->ttype_ptr.io_task_ptr
- = NULL;
- }
+ isci_request_mark_zombie(ihost,
+ isci_request);
+ needs_cleanup_handling = true;
} else
termination_completed = 1;
@@ -1302,7 +1360,8 @@ isci_task_request_complete(struct isci_host *ihost,
enum sci_task_status completion_status)
{
struct isci_tmf *tmf = isci_request_access_tmf(ireq);
- struct completion *tmf_complete;
+ struct completion *tmf_complete = NULL;
+ struct completion *request_complete = ireq->io_request_completion;
dev_dbg(&ihost->pdev->dev,
"%s: request = %p, status=%d\n",
@@ -1310,22 +1369,23 @@ isci_task_request_complete(struct isci_host *ihost,
isci_request_change_state(ireq, completed);
- tmf->status = completion_status;
set_bit(IREQ_COMPLETE_IN_TARGET, &ireq->flags);
- if (tmf->proto == SAS_PROTOCOL_SSP) {
- memcpy(&tmf->resp.resp_iu,
- &ireq->ssp.rsp,
- SSP_RESP_IU_MAX_SIZE);
- } else if (tmf->proto == SAS_PROTOCOL_SATA) {
- memcpy(&tmf->resp.d2h_fis,
- &ireq->stp.rsp,
- sizeof(struct dev_to_host_fis));
+ if (tmf) {
+ tmf->status = completion_status;
+
+ if (tmf->proto == SAS_PROTOCOL_SSP) {
+ memcpy(&tmf->resp.resp_iu,
+ &ireq->ssp.rsp,
+ SSP_RESP_IU_MAX_SIZE);
+ } else if (tmf->proto == SAS_PROTOCOL_SATA) {
+ memcpy(&tmf->resp.d2h_fis,
+ &ireq->stp.rsp,
+ sizeof(struct dev_to_host_fis));
+ }
+ /* PRINT_TMF( ((struct isci_tmf *)request->task)); */
+ tmf_complete = tmf->complete;
}
-
- /* PRINT_TMF( ((struct isci_tmf *)request->task)); */
- tmf_complete = tmf->complete;
-
sci_controller_complete_io(ihost, ireq->target_device, ireq);
/* set the 'terminated' flag handle to make sure it cannot be terminated
* or completed again.
@@ -1343,8 +1403,13 @@ isci_task_request_complete(struct isci_host *ihost,
list_del_init(&ireq->dev_node);
}
+ /* "request_complete" is set if the task was being terminated. */
+ if (request_complete)
+ complete(request_complete);
+
/* The task management part completes last. */
- complete(tmf_complete);
+ if (tmf_complete)
+ complete(tmf_complete);
}
static void isci_smp_task_timedout(unsigned long _task)
diff --git a/drivers/scsi/isci/task.h b/drivers/scsi/isci/task.h
index 15b18d1..c9ccd0b 100644
--- a/drivers/scsi/isci/task.h
+++ b/drivers/scsi/isci/task.h
@@ -58,6 +58,8 @@
#include <scsi/sas_ata.h>
#include "host.h"
+#define ISCI_TERMINATION_TIMEOUT_MSEC 500
+
struct isci_request;
/**
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 05/11] isci: No task_done callbacks in error handler paths.
2011-10-27 22:04 [PATCH 00/11] isci fixes: hotplug Dan Williams
` (3 preceding siblings ...)
2011-10-27 22:05 ` [PATCH 04/11] isci: Handle task request timeouts correctly Dan Williams
@ 2011-10-27 22:05 ` Dan Williams
2011-10-27 22:05 ` [PATCH 06/11] isci: Fix task management for SMP, SATA and on dev remove Dan Williams
` (5 subsequent siblings)
10 siblings, 0 replies; 13+ messages in thread
From: Dan Williams @ 2011-10-27 22:05 UTC (permalink / raw)
To: linux-scsi; +Cc: Jeff Skirvin
From: Jeff Skirvin <jeffrey.d.skirvin@intel.com>
libsas will cleanup pending sas_tasks after error handler
path functions are called; do not call task_done callbacks.
Signed-off-by: Jeff Skirvin <jeffrey.d.skirvin@intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
drivers/scsi/isci/task.c | 69 +++++++++-------------------------------------
1 files changed, 14 insertions(+), 55 deletions(-)
diff --git a/drivers/scsi/isci/task.c b/drivers/scsi/isci/task.c
index 4175f17..c1f439b 100644
--- a/drivers/scsi/isci/task.c
+++ b/drivers/scsi/isci/task.c
@@ -585,53 +585,6 @@ static enum isci_request_status isci_task_validate_request_to_abort(
return old_state;
}
-/**
-* isci_request_cleanup_completed_loiterer() - This function will take care of
-* the final cleanup on any request which has been explicitly terminated.
-* @isci_host: This parameter specifies the ISCI host object
-* @isci_device: This is the device to which the request is pending.
-* @isci_request: This parameter specifies the terminated request object.
-* @task: This parameter is the libsas I/O request.
-*/
-static void isci_request_cleanup_completed_loiterer(
- struct isci_host *isci_host,
- struct isci_remote_device *isci_device,
- struct isci_request *isci_request,
- struct sas_task *task)
-{
- unsigned long flags;
-
- dev_dbg(&isci_host->pdev->dev,
- "%s: isci_device=%p, request=%p, task=%p\n",
- __func__, isci_device, isci_request, task);
-
- if (task != NULL) {
-
- spin_lock_irqsave(&task->task_state_lock, flags);
- task->lldd_task = NULL;
-
- task->task_state_flags &= ~SAS_TASK_NEED_DEV_RESET;
-
- isci_set_task_doneflags(task);
-
- /* If this task is not in the abort path, call task_done. */
- if (!(task->task_state_flags & SAS_TASK_STATE_ABORTED)) {
-
- spin_unlock_irqrestore(&task->task_state_lock, flags);
- task->task_done(task);
- } else
- spin_unlock_irqrestore(&task->task_state_lock, flags);
- }
-
- if (isci_request != NULL) {
- spin_lock_irqsave(&isci_host->scic_lock, flags);
- isci_free_tag(isci_host, isci_request->io_tag);
- isci_request_change_state(isci_request, unallocated);
- list_del_init(&isci_request->dev_node);
- spin_unlock_irqrestore(&isci_host->scic_lock, flags);
- }
-}
-
static int isci_request_is_dealloc_managed(enum isci_request_status stat)
{
switch (stat) {
@@ -666,7 +619,6 @@ static void isci_terminate_request_core(struct isci_host *ihost,
unsigned long flags;
unsigned long termination_completed = 1;
struct completion *io_request_completion;
- struct sas_task *task;
dev_dbg(&ihost->pdev->dev,
"%s: device = %p; request = %p\n",
@@ -676,10 +628,6 @@ static void isci_terminate_request_core(struct isci_host *ihost,
io_request_completion = isci_request->io_request_completion;
- task = (isci_request->ttype == io_task)
- ? isci_request_access_task(isci_request)
- : NULL;
-
/* Note that we are not going to control
* the target to abort the request.
*/
@@ -783,9 +731,20 @@ static void isci_terminate_request_core(struct isci_host *ihost,
spin_unlock_irqrestore(&isci_request->state_lock, flags);
}
- if (needs_cleanup_handling)
- isci_request_cleanup_completed_loiterer(
- ihost, idev, isci_request, task);
+ if (needs_cleanup_handling) {
+
+ dev_dbg(&ihost->pdev->dev,
+ "%s: cleanup isci_device=%p, request=%p\n",
+ __func__, idev, isci_request);
+
+ if (isci_request != NULL) {
+ spin_lock_irqsave(&ihost->scic_lock, flags);
+ isci_free_tag(ihost, isci_request->io_tag);
+ isci_request_change_state(isci_request, unallocated);
+ list_del_init(&isci_request->dev_node);
+ spin_unlock_irqrestore(&ihost->scic_lock, flags);
+ }
+ }
}
}
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 06/11] isci: Fix task management for SMP, SATA and on dev remove.
2011-10-27 22:04 [PATCH 00/11] isci fixes: hotplug Dan Williams
` (4 preceding siblings ...)
2011-10-27 22:05 ` [PATCH 05/11] isci: No task_done callbacks in error handler paths Dan Williams
@ 2011-10-27 22:05 ` Dan Williams
2011-10-27 22:05 ` [PATCH 07/11] isci: Remove redundant isci_request.ttype field Dan Williams
` (4 subsequent siblings)
10 siblings, 0 replies; 13+ messages in thread
From: Dan Williams @ 2011-10-27 22:05 UTC (permalink / raw)
To: linux-scsi; +Cc: Jeff Skirvin
From: Jeff Skirvin <jeffrey.d.skirvin@intel.com>
libsas uses the LLDD abort task interface to handle I/O timeouts
in the SATA/STP and SMP discovery paths, so this change will terminate
STP/SMP requests. Also, if the device is gone, the lldd will prevent
libsas from further escalations in the error handler.
Signed-off-by: Jeff Skirvin <jeffrey.d.skirvin@intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
drivers/scsi/isci/remote_device.c | 47 ----------
drivers/scsi/isci/remote_device.h | 2
drivers/scsi/isci/task.c | 169 ++++++++++++++-----------------------
drivers/scsi/isci/task.h | 33 +------
4 files changed, 67 insertions(+), 184 deletions(-)
diff --git a/drivers/scsi/isci/remote_device.c b/drivers/scsi/isci/remote_device.c
index fbf9ce2..20c77ed 100644
--- a/drivers/scsi/isci/remote_device.c
+++ b/drivers/scsi/isci/remote_device.c
@@ -1438,53 +1438,6 @@ int isci_remote_device_found(struct domain_device *domain_dev)
return status == SCI_SUCCESS ? 0 : -ENODEV;
}
-/**
- * isci_device_is_reset_pending() - This function will check if there is any
- * pending reset condition on the device.
- * @request: This parameter is the isci_device object.
- *
- * true if there is a reset pending for the device.
- */
-bool isci_device_is_reset_pending(
- struct isci_host *isci_host,
- struct isci_remote_device *isci_device)
-{
- struct isci_request *isci_request;
- struct isci_request *tmp_req;
- bool reset_is_pending = false;
- unsigned long flags;
-
- dev_dbg(&isci_host->pdev->dev,
- "%s: isci_device = %p\n", __func__, isci_device);
-
- spin_lock_irqsave(&isci_host->scic_lock, flags);
-
- /* Check for reset on all pending requests. */
- list_for_each_entry_safe(isci_request, tmp_req,
- &isci_device->reqs_in_process, dev_node) {
- dev_dbg(&isci_host->pdev->dev,
- "%s: isci_device = %p request = %p\n",
- __func__, isci_device, isci_request);
-
- if (isci_request->ttype == io_task) {
- struct sas_task *task = isci_request_access_task(
- isci_request);
-
- spin_lock(&task->task_state_lock);
- if (task->task_state_flags & SAS_TASK_NEED_DEV_RESET)
- reset_is_pending = true;
- spin_unlock(&task->task_state_lock);
- }
- }
-
- spin_unlock_irqrestore(&isci_host->scic_lock, flags);
-
- dev_dbg(&isci_host->pdev->dev,
- "%s: isci_device = %p reset_is_pending = %d\n",
- __func__, isci_device, reset_is_pending);
-
- return reset_is_pending;
-}
/**
* isci_device_clear_reset_pending() - This function will clear if any pending
diff --git a/drivers/scsi/isci/remote_device.h b/drivers/scsi/isci/remote_device.h
index e1747ea..bee6dd2 100644
--- a/drivers/scsi/isci/remote_device.h
+++ b/drivers/scsi/isci/remote_device.h
@@ -132,8 +132,6 @@ void isci_remote_device_nuke_requests(struct isci_host *ihost,
struct isci_remote_device *idev);
void isci_remote_device_gone(struct domain_device *domain_dev);
int isci_remote_device_found(struct domain_device *domain_dev);
-bool isci_device_is_reset_pending(struct isci_host *ihost,
- struct isci_remote_device *idev);
void isci_device_clear_reset_pending(struct isci_host *ihost,
struct isci_remote_device *idev);
/**
diff --git a/drivers/scsi/isci/task.c b/drivers/scsi/isci/task.c
index c1f439b..ec85beb 100644
--- a/drivers/scsi/isci/task.c
+++ b/drivers/scsi/isci/task.c
@@ -920,22 +920,14 @@ int isci_task_lu_reset(struct domain_device *domain_device, u8 *lun)
"%s: domain_device=%p, isci_host=%p; isci_device=%p\n",
__func__, domain_device, isci_host, isci_device);
- if (isci_device)
- set_bit(IDEV_EH, &isci_device->flags);
+ if (!isci_device) {
+ /* If the device is gone, stop the escalations. */
+ dev_dbg(&isci_host->pdev->dev, "%s: No dev\n", __func__);
- /* If there is a device reset pending on any request in the
- * device's list, fail this LUN reset request in order to
- * escalate to the device reset.
- */
- if (!isci_device ||
- isci_device_is_reset_pending(isci_host, isci_device)) {
- dev_dbg(&isci_host->pdev->dev,
- "%s: No dev (%p), or "
- "RESET PENDING: domain_device=%p\n",
- __func__, isci_device, domain_device);
- ret = TMF_RESP_FUNC_FAILED;
+ ret = TMF_RESP_FUNC_COMPLETE;
goto out;
}
+ set_bit(IDEV_EH, &isci_device->flags);
/* Send the task management part of the reset. */
if (sas_protocol_ata(domain_device->tproto)) {
@@ -1044,7 +1036,7 @@ int isci_task_abort_task(struct sas_task *task)
struct isci_tmf tmf;
int ret = TMF_RESP_FUNC_FAILED;
unsigned long flags;
- bool any_dev_reset = false;
+ int perform_termination = 0;
/* Get the isci_request reference from the task. Note that
* this check does not depend on the pending request list
@@ -1066,89 +1058,34 @@ int isci_task_abort_task(struct sas_task *task)
spin_unlock_irqrestore(&isci_host->scic_lock, flags);
dev_dbg(&isci_host->pdev->dev,
- "%s: task = %p\n", __func__, task);
-
- if (!isci_device || !old_request)
- goto out;
+ "%s: dev = %p, task = %p, old_request == %p\n",
+ __func__, isci_device, task, old_request);
- set_bit(IDEV_EH, &isci_device->flags);
-
- /* This version of the driver will fail abort requests for
- * SATA/STP. Failing the abort request this way will cause the
- * SCSI error handler thread to escalate to LUN reset
- */
- if (sas_protocol_ata(task->task_proto)) {
- dev_dbg(&isci_host->pdev->dev,
- " task %p is for a STP/SATA device;"
- " returning TMF_RESP_FUNC_FAILED\n"
- " to cause a LUN reset...\n", task);
- goto out;
- }
-
- dev_dbg(&isci_host->pdev->dev,
- "%s: old_request == %p\n", __func__, old_request);
-
- any_dev_reset = isci_device_is_reset_pending(isci_host, isci_device);
-
- spin_lock_irqsave(&task->task_state_lock, flags);
-
- any_dev_reset = any_dev_reset || (task->task_state_flags & SAS_TASK_NEED_DEV_RESET);
+ if (isci_device)
+ set_bit(IDEV_EH, &isci_device->flags);
- /* If the extraction of the request reference from the task
- * failed, then the request has been completed (or if there is a
- * pending reset then this abort request function must be failed
- * in order to escalate to the target reset).
+ /* Device reset conditions signalled in task_state_flags are the
+ * responsbility of libsas to observe at the start of the error
+ * handler thread.
*/
- if ((old_request == NULL) || any_dev_reset) {
-
- /* If the device reset task flag is set, fail the task
- * management request. Otherwise, the original request
- * has completed.
- */
- if (any_dev_reset) {
-
- /* Turn off the task's DONE to make sure this
- * task is escalated to a target reset.
- */
- task->task_state_flags &= ~SAS_TASK_STATE_DONE;
-
- /* Make the reset happen as soon as possible. */
- task->task_state_flags |= SAS_TASK_NEED_DEV_RESET;
-
- spin_unlock_irqrestore(&task->task_state_lock, flags);
-
- /* Fail the task management request in order to
- * escalate to the target reset.
- */
- ret = TMF_RESP_FUNC_FAILED;
-
- dev_dbg(&isci_host->pdev->dev,
- "%s: Failing task abort in order to "
- "escalate to target reset because\n"
- "SAS_TASK_NEED_DEV_RESET is set for "
- "task %p on dev %p\n",
- __func__, task, isci_device);
-
-
- } else {
- /* The request has already completed and there
- * is nothing to do here other than to set the task
- * done bit, and indicate that the task abort function
- * was sucessful.
- */
- isci_set_task_doneflags(task);
-
- spin_unlock_irqrestore(&task->task_state_lock, flags);
+ if (!isci_device || !old_request) {
+ /* The request has already completed and there
+ * is nothing to do here other than to set the task
+ * done bit, and indicate that the task abort function
+ * was sucessful.
+ */
+ spin_lock_irqsave(&task->task_state_lock, flags);
+ task->task_state_flags |= SAS_TASK_STATE_DONE;
+ task->task_state_flags &= ~(SAS_TASK_AT_INITIATOR |
+ SAS_TASK_STATE_PENDING);
+ spin_unlock_irqrestore(&task->task_state_lock, flags);
- ret = TMF_RESP_FUNC_COMPLETE;
+ ret = TMF_RESP_FUNC_COMPLETE;
- dev_dbg(&isci_host->pdev->dev,
- "%s: abort task not needed for %p\n",
- __func__, task);
- }
+ dev_dbg(&isci_host->pdev->dev,
+ "%s: abort task not needed for %p\n",
+ __func__, task);
goto out;
- } else {
- spin_unlock_irqrestore(&task->task_state_lock, flags);
}
spin_lock_irqsave(&isci_host->scic_lock, flags);
@@ -1177,24 +1114,44 @@ int isci_task_abort_task(struct sas_task *task)
goto out;
}
if (task->task_proto == SAS_PROTOCOL_SMP ||
+ sas_protocol_ata(task->task_proto) ||
test_bit(IREQ_COMPLETE_IN_TARGET, &old_request->flags)) {
spin_unlock_irqrestore(&isci_host->scic_lock, flags);
dev_dbg(&isci_host->pdev->dev,
- "%s: SMP request (%d)"
+ "%s: %s request"
" or complete_in_target (%d), thus no TMF\n",
- __func__, (task->task_proto == SAS_PROTOCOL_SMP),
+ __func__,
+ ((task->task_proto == SAS_PROTOCOL_SMP)
+ ? "SMP"
+ : (sas_protocol_ata(task->task_proto)
+ ? "SATA/STP"
+ : "<other>")
+ ),
test_bit(IREQ_COMPLETE_IN_TARGET, &old_request->flags));
- /* Set the state on the task. */
- isci_task_all_done(task);
-
- ret = TMF_RESP_FUNC_COMPLETE;
+ if (test_bit(IREQ_COMPLETE_IN_TARGET, &old_request->flags)) {
+ spin_lock_irqsave(&task->task_state_lock, flags);
+ task->task_state_flags |= SAS_TASK_STATE_DONE;
+ task->task_state_flags &= ~(SAS_TASK_AT_INITIATOR |
+ SAS_TASK_STATE_PENDING);
+ spin_unlock_irqrestore(&task->task_state_lock, flags);
+ ret = TMF_RESP_FUNC_COMPLETE;
+ } else {
+ spin_lock_irqsave(&task->task_state_lock, flags);
+ task->task_state_flags &= ~(SAS_TASK_AT_INITIATOR |
+ SAS_TASK_STATE_PENDING);
+ spin_unlock_irqrestore(&task->task_state_lock, flags);
+ }
- /* Stopping and SMP devices are not sent a TMF, and are not
- * reset, but the outstanding I/O request is terminated below.
+ /* STP and SMP devices are not sent a TMF, but the
+ * outstanding I/O request is terminated below. This is
+ * because SATA/STP and SMP discovery path timeouts directly
+ * call the abort task interface for cleanup.
*/
+ perform_termination = 1;
+
} else {
/* Fill in the tmf stucture */
isci_task_build_abort_task_tmf(&tmf, isci_tmf_ssp_task_abort,
@@ -1203,22 +1160,24 @@ int isci_task_abort_task(struct sas_task *task)
spin_unlock_irqrestore(&isci_host->scic_lock, flags);
- #define ISCI_ABORT_TASK_TIMEOUT_MS 500 /* half second timeout. */
+ #define ISCI_ABORT_TASK_TIMEOUT_MS 500 /* 1/2 second timeout */
ret = isci_task_execute_tmf(isci_host, isci_device, &tmf,
ISCI_ABORT_TASK_TIMEOUT_MS);
- if (ret != TMF_RESP_FUNC_COMPLETE)
+ if (ret == TMF_RESP_FUNC_COMPLETE)
+ perform_termination = 1;
+ else
dev_dbg(&isci_host->pdev->dev,
- "%s: isci_task_send_tmf failed\n",
- __func__);
+ "%s: isci_task_send_tmf failed\n", __func__);
}
- if (ret == TMF_RESP_FUNC_COMPLETE) {
+ if (perform_termination) {
set_bit(IREQ_COMPLETE_IN_TARGET, &old_request->flags);
/* Clean up the request on our side, and wait for the aborted
* I/O to complete.
*/
- isci_terminate_request_core(isci_host, isci_device, old_request);
+ isci_terminate_request_core(isci_host, isci_device,
+ old_request);
}
/* Make sure we do not leave a reference to aborted_io_completion */
diff --git a/drivers/scsi/isci/task.h b/drivers/scsi/isci/task.h
index c9ccd0b..bc78c0a 100644
--- a/drivers/scsi/isci/task.h
+++ b/drivers/scsi/isci/task.h
@@ -226,35 +226,6 @@ enum isci_completion_selection {
isci_perform_error_io_completion /* Use sas_task_abort */
};
-static inline void isci_set_task_doneflags(
- struct sas_task *task)
-{
- /* Since no futher action will be taken on this task,
- * make sure to mark it complete from the lldd perspective.
- */
- task->task_state_flags |= SAS_TASK_STATE_DONE;
- task->task_state_flags &= ~SAS_TASK_AT_INITIATOR;
- task->task_state_flags &= ~SAS_TASK_STATE_PENDING;
-}
-/**
- * isci_task_all_done() - This function clears the task bits to indicate the
- * LLDD is done with the task.
- *
- *
- */
-static inline void isci_task_all_done(
- struct sas_task *task)
-{
- unsigned long flags;
-
- /* Since no futher action will be taken on this task,
- * make sure to mark it complete from the lldd perspective.
- */
- spin_lock_irqsave(&task->task_state_lock, flags);
- isci_set_task_doneflags(task);
- spin_unlock_irqrestore(&task->task_state_lock, flags);
-}
-
/**
* isci_task_set_completion_status() - This function sets the completion status
* for the request.
@@ -336,7 +307,9 @@ isci_task_set_completion_status(
/* Fall through to the normal case... */
case isci_perform_normal_io_completion:
/* Normal notification (task_done) */
- isci_set_task_doneflags(task);
+ task->task_state_flags |= SAS_TASK_STATE_DONE;
+ task->task_state_flags &= ~(SAS_TASK_AT_INITIATOR |
+ SAS_TASK_STATE_PENDING);
break;
default:
WARN_ONCE(1, "unknown task_notification_selection: %d\n",
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 07/11] isci: Remove redundant isci_request.ttype field.
2011-10-27 22:04 [PATCH 00/11] isci fixes: hotplug Dan Williams
` (5 preceding siblings ...)
2011-10-27 22:05 ` [PATCH 06/11] isci: Fix task management for SMP, SATA and on dev remove Dan Williams
@ 2011-10-27 22:05 ` Dan Williams
2011-10-31 9:23 ` James Bottomley
2011-10-27 22:05 ` [PATCH 08/11] isci: No need to manage the pending reset bit on pending requests Dan Williams
` (3 subsequent siblings)
10 siblings, 1 reply; 13+ messages in thread
From: Dan Williams @ 2011-10-27 22:05 UTC (permalink / raw)
To: linux-scsi; +Cc: Jeff Skirvin
From: Jeff Skirvin <jeffrey.d.skirvin@intel.com>
Use the existing IREQ_TMF flag as a request type indicator.
Signed-off-by: Jeff Skirvin <jeffrey.d.skirvin@intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
drivers/scsi/isci/remote_device.c | 11 +++------
drivers/scsi/isci/request.c | 45 ++++++++++---------------------------
drivers/scsi/isci/request.h | 6 -----
drivers/scsi/isci/task.c | 29 ++++++++++++------------
4 files changed, 31 insertions(+), 60 deletions(-)
diff --git a/drivers/scsi/isci/remote_device.c b/drivers/scsi/isci/remote_device.c
index 20c77ed..9d9e33d 100644
--- a/drivers/scsi/isci/remote_device.c
+++ b/drivers/scsi/isci/remote_device.c
@@ -1463,15 +1463,12 @@ void isci_device_clear_reset_pending(struct isci_host *ihost, struct isci_remote
dev_dbg(&ihost->pdev->dev, "%s: idev = %p request = %p\n",
__func__, idev, isci_request);
- if (isci_request->ttype == io_task) {
+ if (!test_bit(IREQ_TMF, &isci_request->flags)) {
+ struct sas_task *task = isci_request_access_task(isci_request);
- unsigned long flags2;
- struct sas_task *task = isci_request_access_task(
- isci_request);
-
- spin_lock_irqsave(&task->task_state_lock, flags2);
+ spin_lock(&task->task_state_lock);
task->task_state_flags &= ~SAS_TASK_NEED_DEV_RESET;
- spin_unlock_irqrestore(&task->task_state_lock, flags2);
+ spin_unlock(&task->task_state_lock);
}
}
spin_unlock_irqrestore(&ihost->scic_lock, flags);
diff --git a/drivers/scsi/isci/request.c b/drivers/scsi/isci/request.c
index bfc7379..dccb431 100644
--- a/drivers/scsi/isci/request.c
+++ b/drivers/scsi/isci/request.c
@@ -191,7 +191,7 @@ static void sci_task_request_build_ssp_task_iu(struct isci_request *ireq)
task_iu->task_func = isci_tmf->tmf_code;
task_iu->task_tag =
- (ireq->ttype == tmf_task) ?
+ (test_bit(IREQ_TMF, &ireq->flags)) ?
isci_tmf->io_tag :
SCI_CONTROLLER_INVALID_IO_TAG;
}
@@ -516,7 +516,7 @@ sci_io_request_construct_sata(struct isci_request *ireq,
struct domain_device *dev = ireq->target_device->domain_dev;
/* check for management protocols */
- if (ireq->ttype == tmf_task) {
+ if (test_bit(IREQ_TMF, &ireq->flags)) {
struct isci_tmf *tmf = isci_request_access_tmf(ireq);
if (tmf->tmf_code == isci_tmf_sata_srst_high ||
@@ -632,7 +632,7 @@ enum sci_status sci_task_request_construct_sata(struct isci_request *ireq)
enum sci_status status = SCI_SUCCESS;
/* check for management protocols */
- if (ireq->ttype == tmf_task) {
+ if (test_bit(IREQ_TMF, &ireq->flags)) {
struct isci_tmf *tmf = isci_request_access_tmf(ireq);
if (tmf->tmf_code == isci_tmf_sata_srst_high ||
@@ -2630,14 +2630,8 @@ static void isci_task_save_for_upper_layer_completion(
switch (task_notification_selection) {
case isci_perform_normal_io_completion:
-
/* Normal notification (task_done) */
- dev_dbg(&host->pdev->dev,
- "%s: Normal - task = %p, response=%d (%d), status=%d (%d)\n",
- __func__,
- task,
- task->task_status.resp, response,
- task->task_status.stat, status);
+
/* Add to the completed list. */
list_add(&request->completed_node,
&host->requests_to_complete);
@@ -2650,13 +2644,6 @@ static void isci_task_save_for_upper_layer_completion(
/* No notification to libsas because this request is
* already in the abort path.
*/
- dev_dbg(&host->pdev->dev,
- "%s: Aborted - task = %p, response=%d (%d), status=%d (%d)\n",
- __func__,
- task,
- task->task_status.resp, response,
- task->task_status.stat, status);
-
/* Wake up whatever process was waiting for this
* request to complete.
*/
@@ -2673,30 +2660,22 @@ static void isci_task_save_for_upper_layer_completion(
case isci_perform_error_io_completion:
/* Use sas_task_abort */
- dev_dbg(&host->pdev->dev,
- "%s: Error - task = %p, response=%d (%d), status=%d (%d)\n",
- __func__,
- task,
- task->task_status.resp, response,
- task->task_status.stat, status);
/* Add to the aborted list. */
list_add(&request->completed_node,
&host->requests_to_errorback);
break;
default:
- dev_dbg(&host->pdev->dev,
- "%s: Unknown - task = %p, response=%d (%d), status=%d (%d)\n",
- __func__,
- task,
- task->task_status.resp, response,
- task->task_status.stat, status);
-
/* Add to the error to libsas list. */
list_add(&request->completed_node,
&host->requests_to_errorback);
break;
}
+ dev_dbg(&host->pdev->dev,
+ "%s: %d - task = %p, response=%d (%d), status=%d (%d)\n",
+ __func__, task_notification_selection, task,
+ (task) ? task->task_status.resp : 0, response,
+ (task) ? task->task_status.stat : 0, status);
}
static void isci_process_stp_response(struct sas_task *task, struct dev_to_host_fis *fis)
@@ -3079,7 +3058,7 @@ static void sci_request_started_state_enter(struct sci_base_state_machine *sm)
/* XXX as hch said always creating an internal sas_task for tmf
* requests would simplify the driver
*/
- task = ireq->ttype == io_task ? isci_request_access_task(ireq) : NULL;
+ task = (test_bit(IREQ_TMF, &ireq->flags)) ? NULL : isci_request_access_task(ireq);
/* all unaccelerated request types (non ssp or ncq) handled with
* substates
@@ -3563,7 +3542,7 @@ static struct isci_request *isci_io_request_from_tag(struct isci_host *ihost,
ireq = isci_request_from_tag(ihost, tag);
ireq->ttype_ptr.io_task_ptr = task;
- ireq->ttype = io_task;
+ clear_bit(IREQ_TMF, &ireq->flags);;
task->lldd_task = ireq;
return ireq;
@@ -3577,7 +3556,7 @@ struct isci_request *isci_tmf_request_from_tag(struct isci_host *ihost,
ireq = isci_request_from_tag(ihost, tag);
ireq->ttype_ptr.tmf_task_ptr = isci_tmf;
- ireq->ttype = tmf_task;
+ set_bit(IREQ_TMF, &ireq->flags);
return ireq;
}
diff --git a/drivers/scsi/isci/request.h b/drivers/scsi/isci/request.h
index f720b97..be38933 100644
--- a/drivers/scsi/isci/request.h
+++ b/drivers/scsi/isci/request.h
@@ -77,11 +77,6 @@ enum isci_request_status {
dead = 0x07
};
-enum task_type {
- io_task = 0,
- tmf_task = 1
-};
-
enum sci_request_protocol {
SCIC_NO_PROTOCOL,
SCIC_SMP_PROTOCOL,
@@ -116,7 +111,6 @@ struct isci_request {
#define IREQ_ACTIVE 3
unsigned long flags;
/* XXX kill ttype and ttype_ptr, allocate full sas_task */
- enum task_type ttype;
union ttype_ptr_union {
struct sas_task *io_task_ptr; /* When ttype==io_task */
struct isci_tmf *tmf_task_ptr; /* When ttype==tmf_task */
diff --git a/drivers/scsi/isci/task.c b/drivers/scsi/isci/task.c
index ec85beb..80e1a69 100644
--- a/drivers/scsi/isci/task.c
+++ b/drivers/scsi/isci/task.c
@@ -254,7 +254,7 @@ static enum sci_status isci_sata_management_task_request_build(struct isci_reque
struct isci_tmf *isci_tmf;
enum sci_status status;
- if (tmf_task != ireq->ttype)
+ if (!test_bit(IREQ_TMF, &ireq->flags))
return SCI_FAILURE;
isci_tmf = isci_request_access_tmf(ireq);
@@ -352,18 +352,7 @@ static void isci_request_mark_zombie(struct isci_host *ihost, struct isci_reques
req_completion = ireq->io_request_completion;
ireq->io_request_completion = NULL;
- if (ireq->ttype == io_task) {
-
- /* Break links with the sas_task - the callback is done
- * elsewhere.
- */
- struct sas_task *task = isci_request_access_task(ireq);
-
- if (task)
- task->lldd_task = NULL;
-
- ireq->ttype_ptr.io_task_ptr = NULL;
- } else {
+ if (test_bit(IREQ_TMF, &ireq->flags)) {
/* Break links with the TMF request. */
struct isci_tmf *tmf = isci_request_access_tmf(ireq);
@@ -380,6 +369,16 @@ static void isci_request_mark_zombie(struct isci_host *ihost, struct isci_reques
ireq->ttype_ptr.tmf_task_ptr = NULL;
dev_dbg(&ihost->pdev->dev, "%s: tmf_code %d, managed tag %#x\n",
__func__, tmf->tmf_code, tmf->io_tag);
+ } else {
+ /* Break links with the sas_task - the callback is done
+ * elsewhere.
+ */
+ struct sas_task *task = isci_request_access_task(ireq);
+
+ if (task)
+ task->lldd_task = NULL;
+
+ ireq->ttype_ptr.io_task_ptr = NULL;
}
dev_warn(&ihost->pdev->dev, "task context unrecoverable (tag: %#x)\n",
@@ -803,7 +802,9 @@ void isci_terminate_pending_requests(struct isci_host *ihost,
dev_dbg(&ihost->pdev->dev,
"%s: idev=%p request=%p; task=%p old_state=%d\n",
__func__, idev, ireq,
- ireq->ttype == io_task ? isci_request_access_task(ireq) : NULL,
+ (!test_bit(IREQ_TMF, &ireq->flags)
+ ? isci_request_access_task(ireq)
+ : NULL),
old_state);
/* If the old_state is started:
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 08/11] isci: No need to manage the pending reset bit on pending requests.
2011-10-27 22:04 [PATCH 00/11] isci fixes: hotplug Dan Williams
` (6 preceding siblings ...)
2011-10-27 22:05 ` [PATCH 07/11] isci: Remove redundant isci_request.ttype field Dan Williams
@ 2011-10-27 22:05 ` Dan Williams
2011-10-27 22:05 ` [PATCH 09/11] isci: Fix hard reset timeout conditions Dan Williams
` (2 subsequent siblings)
10 siblings, 0 replies; 13+ messages in thread
From: Dan Williams @ 2011-10-27 22:05 UTC (permalink / raw)
To: linux-scsi; +Cc: Jeff Skirvin
From: Jeff Skirvin <jeffrey.d.skirvin@intel.com>
The lldd does not need to look at or manage the pending device
reset bit in pending sas_tasks.
Signed-off-by: Jeff Skirvin <jeffrey.d.skirvin@intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
drivers/scsi/isci/remote_device.c | 35 -----------------------------------
drivers/scsi/isci/remote_device.h | 3 +--
drivers/scsi/isci/task.c | 3 ---
3 files changed, 1 insertions(+), 40 deletions(-)
diff --git a/drivers/scsi/isci/remote_device.c b/drivers/scsi/isci/remote_device.c
index 9d9e33d..b207cd3 100644
--- a/drivers/scsi/isci/remote_device.c
+++ b/drivers/scsi/isci/remote_device.c
@@ -1438,38 +1438,3 @@ int isci_remote_device_found(struct domain_device *domain_dev)
return status == SCI_SUCCESS ? 0 : -ENODEV;
}
-
-/**
- * isci_device_clear_reset_pending() - This function will clear if any pending
- * reset condition flags on the device.
- * @request: This parameter is the isci_device object.
- *
- * true if there is a reset pending for the device.
- */
-void isci_device_clear_reset_pending(struct isci_host *ihost, struct isci_remote_device *idev)
-{
- struct isci_request *isci_request;
- struct isci_request *tmp_req;
- unsigned long flags = 0;
-
- dev_dbg(&ihost->pdev->dev, "%s: idev=%p, ihost=%p\n",
- __func__, idev, ihost);
-
- spin_lock_irqsave(&ihost->scic_lock, flags);
-
- /* Clear reset pending on all pending requests. */
- list_for_each_entry_safe(isci_request, tmp_req,
- &idev->reqs_in_process, dev_node) {
- dev_dbg(&ihost->pdev->dev, "%s: idev = %p request = %p\n",
- __func__, idev, isci_request);
-
- if (!test_bit(IREQ_TMF, &isci_request->flags)) {
- struct sas_task *task = isci_request_access_task(isci_request);
-
- spin_lock(&task->task_state_lock);
- task->task_state_flags &= ~SAS_TASK_NEED_DEV_RESET;
- spin_unlock(&task->task_state_lock);
- }
- }
- spin_unlock_irqrestore(&ihost->scic_lock, flags);
-}
diff --git a/drivers/scsi/isci/remote_device.h b/drivers/scsi/isci/remote_device.h
index bee6dd2..483ee50 100644
--- a/drivers/scsi/isci/remote_device.h
+++ b/drivers/scsi/isci/remote_device.h
@@ -132,8 +132,7 @@ void isci_remote_device_nuke_requests(struct isci_host *ihost,
struct isci_remote_device *idev);
void isci_remote_device_gone(struct domain_device *domain_dev);
int isci_remote_device_found(struct domain_device *domain_dev);
-void isci_device_clear_reset_pending(struct isci_host *ihost,
- struct isci_remote_device *idev);
+
/**
* sci_remote_device_stop() - This method will stop both transmission and
* reception of link activity for the supplied remote device. This method
diff --git a/drivers/scsi/isci/task.c b/drivers/scsi/isci/task.c
index 80e1a69..6d8ff15 100644
--- a/drivers/scsi/isci/task.c
+++ b/drivers/scsi/isci/task.c
@@ -1570,9 +1570,6 @@ static int isci_reset_device(struct isci_host *ihost,
}
spin_unlock_irqrestore(&ihost->scic_lock, flags);
- /* Make sure all pending requests are able to be fully terminated. */
- isci_device_clear_reset_pending(ihost, idev);
-
/* If this is a device on an expander, disable BCN processing. */
if (!scsi_is_sas_phy_local(phy))
set_bit(IPORT_BCN_BLOCKED, &iport->flags);
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 09/11] isci: Fix hard reset timeout conditions.
2011-10-27 22:04 [PATCH 00/11] isci fixes: hotplug Dan Williams
` (7 preceding siblings ...)
2011-10-27 22:05 ` [PATCH 08/11] isci: No need to manage the pending reset bit on pending requests Dan Williams
@ 2011-10-27 22:05 ` Dan Williams
2011-10-27 22:05 ` [PATCH 10/11] isci: revert bcn filtering Dan Williams
2011-10-27 22:05 ` [PATCH 11/11] isci: overriding max_concurr_spinup oem parameter by max(oem, user) Dan Williams
10 siblings, 0 replies; 13+ messages in thread
From: Dan Williams @ 2011-10-27 22:05 UTC (permalink / raw)
To: linux-scsi; +Cc: Jeff Skirvin
From: Jeff Skirvin <jeffrey.d.skirvin@intel.com>
A hard reset can timeout before or after the last phy in the
port goes away. If after, then notify the OS that the last
phy has failed.
The recovery for the failed hard reset has been removed.
This recovery code was unecessary in that the link would
recover from the failure normally by a new link reset sequence
or hotplug of the remote device.
Signed-off-by: Jeff Skirvin <jeffrey.d.skirvin@intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
drivers/scsi/isci/port.c | 101 +++++++++++++++++++++++++++-------------------
drivers/scsi/isci/port.h | 1
2 files changed, 60 insertions(+), 42 deletions(-)
diff --git a/drivers/scsi/isci/port.c b/drivers/scsi/isci/port.c
index 8e59c88..bfeb879 100644
--- a/drivers/scsi/isci/port.c
+++ b/drivers/scsi/isci/port.c
@@ -350,6 +350,34 @@ static void isci_port_stop_complete(struct isci_host *ihost,
dev_dbg(&ihost->pdev->dev, "Port stop complete\n");
}
+
+static bool is_port_ready_state(enum sci_port_states state)
+{
+ switch (state) {
+ case SCI_PORT_READY:
+ case SCI_PORT_SUB_WAITING:
+ case SCI_PORT_SUB_OPERATIONAL:
+ case SCI_PORT_SUB_CONFIGURING:
+ return true;
+ default:
+ return false;
+ }
+}
+
+/* flag dummy rnc hanling when exiting a ready state */
+static void port_state_machine_change(struct isci_port *iport,
+ enum sci_port_states state)
+{
+ struct sci_base_state_machine *sm = &iport->sm;
+ enum sci_port_states old_state = sm->current_state_id;
+
+ if (is_port_ready_state(old_state) && !is_port_ready_state(state))
+ iport->ready_exit = true;
+
+ sci_change_state(sm, state);
+ iport->ready_exit = false;
+}
+
/**
* isci_port_hard_reset_complete() - This function is called by the sci core
* when the hard reset complete notification has been received.
@@ -368,6 +396,26 @@ static void isci_port_hard_reset_complete(struct isci_port *isci_port,
/* Save the status of the hard reset from the port. */
isci_port->hard_reset_status = completion_status;
+ if (completion_status != SCI_SUCCESS) {
+
+ /* The reset failed. The port state is now SCI_PORT_FAILED. */
+ if (isci_port->active_phy_mask == 0) {
+
+ /* Generate the link down now to the host, since it
+ * was intercepted by the hard reset state machine when
+ * it really happened.
+ */
+ isci_port_link_down(isci_port->isci_host,
+ &isci_port->isci_host->phys[
+ isci_port->last_active_phy],
+ isci_port);
+ }
+ /* Advance the port state so that link state changes will be
+ * noticed.
+ */
+ port_state_machine_change(isci_port, SCI_PORT_SUB_WAITING);
+
+ }
complete_all(&isci_port->hard_reset_complete);
}
@@ -657,6 +705,8 @@ void sci_port_deactivate_phy(struct isci_port *iport, struct isci_phy *iphy,
struct isci_host *ihost = iport->owning_controller;
iport->active_phy_mask &= ~(1 << iphy->phy_index);
+ if (!iport->active_phy_mask)
+ iport->last_active_phy = iphy->phy_index;
iphy->max_negotiated_speed = SAS_LINK_RATE_UNKNOWN;
@@ -683,33 +733,6 @@ static void sci_port_invalid_link_up(struct isci_port *iport, struct isci_phy *i
}
}
-static bool is_port_ready_state(enum sci_port_states state)
-{
- switch (state) {
- case SCI_PORT_READY:
- case SCI_PORT_SUB_WAITING:
- case SCI_PORT_SUB_OPERATIONAL:
- case SCI_PORT_SUB_CONFIGURING:
- return true;
- default:
- return false;
- }
-}
-
-/* flag dummy rnc hanling when exiting a ready state */
-static void port_state_machine_change(struct isci_port *iport,
- enum sci_port_states state)
-{
- struct sci_base_state_machine *sm = &iport->sm;
- enum sci_port_states old_state = sm->current_state_id;
-
- if (is_port_ready_state(old_state) && !is_port_ready_state(state))
- iport->ready_exit = true;
-
- sci_change_state(sm, state);
- iport->ready_exit = false;
-}
-
/**
* sci_port_general_link_up_handler - phy can be assigned to port?
* @sci_port: sci_port object for which has a phy that has gone link up.
@@ -1622,7 +1645,8 @@ void sci_port_construct(struct isci_port *iport, u8 index,
iport->logical_port_index = SCIC_SDS_DUMMY_PORT;
iport->physical_port_index = index;
iport->active_phy_mask = 0;
- iport->ready_exit = false;
+ iport->last_active_phy = 0;
+ iport->ready_exit = false;
iport->owning_controller = ihost;
@@ -1676,7 +1700,7 @@ int isci_port_perform_hard_reset(struct isci_host *ihost, struct isci_port *ipor
{
unsigned long flags;
enum sci_status status;
- int idx, ret = TMF_RESP_FUNC_COMPLETE;
+ int ret = TMF_RESP_FUNC_COMPLETE;
dev_dbg(&ihost->pdev->dev, "%s: iport = %p\n",
__func__, iport);
@@ -1697,8 +1721,13 @@ int isci_port_perform_hard_reset(struct isci_host *ihost, struct isci_port *ipor
"%s: iport = %p; hard reset completion\n",
__func__, iport);
- if (iport->hard_reset_status != SCI_SUCCESS)
+ if (iport->hard_reset_status != SCI_SUCCESS) {
ret = TMF_RESP_FUNC_FAILED;
+
+ dev_err(&ihost->pdev->dev,
+ "%s: iport = %p; hard reset failed (0x%x)\n",
+ __func__, iport, iport->hard_reset_status);
+ }
} else {
ret = TMF_RESP_FUNC_FAILED;
@@ -1718,18 +1747,6 @@ int isci_port_perform_hard_reset(struct isci_host *ihost, struct isci_port *ipor
"%s: iport = %p; hard reset failed "
"(0x%x) - driving explicit link fail for all phys\n",
__func__, iport, iport->hard_reset_status);
-
- /* Down all phys in the port. */
- spin_lock_irqsave(&ihost->scic_lock, flags);
- for (idx = 0; idx < SCI_MAX_PHYS; ++idx) {
- struct isci_phy *iphy = iport->phy_table[idx];
-
- if (!iphy)
- continue;
- sci_phy_stop(iphy);
- sci_phy_start(iphy);
- }
- spin_unlock_irqrestore(&ihost->scic_lock, flags);
}
return ret;
}
diff --git a/drivers/scsi/isci/port.h b/drivers/scsi/isci/port.h
index b50ecd4..e84d22a 100644
--- a/drivers/scsi/isci/port.h
+++ b/drivers/scsi/isci/port.h
@@ -109,6 +109,7 @@ struct isci_port {
u8 logical_port_index;
u8 physical_port_index;
u8 active_phy_mask;
+ u8 last_active_phy;
u16 reserved_rni;
u16 reserved_tag;
u32 started_request_count;
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 10/11] isci: revert bcn filtering
2011-10-27 22:04 [PATCH 00/11] isci fixes: hotplug Dan Williams
` (8 preceding siblings ...)
2011-10-27 22:05 ` [PATCH 09/11] isci: Fix hard reset timeout conditions Dan Williams
@ 2011-10-27 22:05 ` Dan Williams
2011-10-27 22:05 ` [PATCH 11/11] isci: overriding max_concurr_spinup oem parameter by max(oem, user) Dan Williams
10 siblings, 0 replies; 13+ messages in thread
From: Dan Williams @ 2011-10-27 22:05 UTC (permalink / raw)
To: linux-scsi
The initial bcn filtering implementation was validated on a kernel
baseline that predated the switch to new libata error handling. Also,
prior to that conversion we borrowed the mvsas MVS_DEV_EH approach to
prevent the unwanted extra ap->ops->phy_reset(ap) that occurred in the
ata_bus_probe() path.
After the conversion to new libata eh resets at discovery are more
frequent and get filtered prematurely by IDEV_EH. The result is that
our bcn filtering has been blocked from running and at discovery and it
appears to stall discovery completion to the point of triggering hung
task timeouts. So, revert the implementation for now. When it returns
it will go into libsas proper.
The domain rediscovery that takes place due to ->lldd_I_T_nexus_reset()
events should now be properly waited for by the ata_port_wait_eh() call
in ata_port_probe(). So the hard coded delay in the isci
->lldd_I_T_nexus_reset() and other libsas drivers should help debounce
the libsas thread from seeing temporary device removals.
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
drivers/scsi/isci/port.c | 45 +--------
drivers/scsi/isci/port.h | 5 -
drivers/scsi/isci/task.c | 235 ----------------------------------------------
3 files changed, 4 insertions(+), 281 deletions(-)
diff --git a/drivers/scsi/isci/port.c b/drivers/scsi/isci/port.c
index bfeb879..ac7f277 100644
--- a/drivers/scsi/isci/port.c
+++ b/drivers/scsi/isci/port.c
@@ -145,48 +145,15 @@ static void sci_port_bcn_enable(struct isci_port *iport)
}
}
-/* called under sci_lock to stabilize phy:port associations */
-void isci_port_bcn_enable(struct isci_host *ihost, struct isci_port *iport)
-{
- int i;
-
- clear_bit(IPORT_BCN_BLOCKED, &iport->flags);
- wake_up(&ihost->eventq);
-
- if (!test_and_clear_bit(IPORT_BCN_PENDING, &iport->flags))
- return;
-
- for (i = 0; i < ARRAY_SIZE(iport->phy_table); i++) {
- struct isci_phy *iphy = iport->phy_table[i];
-
- if (!iphy)
- continue;
-
- ihost->sas_ha.notify_port_event(&iphy->sas_phy,
- PORTE_BROADCAST_RCVD);
- break;
- }
-}
-
static void isci_port_bc_change_received(struct isci_host *ihost,
struct isci_port *iport,
struct isci_phy *iphy)
{
- if (iport && test_bit(IPORT_BCN_BLOCKED, &iport->flags)) {
- dev_dbg(&ihost->pdev->dev,
- "%s: disabled BCN; isci_phy = %p, sas_phy = %p\n",
- __func__, iphy, &iphy->sas_phy);
- set_bit(IPORT_BCN_PENDING, &iport->flags);
- atomic_inc(&iport->event);
- wake_up(&ihost->eventq);
- } else {
- dev_dbg(&ihost->pdev->dev,
- "%s: isci_phy = %p, sas_phy = %p\n",
- __func__, iphy, &iphy->sas_phy);
+ dev_dbg(&ihost->pdev->dev,
+ "%s: isci_phy = %p, sas_phy = %p\n",
+ __func__, iphy, &iphy->sas_phy);
- ihost->sas_ha.notify_port_event(&iphy->sas_phy,
- PORTE_BROADCAST_RCVD);
- }
+ ihost->sas_ha.notify_port_event(&iphy->sas_phy, PORTE_BROADCAST_RCVD);
sci_port_bcn_enable(iport);
}
@@ -278,9 +245,6 @@ static void isci_port_link_down(struct isci_host *isci_host,
/* check to see if this is the last phy on this port. */
if (isci_phy->sas_phy.port &&
isci_phy->sas_phy.port->num_phys == 1) {
- atomic_inc(&isci_port->event);
- isci_port_bcn_enable(isci_host, isci_port);
-
/* change the state for all devices on this port. The
* next task sent to this device will be returned as
* SAS_TASK_UNDELIVERED, and the scsi mid layer will
@@ -1672,7 +1636,6 @@ void isci_port_init(struct isci_port *iport, struct isci_host *ihost, int index)
init_completion(&iport->start_complete);
iport->isci_host = ihost;
isci_port_change_state(iport, isci_freed);
- atomic_set(&iport->event, 0);
}
/**
diff --git a/drivers/scsi/isci/port.h b/drivers/scsi/isci/port.h
index e84d22a..cb5ffbc 100644
--- a/drivers/scsi/isci/port.h
+++ b/drivers/scsi/isci/port.h
@@ -77,7 +77,6 @@ enum isci_status {
/**
* struct isci_port - isci direct attached sas port object
- * @event: counts bcns and port stop events (for bcn filtering)
* @ready_exit: several states constitute 'ready'. When exiting ready we
* need to take extra port-teardown actions that are
* skipped when exiting to another 'ready' state.
@@ -92,10 +91,6 @@ enum isci_status {
*/
struct isci_port {
enum isci_status status;
- #define IPORT_BCN_BLOCKED 0
- #define IPORT_BCN_PENDING 1
- unsigned long flags;
- atomic_t event;
struct isci_host *isci_host;
struct asd_sas_port sas_port;
struct list_head remote_dev_list;
diff --git a/drivers/scsi/isci/task.c b/drivers/scsi/isci/task.c
index 6d8ff15..66ad3dc 100644
--- a/drivers/scsi/isci/task.c
+++ b/drivers/scsi/isci/task.c
@@ -1331,226 +1331,10 @@ isci_task_request_complete(struct isci_host *ihost,
complete(tmf_complete);
}
-static void isci_smp_task_timedout(unsigned long _task)
-{
- struct sas_task *task = (void *) _task;
- unsigned long flags;
-
- spin_lock_irqsave(&task->task_state_lock, flags);
- if (!(task->task_state_flags & SAS_TASK_STATE_DONE))
- task->task_state_flags |= SAS_TASK_STATE_ABORTED;
- spin_unlock_irqrestore(&task->task_state_lock, flags);
-
- complete(&task->completion);
-}
-
-static void isci_smp_task_done(struct sas_task *task)
-{
- if (!del_timer(&task->timer))
- return;
- complete(&task->completion);
-}
-
-static int isci_smp_execute_task(struct isci_host *ihost,
- struct domain_device *dev, void *req,
- int req_size, void *resp, int resp_size)
-{
- int res, retry;
- struct sas_task *task = NULL;
-
- for (retry = 0; retry < 3; retry++) {
- task = sas_alloc_task(GFP_KERNEL);
- if (!task)
- return -ENOMEM;
-
- task->dev = dev;
- task->task_proto = dev->tproto;
- sg_init_one(&task->smp_task.smp_req, req, req_size);
- sg_init_one(&task->smp_task.smp_resp, resp, resp_size);
-
- task->task_done = isci_smp_task_done;
-
- task->timer.data = (unsigned long) task;
- task->timer.function = isci_smp_task_timedout;
- task->timer.expires = jiffies + 10*HZ;
- add_timer(&task->timer);
-
- res = isci_task_execute_task(task, 1, GFP_KERNEL);
-
- if (res) {
- del_timer(&task->timer);
- dev_dbg(&ihost->pdev->dev,
- "%s: executing SMP task failed:%d\n",
- __func__, res);
- goto ex_err;
- }
-
- wait_for_completion(&task->completion);
- res = -ECOMM;
- if ((task->task_state_flags & SAS_TASK_STATE_ABORTED)) {
- dev_dbg(&ihost->pdev->dev,
- "%s: smp task timed out or aborted\n",
- __func__);
- isci_task_abort_task(task);
- if (!(task->task_state_flags & SAS_TASK_STATE_DONE)) {
- dev_dbg(&ihost->pdev->dev,
- "%s: SMP task aborted and not done\n",
- __func__);
- goto ex_err;
- }
- }
- if (task->task_status.resp == SAS_TASK_COMPLETE &&
- task->task_status.stat == SAM_STAT_GOOD) {
- res = 0;
- break;
- }
- if (task->task_status.resp == SAS_TASK_COMPLETE &&
- task->task_status.stat == SAS_DATA_UNDERRUN) {
- /* no error, but return the number of bytes of
- * underrun */
- res = task->task_status.residual;
- break;
- }
- if (task->task_status.resp == SAS_TASK_COMPLETE &&
- task->task_status.stat == SAS_DATA_OVERRUN) {
- res = -EMSGSIZE;
- break;
- } else {
- dev_dbg(&ihost->pdev->dev,
- "%s: task to dev %016llx response: 0x%x "
- "status 0x%x\n", __func__,
- SAS_ADDR(dev->sas_addr),
- task->task_status.resp,
- task->task_status.stat);
- sas_free_task(task);
- task = NULL;
- }
- }
-ex_err:
- BUG_ON(retry == 3 && task != NULL);
- sas_free_task(task);
- return res;
-}
-
-#define DISCOVER_REQ_SIZE 16
-#define DISCOVER_RESP_SIZE 56
-
-int isci_smp_get_phy_attached_dev_type(struct isci_host *ihost,
- struct domain_device *dev,
- int phy_id, int *adt)
-{
- struct smp_resp *disc_resp;
- u8 *disc_req;
- int res;
-
- disc_resp = kzalloc(DISCOVER_RESP_SIZE, GFP_KERNEL);
- if (!disc_resp)
- return -ENOMEM;
-
- disc_req = kzalloc(DISCOVER_REQ_SIZE, GFP_KERNEL);
- if (disc_req) {
- disc_req[0] = SMP_REQUEST;
- disc_req[1] = SMP_DISCOVER;
- disc_req[9] = phy_id;
- } else {
- kfree(disc_resp);
- return -ENOMEM;
- }
- res = isci_smp_execute_task(ihost, dev, disc_req, DISCOVER_REQ_SIZE,
- disc_resp, DISCOVER_RESP_SIZE);
- if (!res) {
- if (disc_resp->result != SMP_RESP_FUNC_ACC)
- res = disc_resp->result;
- else
- *adt = disc_resp->disc.attached_dev_type;
- }
- kfree(disc_req);
- kfree(disc_resp);
-
- return res;
-}
-
-static void isci_wait_for_smp_phy_reset(struct isci_remote_device *idev, int phy_num)
-{
- struct domain_device *dev = idev->domain_dev;
- struct isci_port *iport = idev->isci_port;
- struct isci_host *ihost = iport->isci_host;
- int res, iteration = 0, attached_device_type;
- #define STP_WAIT_MSECS 25000
- unsigned long tmo = msecs_to_jiffies(STP_WAIT_MSECS);
- unsigned long deadline = jiffies + tmo;
- enum {
- SMP_PHYWAIT_PHYDOWN,
- SMP_PHYWAIT_PHYUP,
- SMP_PHYWAIT_DONE
- } phy_state = SMP_PHYWAIT_PHYDOWN;
-
- /* While there is time, wait for the phy to go away and come back */
- while (time_is_after_jiffies(deadline) && phy_state != SMP_PHYWAIT_DONE) {
- int event = atomic_read(&iport->event);
-
- ++iteration;
-
- tmo = wait_event_timeout(ihost->eventq,
- event != atomic_read(&iport->event) ||
- !test_bit(IPORT_BCN_BLOCKED, &iport->flags),
- tmo);
- /* link down, stop polling */
- if (!test_bit(IPORT_BCN_BLOCKED, &iport->flags))
- break;
-
- dev_dbg(&ihost->pdev->dev,
- "%s: iport %p, iteration %d,"
- " phase %d: time_remaining %lu, bcns = %d\n",
- __func__, iport, iteration, phy_state,
- tmo, test_bit(IPORT_BCN_PENDING, &iport->flags));
-
- res = isci_smp_get_phy_attached_dev_type(ihost, dev, phy_num,
- &attached_device_type);
- tmo = deadline - jiffies;
-
- if (res) {
- dev_dbg(&ihost->pdev->dev,
- "%s: iteration %d, phase %d:"
- " SMP error=%d, time_remaining=%lu\n",
- __func__, iteration, phy_state, res, tmo);
- break;
- }
- dev_dbg(&ihost->pdev->dev,
- "%s: iport %p, iteration %d,"
- " phase %d: time_remaining %lu, bcns = %d, "
- "attdevtype = %x\n",
- __func__, iport, iteration, phy_state,
- tmo, test_bit(IPORT_BCN_PENDING, &iport->flags),
- attached_device_type);
-
- switch (phy_state) {
- case SMP_PHYWAIT_PHYDOWN:
- /* Has the device gone away? */
- if (!attached_device_type)
- phy_state = SMP_PHYWAIT_PHYUP;
-
- break;
-
- case SMP_PHYWAIT_PHYUP:
- /* Has the device come back? */
- if (attached_device_type)
- phy_state = SMP_PHYWAIT_DONE;
- break;
-
- case SMP_PHYWAIT_DONE:
- break;
- }
-
- }
- dev_dbg(&ihost->pdev->dev, "%s: done\n", __func__);
-}
-
static int isci_reset_device(struct isci_host *ihost,
struct isci_remote_device *idev)
{
struct sas_phy *phy = sas_find_local_phy(idev->domain_dev);
- struct isci_port *iport = idev->isci_port;
enum sci_status status;
unsigned long flags;
int rc;
@@ -1570,10 +1354,6 @@ static int isci_reset_device(struct isci_host *ihost,
}
spin_unlock_irqrestore(&ihost->scic_lock, flags);
- /* If this is a device on an expander, disable BCN processing. */
- if (!scsi_is_sas_phy_local(phy))
- set_bit(IPORT_BCN_BLOCKED, &iport->flags);
-
rc = sas_phy_reset(phy, true);
/* Terminate in-progress I/O now. */
@@ -1584,21 +1364,6 @@ static int isci_reset_device(struct isci_host *ihost,
status = sci_remote_device_reset_complete(idev);
spin_unlock_irqrestore(&ihost->scic_lock, flags);
- /* If this is a device on an expander, bring the phy back up. */
- if (!scsi_is_sas_phy_local(phy)) {
- /* A phy reset will cause the device to go away then reappear.
- * Since libsas will take action on incoming BCNs (eg. remove
- * a device going through an SMP phy-control driven reset),
- * we need to wait until the phy comes back up before letting
- * discovery proceed in libsas.
- */
- isci_wait_for_smp_phy_reset(idev, phy->number);
-
- spin_lock_irqsave(&ihost->scic_lock, flags);
- isci_port_bcn_enable(ihost, idev->isci_port);
- spin_unlock_irqrestore(&ihost->scic_lock, flags);
- }
-
if (status != SCI_SUCCESS) {
dev_dbg(&ihost->pdev->dev,
"%s: sci_remote_device_reset_complete(%p) "
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 11/11] isci: overriding max_concurr_spinup oem parameter by max(oem, user)
2011-10-27 22:04 [PATCH 00/11] isci fixes: hotplug Dan Williams
` (9 preceding siblings ...)
2011-10-27 22:05 ` [PATCH 10/11] isci: revert bcn filtering Dan Williams
@ 2011-10-27 22:05 ` Dan Williams
10 siblings, 0 replies; 13+ messages in thread
From: Dan Williams @ 2011-10-27 22:05 UTC (permalink / raw)
To: linux-scsi; +Cc: Andrzej Jakowski
From: Andrzej Jakowski <andrzej.jakowski@intel.com>
Fixes bug where max_concurr_spinup oem parameter should be
overriden by max_concurr_spinup user parameter. Override should
happen only when max_concurr_spinup user parameter is specified
in command line (greater than 0). Also this fix shortens variables
representing max_conxurr_spinup for oem and user parameters.
Signed-off-by: Andrzej Jakowski <andrzej.jakowski@intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
drivers/scsi/isci/host.c | 23 ++++++++++++++++-------
drivers/scsi/isci/init.c | 2 +-
drivers/scsi/isci/probe_roms.h | 4 ++--
3 files changed, 19 insertions(+), 10 deletions(-)
diff --git a/drivers/scsi/isci/host.c b/drivers/scsi/isci/host.c
index f07f30f..e7fe9c4 100644
--- a/drivers/scsi/isci/host.c
+++ b/drivers/scsi/isci/host.c
@@ -1350,7 +1350,7 @@ static void isci_user_parameters_get(struct sci_user_parameters *u)
u->stp_max_occupancy_timeout = stp_max_occ_to;
u->ssp_max_occupancy_timeout = ssp_max_occ_to;
u->no_outbound_task_timeout = no_outbound_task_to;
- u->max_number_concurrent_device_spin_up = max_concurr_spinup;
+ u->max_concurr_spinup = max_concurr_spinup;
}
static void sci_controller_initial_state_enter(struct sci_base_state_machine *sm)
@@ -1661,7 +1661,7 @@ static void sci_controller_set_default_config_parameters(struct isci_host *ihost
ihost->oem_parameters.controller.mode_type = SCIC_PORT_AUTOMATIC_CONFIGURATION_MODE;
/* Default to APC mode. */
- ihost->oem_parameters.controller.max_concurrent_dev_spin_up = 1;
+ ihost->oem_parameters.controller.max_concurr_spin_up = 1;
/* Default to no SSC operation. */
ihost->oem_parameters.controller.do_enable_ssc = false;
@@ -1787,7 +1787,8 @@ int sci_oem_parameters_validate(struct sci_oem_params *oem)
} else
return -EINVAL;
- if (oem->controller.max_concurrent_dev_spin_up > MAX_CONCURRENT_DEVICE_SPIN_UP_COUNT)
+ if (oem->controller.max_concurr_spin_up > MAX_CONCURRENT_DEVICE_SPIN_UP_COUNT ||
+ oem->controller.max_concurr_spin_up < 1)
return -EINVAL;
return 0;
@@ -1810,6 +1811,16 @@ static enum sci_status sci_oem_parameters_set(struct isci_host *ihost)
return SCI_FAILURE_INVALID_STATE;
}
+static u8 max_spin_up(struct isci_host *ihost)
+{
+ if (ihost->user_parameters.max_concurr_spinup)
+ return min_t(u8, ihost->user_parameters.max_concurr_spinup,
+ MAX_CONCURRENT_DEVICE_SPIN_UP_COUNT);
+ else
+ return min_t(u8, ihost->oem_parameters.controller.max_concurr_spin_up,
+ MAX_CONCURRENT_DEVICE_SPIN_UP_COUNT);
+}
+
static void power_control_timeout(unsigned long data)
{
struct sci_timer *tmr = (struct sci_timer *)data;
@@ -1839,8 +1850,7 @@ static void power_control_timeout(unsigned long data)
if (iphy == NULL)
continue;
- if (ihost->power_control.phys_granted_power >=
- ihost->oem_parameters.controller.max_concurrent_dev_spin_up)
+ if (ihost->power_control.phys_granted_power >= max_spin_up(ihost))
break;
ihost->power_control.requesters[i] = NULL;
@@ -1865,8 +1875,7 @@ void sci_controller_power_control_queue_insert(struct isci_host *ihost,
{
BUG_ON(iphy == NULL);
- if (ihost->power_control.phys_granted_power <
- ihost->oem_parameters.controller.max_concurrent_dev_spin_up) {
+ if (ihost->power_control.phys_granted_power < max_spin_up(ihost)) {
ihost->power_control.phys_granted_power++;
sci_phy_consume_power_handler(iphy);
diff --git a/drivers/scsi/isci/init.c b/drivers/scsi/isci/init.c
index 43fe840..a97edab 100644
--- a/drivers/scsi/isci/init.c
+++ b/drivers/scsi/isci/init.c
@@ -118,7 +118,7 @@ unsigned char phy_gen = 3;
module_param(phy_gen, byte, 0);
MODULE_PARM_DESC(phy_gen, "PHY generation (1: 1.5Gbps 2: 3.0Gbps 3: 6.0Gbps)");
-unsigned char max_concurr_spinup = 1;
+unsigned char max_concurr_spinup;
module_param(max_concurr_spinup, byte, 0);
MODULE_PARM_DESC(max_concurr_spinup, "Max concurrent device spinup");
diff --git a/drivers/scsi/isci/probe_roms.h b/drivers/scsi/isci/probe_roms.h
index dc007e6..2c75248 100644
--- a/drivers/scsi/isci/probe_roms.h
+++ b/drivers/scsi/isci/probe_roms.h
@@ -112,7 +112,7 @@ struct sci_user_parameters {
* This field specifies the maximum number of direct attached devices
* that can have power supplied to them simultaneously.
*/
- u8 max_number_concurrent_device_spin_up;
+ u8 max_concurr_spinup;
/**
* This field specifies the number of seconds to allow a phy to consume
@@ -219,7 +219,7 @@ struct sci_bios_oem_param_block_hdr {
struct sci_oem_params {
struct {
uint8_t mode_type;
- uint8_t max_concurrent_dev_spin_up;
+ uint8_t max_concurr_spin_up;
uint8_t do_enable_ssc;
uint8_t reserved;
} controller;
^ permalink raw reply related [flat|nested] 13+ messages in thread