Re: [PATCH v6 2/2] ufs: core: requeue aborted request

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, 2024-09-12 at 14:11 -0700, Bart Van Assche wrote:
>  	 
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
>  On 9/12/24 6:49 AM, Peter Wang (王信友) wrote:
> > On Wed, 2024-09-11 at 12:37 -0700, Bart Van Assche wrote:
> [ ... ]
> > So at this time, the device will not have a corresponding
> > response coming back. The host controller will automatically
> > fill in the response for the corresponding command based on
> > the results of SQ cleanup (MCQ) or UTRLCLR (DBR, mediatek),
> > with the COS content being ABORTED by interrupt.
> 
> I don't think so. In SDB mode, writing into the UTRLCLR register does
> NOT cause the ABORTED status to be written into the OCS field. Even
> if
> there would be UFSHCI controllers that do this, the UFSHCI
> specification
> does not require this behavior and hence the UFS driver should not
> assume this behavior.
> 

Hi Bart,

Okay, I will add MCQ check only set this flag in MCQ mode.


> >> The above change will cause lrbp->abort_initiated_by_eh to be set
> not
> >> only if the UFS error handler decides to abort a command but also
> if
> >> the
> >> SCSI core decides to abort a command. I think this is wrong.
> > 
> > Sorry, I might have missed something, but I didn't see
> > scsi abort (ufshcd_abort) calling ufshcd_set_eh_in_progress.
> > So, during a SCSI abort, ufshcd_eh_in_progress(hba)
> > should return false and not set this flag, right?
> 
> I think you are right so please ignore my comment above.
> 
> > Additionally, SCSI abort (ufshcd_abort) will have different
> > return values for MCQ mode and DBR mode when the device
> > does not respond with a response.
> > MCQ mode will receivce OCS_ABORTED (nullify)
> > case OCS_ABORTED:
> > result |= DID_ABORT << 16;
> > break;
> 
> No. ufshcd_abort() submits an abort TMF and the OCS status is not
> modified if the abort TMF succeeds.
> 

Yes, TMF won't update the OCS status.
MCQ mode update OCS status by 
1. nullify: ufshcd_mcq_nullify_sqe
2. sq cleanup: ufshcd_mcq_sq_cleanup
In both case, host will fills OCS with ABORTED.



> > But DBR mode, OCS won't change, it is 0x0F
> > case OCS_INVALID_COMMAND_STATUS:
> > result |= DID_REQUEUE << 16;
> > break;
> > In this case, should we also return DID_ABORT for DBR mode?
> 
> The above code comes from ufshcd_transfer_rsp_status(), isn't it?
> ufshcd_transfer_rsp_status() should not be called if the SCSI core
> aborts a SCSI command (ufshcd_abort()). It is not allowed to call
> scsi_done() from a SCSI abort handler like ufshcd_abort(). SCSI
> abort handlers must return SUCCESS, FAILED or FAST_IO_FAIL and let
> the SCSI core decide whether to complete or whether to resubmit the
> SCSI command.
> 


Yes, it is from ufshcd_transfer_rsp_status, another ISR process.

Regarding the specification of UTRLDBR:
When a transfer request is "completed" (with success or error),
the corresponding bit is cleared to '0' by the host controller.

ufshcd_abort is error case that host controller clear UTRLDBR 
tag bit by UTRLCLR.

Regarding the specification of UTRCS:
This bit is set to '1' by the host controller upon one of the
following:
	Completion of a UTP transfer request with its UTRD
	Interrupt bit set to "1"
	Overall command Status (OCS) of the completed command is not 
	equal to 'SUCCESS' even if its UTRD Interrupt bit set to '0'

So, this case is transfer request is "completed" with error.
Whether the OCS is 'ABORTED' or 'INVALID_OCS_VALUE'
We should always receive an interrupt upon completion 
becasue UTRCS will set to '1'


> Thanks,
> 
> Bart.




[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Device Mapper]

  Powered by Linux