On 2020-07-30 18:30, Stanley Chu wrote: > On Mon, 2020-07-27 at 11:18 +0000, Avri Altman wrote: >> Looks good to me. >> But better wait and see if Bart have any further reservations. > > Would you have any further suggestions? Today is the first time that I took a look at ufshcd_abort(). The approach of that function looks wrong to me. This is how I think that a SCSI LLD abort handler should work: (1) Serialize against the completion path (__ufshcd_transfer_req_compl()) such that it cannot happen that the abort handler and the regular completion path both call cmd->scsi_done(cmd) at the same time. I'm not sure whether an existing synchronization object can be used for this purpose or whether a new synchronization object has to be introduced to serialize scsi_done() calls from __ufshcd_transfer_req_compl() and ufshcd_abort(). (2) While holding that synchronization object, check whether the SCSI command is still outstanding. If so, submit a SCSI abort TMR to the device. (3) If the command has been aborted, call scsi_done() and return SUCCESS. If aborting failed and the command is still in progress, return FAILED. An example is available in srp_abort() in drivers/infiniband/ulp/srp/ib_srp.c. Bart.
