>-----Original Message-----
>From: Hannes Reinecke [mailto:h...@suse.de]
>Sent: Monday, October 17, 2016 5:01 PM
>To: Sumit Saxena; linux-scsi@vger.kernel.org
>Cc: martin.peter...@oracle.com; the...@redhat.com;
j...@linux.vnet.ibm.com;
>kashyap.de...@broadcom.com
>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.sax...@broadcom.com>
>> ---
>>  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
>h...@suse.de                                  +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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to