>-----Original Message----- >From: Hannes Reinecke [mailto:hare@xxxxxxx] >Sent: Monday, October 17, 2016 5:01 PM >To: Sumit Saxena; linux-scsi@xxxxxxxxxxxxxxx >Cc: martin.petersen@xxxxxxxxxx; thenzl@xxxxxxxxxx; jejb@xxxxxxxxxxxxxxxxxx; >kashyap.desai@xxxxxxxxxxxx >Subject: Re: [PATCH 3/7] megaraid_sas: Do not fire DCMDs during PCI >shutdown/detach > >On 10/17/2016 12:24 PM, Sumit Saxena wrote: >> This patch addresses the issue of driver firing DCMDs in PCI >> shutdown/detach path irrespective of firmware state. >> Driver will check for whether firmware is operational state or not >> before firing DCMDs. If firmware is in unrecoverbale state or does not >> become operational within specfied time, driver will skip firing >> DCMDs. >> >> Signed-off-by: Sumit Saxena <sumit.saxena@xxxxxxxxxxxx> >> --- >> drivers/scsi/megaraid/megaraid_sas_base.c | 46 >+++++++++++++++++++++++++++++ >> drivers/scsi/megaraid/megaraid_sas_fusion.c | 9 ++++-- >> 2 files changed, 52 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c >> b/drivers/scsi/megaraid/megaraid_sas_base.c >> index 9ff57de..092387f 100644 >> --- a/drivers/scsi/megaraid/megaraid_sas_base.c >> +++ b/drivers/scsi/megaraid/megaraid_sas_base.c >> @@ -6248,6 +6248,32 @@ fail_reenable_msix: >> #define megasas_resume NULL >> #endif >> >> +static inline int >> +megasas_wait_for_adapter_operational(struct megasas_instance >> +*instance) { >> + int wait_time = MEGASAS_RESET_WAIT_TIME * 2; >> + int i; >> + >> + for (i = 0; i < wait_time; i++) { >> + if (atomic_read(&instance->adprecovery) >> + == MEGASAS_HBA_OPERATIONAL) >> + break; >> + >> + if (!(i % MEGASAS_RESET_NOTICE_INTERVAL)) >> + dev_notice(&instance->pdev->dev, "waiting for >controller reset to >> +finish\n"); >> + >> + msleep(1000); >> + } >> + >> + if (atomic_read(&instance->adprecovery) != >MEGASAS_HBA_OPERATIONAL) { >> + dev_info(&instance->pdev->dev, "%s timed out while waiting for >HBA to recover.\n", >> + __func__); >> + return 1; >> + } >> + >> + return 0; >> +} >> + >> /** >> * megasas_detach_one - PCI hot"un"plug entry point >> * @pdev: PCI device structure >> @@ -6272,9 +6298,18 @@ static void megasas_detach_one(struct pci_dev >*pdev) >> if (instance->fw_crash_state != UNAVAILABLE) >> megasas_free_host_crash_buffer(instance); >> scsi_remove_host(instance->host); >> + >> + msleep(1000); >> + if ((atomic_read(&instance->adprecovery) == >MEGASAS_HW_CRITICAL_ERROR) >> + || ((atomic_read(&instance->adprecovery) >> + != MEGASAS_HBA_OPERATIONAL) && >> + megasas_wait_for_adapter_operational(instance))) >> + goto skip_firing_dcmds; >> + >> megasas_flush_cache(instance); >> megasas_shutdown_controller(instance, >MR_DCMD_CTRL_SHUTDOWN); >> >> +skip_firing_dcmds: >> /* cancel the delayed work if this work still in queue*/ >> if (instance->ev != NULL) { >> struct megasas_aen_event *ev = instance->ev; >Why not move the 'msleep' and 'atomic_read' into the call to >megasas_wait_for_adapter_operational? I will rectify this in next version of this patch. >Plus I'm not sure if the unconditional 'msleep' is a good idea here; maybe one >should read 'adprecovery' first and _then_ call msleep if required? Agreed.. I will cleanup this and resend the patch. Thanks, Sumit > >Cheers, > >Hannes >-- >Dr. Hannes Reinecke Teamlead Storage & Networking >hare@xxxxxxx +49 911 74053 688 >SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg >GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton HRB 21284 (AG >Nürnberg) -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
