[openib-general][patch review] srp: fmr implementation,

[Vu Pham]

Roland Dreier wrote:
>     Vu> Have you read scsi_eh_try_stu(scmnd) and scsi_eh_tur(scmnd)?
>     Vu> These functions use the same scmnd and reformat it with new
>     Vu> cdb and call srp_queuecommand() which uses new req and put
>     Vu> this new req in request queue for this same scmnd with
>     Vu> different cdb
> 
> Yes, but why are there any commands in the work_q at all?  In other
> words, why is this loop entered at all?
> 
>  			list_for_each_entry_safe(scmd, next, work_q, eh_entry) {
>  				if (!scsi_device_online(scmd->device) ||
>  				    (!scsi_eh_try_stu(scmd) && !scsi_eh_tur(scmd)) ||
>  				    !scsi_eh_tur(scmd))
>  					scsi_eh_finish_cmd(scmd, done_q);
>  			}
> 


The command is removed from work_q in scsi_eh_finish_cmd() 
if scsi_eh_abort_cmds() or scsi_eh_bus_device_reset() or 
scsi_eh_host_reset() return SUCCESS. This require at least 
one scsi_eh_tur or stu has to be successfully sent.

In our case we failed scsi_eh_abort_cmds() and 
scsi_eh_bus_device_reset(). Therefore, if we enter 
scsi_eh_host_reset with 2 outstanding scmnd in work_q, we 
end up in this loop.

> srp_reconnect_target() should get rid of all queued commands already:
> 
> 	list_for_each_entry(req, &target->req_queue, list) {
> 		req->scmnd->result = DID_RESET << 16;
> 		req->scmnd->scsi_done(req->scmnd);
> 		srp_unmap_data(req->scmnd, target, req);
> 	}
> 
> why does the midlayer have any commands around after that loop?
> 


scmnd->scsi_done() does not guarantee the scmnd is removed 
from work_q of scsi midlayer in error handling flow. 
Moreover scmnd may be reformat by scsi milayer to send stu 
or tur command if any and scmnd->scsi_done is already 
changed to scsi_eh_done() by srp.