Re: [PATCH v11 6/9] Make scsi_remove_host() wait until error handling finished

[Bart Van Assche <bvanassche@acm.org>]

On 06/24/13 03:15, Mike Christie wrote:
> On 6/12/13 7:55 AM, Bart Van Assche wrote:
>> A SCSI LLD may start cleaning up host resources as soon as
>> scsi_remove_host() returns. These host resources may be needed by
>> the LLD in an implementation of one of the eh_* functions. So if
>> one of the eh_* functions is in progress when scsi_remove_host()
>> is invoked, wait until the eh_* function has finished. Also, do
>> not invoke any of the eh_* functions after scsi_remove_host() has
>> started. Remove Scsi_Host.tmf_in_progress because it is now
>> superfluous.
> 
> I think the patch looks ok for drivers that do not implement their own
> eh_strategy_handler, but what about SAS? If you added a scsi_begin_eh in 
> scsi_error_handler before the eh_strategy_handler is called and then add 
> a scsi_end_eh after it is called, I think it would cover them too.

I will start testing the modification below for the patch at the start of
this thread:

--- a/drivers/scsi/scsi_error.c
+++ b/drivers/scsi/scsi_error.c
@@ -1950,10 +1950,14 @@ int scsi_error_handler(void *data)
                        continue;
                }
 
-               if (shost->transportt->eh_strategy_handler)
-                       shost->transportt->eh_strategy_handler(shost);
-               else
+               if (shost->transportt->eh_strategy_handler) {
+                       if (scsi_begin_eh(shost) == 0) {
+                               shost->transportt->eh_strategy_handler(shost);
+                               scsi_end_eh(shost);
+                       }
+               } else {
                        scsi_unjam_host(shost);
+               }
 
                /*
                 * Note - if the above fails completely, the action is to take


>> @@ -1894,6 +1962,9 @@ int scsi_error_handler(void *data)
>>       }
>>       __set_current_state(TASK_RUNNING);
>>
>> +    WARN_ONCE(shost->eh_active, "scsi_eh_%d: eh_active = %d\n",
>> +          shost->host_no, shost->eh_active);
>> +
>>       SCSI_LOG_ERROR_RECOVERY(1,
>>           printk("Error handler scsi_eh_%d exiting\n", shost->host_no));
>>       shost->ehandler = NULL;
> 
> What is the warn for? Is there a chance this can happen with some non 
> upstream driver or are you just adding it just in case?

This is code that helped me to test this patch. I can leave it out if
you prefer so.

Bart.