On Fri, Aug 21, 2020 at 01:20:44PM -0700, Sagi Grimberg wrote: >>> Many functions which call __nvme_submit_sync_cmd treat error code in two >>> modes: If error code less than 0, treat as command failed. If erroe code >>> more than 0, treat as target not support or other and continue. >>> NVME_SC_HOST_ABORTED_CMD and NVME_SC_HOST_PATH_ERROR both are cancled io >>> by host, is not the real error code return from target. So we need set >>> the flag:NVME_REQ_CANCELLED. Thus __nvme_submit_sync_cmd translate >>> the error to INTR, nvme_set_queue_count will return error, reconnect >>> process will terminate instead of continue. >> >> But we could still race with a real completion. I suspect the right >> answer is to translate NVME_SC_HOST_ABORTED_CMD and >> NVME_SC_HOST_PATH_ERROR to a negative error code in >> __nvme_submit_sync_cmd. > > So the scheme you suggest is: > - treat any negative status or !DNR as "we never made it to > the target" > - Any positive status with DNR is a "controller generated status" > > This will need a careful audit of all the call-sites we place such > assumptions... No. negative error means never made it to the controller, and we need to map the magic NVME_SC_HOST_ABORTED_CMD and NVME_SC_HOST_PATH_ERROR errors to negative error codes if we want to keep using them (and IIRC we started because it solved an issue, by my memory is foggy). All the real NVMe status codes come from the controller, so the commmand must have made it there.