From 0e2e27990e2dcd415f7974e8460a2f05accdddfb Mon Sep 17 00:00:00 2001 From: Jeff Skirvin Date: Thu, 27 Oct 2011 15:04:50 -0700 Subject: [SCSI] isci: Lookup device references through requests in completions. 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 [] 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:[] I/O error on dev [] 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] [ 229.214401] [] sci_change_state+0x4a/0x4f [isci] [ 229.214401] [] sci_io_request_tc_completion+0x79c/0x7a0 [isci] [ 229.214401] [] sci_controller_process_completions+0x14f/0x396 [isci] [ 229.214401] [] ? spin_lock_irq+0xe/0x10 [isci] [ 229.214401] [] isci_host_completion_routine+0x71/0x2be [isci] [ 229.214401] [] ? mark_held_locks+0x52/0x70 [ 229.214401] [] tasklet_action+0x90/0xf1 [ 229.214401] [] __do_softirq+0xe5/0x1bf [ 229.214401] [] ? hrtimer_interrupt+0x129/0x1bb [ 229.214401] [] call_softirq+0x1c/0x30 [ 229.214401] [] do_softirq+0x4b/0xa3 [ 229.214401] [] irq_exit+0x53/0xb4 [ 229.214401] [] smp_apic_timer_interrupt+0x83/0x91 [ 229.214401] [] apic_timer_interrupt+0x13/0x20 [ 229.214401] [ 229.214401] [] ? retint_restore_args+0x13/0x13 [ 229.214401] [] ? trace_hardirqs_off+0xd/0xf [ 229.214401] [] ? vprintk+0x40b/0x452 [ 229.214401] [] printk+0x41/0x47 [ 229.214401] [] __dev_printk+0x78/0x7a [ 229.214401] [] dev_printk+0x45/0x47 [ 229.214401] [] isci_terminate_request_core+0x15d/0x317 [isci] [ 229.214401] [] isci_terminate_pending_requests+0x1a4/0x204 [isci] [ 229.214401] [] ? sas_phye_oob_error+0xc3/0xc3 [libsas] [ 229.214401] [] isci_remote_device_nuke_requests+0xa6/0xff [isci] [ 229.214401] [] isci_remote_device_stop+0x7c/0x166 [isci] [ 229.214401] [] ? sas_phye_oob_error+0xc3/0xc3 [libsas] [ 229.214401] [] isci_remote_device_gone+0x76/0x7e [isci] [ 229.214401] [] sas_notify_lldd_dev_gone+0x34/0x36 [libsas] [ 229.214401] [] sas_unregister_dev+0x57/0x9c [libsas] [ 229.214401] [] sas_unregister_domain_devices+0x36/0x65 [libsas] [ 229.214401] [] sas_deform_port+0x72/0x1ac [libsas] [ 229.214401] [] ? sas_phye_oob_error+0xc3/0xc3 [libsas] [ 229.214401] [] sas_phye_loss_of_signal+0x3e/0x42 [libsas] Signed-off-by: Jeff Skirvin Signed-off-by: Dan Williams Signed-off-by: James Bottomley --- drivers/scsi/isci/request.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) (limited to 'drivers/scsi/isci/request.c') diff --git a/drivers/scsi/isci/request.c b/drivers/scsi/isci/request.c index 565a9f0a9bc2..bfc7379727b1 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 -- cgit v1.2.3 From 3b34c169f8197e02529fa3ec703703c2ce418c57 Mon Sep 17 00:00:00 2001 From: Jeff Skirvin Date: Thu, 27 Oct 2011 15:05:22 -0700 Subject: [SCSI] isci: Remove redundant isci_request.ttype field. Use the existing IREQ_TMF flag as a request type indicator. Signed-off-by: Jeff Skirvin Signed-off-by: Dan Williams Signed-off-by: James Bottomley --- 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(-) (limited to 'drivers/scsi/isci/request.c') diff --git a/drivers/scsi/isci/remote_device.c b/drivers/scsi/isci/remote_device.c index 20c77edd4711..9d9e33d2ed51 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 bfc7379727b1..192cb48d849a 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 f720b97b7bb5..be38933dd6df 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 ec85beb1f979..80e1a69ac96f 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: -- cgit v1.2.3