Thread (38 messages) 38 messages, 8 authors, 2020-06-29

Re: [PATCHv3 3/5] nvme: implement I/O Command Sets Command Set support

From: Sagi Grimberg <sagi@grimberg.me>
Date: 2020-06-24 18:33:29
Also in: linux-nvme
Subsystem: nvm express driver, the rest · Maintainers: Keith Busch, Jens Axboe, Christoph Hellwig, Sagi Grimberg, Linus Torvalds

quoted
If the controller does not support the CNS value, it may return Invalid
Field with DNR set. That error currently gets propogated back to
nvme_init_ns_head(), which then abandons the namespace. Just as the code
coments say, we had been historically been clearing such errors because
we have other ways to identify the namespace, but now we're not clearing
that error.
I don't understand what you are saying Keith.

My comment was for this block:
-- 
     if (!status && nvme_multi_css(ctrl) && !csi_seen) {
         dev_warn(ctrl->device, "Command set not reported for nsid:%d\n",
              nsid);
         status = -EINVAL;
     }
-- 

I was saying that !status doesn't necessarily mean success, but it can
also mean that its an retry-able error status (due to transport or
controller). If we see a retry-able error we should still clear the
status so we don't abandon the namespace.

This for example would achieve that:
Sorry, meant this:
--
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index ba512c45b40f..3187cf768d08 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -1130,8 +1130,14 @@ static int nvme_identify_ns_descs(struct 
nvme_ctrl *ctrl, unsigned nsid,
                   * Don't treat an error as fatal, as we potentially 
already
                   * have a NGUID or EUI-64.
                   */
-               if (status > 0 && !(status & NVME_SC_DNR))
+               if (status > 0 && !(status & NVME_SC_DNR)) {
                         status = 0;
+               } else if (status == 0 && nvme_multi_css(ctrl) && 
!csi_seen) {
+                               dev_warn(ctrl->device,
+                                       "Command set not reported for 
nsid:%d\n",
+                                       nsid);
+                               status = -EINVAL;
+               }
                 goto free_data;
         }
--
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help