Should NVME_SC_INVALID_NS be translated to BLK_STS_IOERR instead of BLK_STS_NOTSUPP so that multipath(both native and dm) can failover on the failure?

Sagi Grimberg sagi at grimberg.me
Tue Dec 5 00:50:40 PST 2023


>> So the controller through that path used to be able to access the
>> Namespace, then suddenly lost ability to do so, but some other path can
>> still access it if we retry on a failover/alternate path? I think your
>> target is returning the wrong error code. It should be SCT/SC 303h,
>> Asymmetric Access Persistent Loss (NVME_SC_ANA_TRANSITION), for what
>> you're describing.
> 
> Yes, assuming ANA is actually supported by the controllers..

Its a good point (should probably be ana inaccessible). But
semantically, this status is with respect to all the namespace in the
ana group. And the host will not see an updated ANA log page, which will
then override back the ns ana state?

The below can handle the return status, but its not clear what should
the behavior we want to have...
--
diff --git a/drivers/nvme/target/configfs.c b/drivers/nvme/target/configfs.c
index e307a044b1a1..5fd5e74a41a8 100644
--- a/drivers/nvme/target/configfs.c
+++ b/drivers/nvme/target/configfs.c
@@ -726,6 +726,19 @@ static struct configfs_attribute *nvmet_ns_attrs[] = {
         NULL,
  };

+bool nvmet_subsys_nsid_exists(struct nvmet_subsys *subsys, u32 nsid)
+{
+       struct config_item *ns_item;
+       char name[4] = {};
+
+       if (sprintf(name, "%u\n", nsid) <= 0)
+               return false;
+       mutex_lock(&subsys->namespaces_group.cg_subsys->su_mutex);
+       ns_item = config_group_find_item(&subsys->namespaces_group, name);
+       mutex_unlock(&subsys->namespaces_group.cg_subsys->su_mutex);
+       return ns_item != NULL;
+}
+
  static void nvmet_ns_release(struct config_item *item)
  {
         struct nvmet_ns *ns = to_nvmet_ns(item);
diff --git a/drivers/nvme/target/core.c b/drivers/nvme/target/core.c
index 3935165048e7..426ced914a21 100644
--- a/drivers/nvme/target/core.c
+++ b/drivers/nvme/target/core.c
@@ -425,11 +425,15 @@ void nvmet_stop_keep_alive_timer(struct nvmet_ctrl 
*ctrl)
  u16 nvmet_req_find_ns(struct nvmet_req *req)
  {
         u32 nsid = le32_to_cpu(req->cmd->common.nsid);
+       struct nvmet_subsys *subsys = nvmet_req_subsys(req);

-       req->ns = xa_load(&nvmet_req_subsys(req)->namespaces, nsid);
+       req->ns = xa_load(&subsys->namespaces, nsid);
         if (unlikely(!req->ns)) {
                 req->error_loc = offsetof(struct nvme_common_command, 
nsid);
-               return NVME_SC_INVALID_NS | NVME_SC_DNR;
+               if (nvmet_subsys_nsid_exists(subsys, nsid))
+                       return NVME_ANA_PERSISTENT_LOSS;
+               else
+                       return NVME_SC_INVALID_NS | NVME_SC_DNR;
         }

         percpu_ref_get(&req->ns->ref);
diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h
index 6c8acebe1a1a..477416abf85a 100644
--- a/drivers/nvme/target/nvmet.h
+++ b/drivers/nvme/target/nvmet.h
@@ -542,6 +542,7 @@ void nvmet_subsys_disc_changed(struct nvmet_subsys 
*subsys,
                 struct nvmet_host *host);
  void nvmet_add_async_event(struct nvmet_ctrl *ctrl, u8 event_type,
                 u8 event_info, u8 log_page);
+bool nvmet_subsys_nsid_exists(struct nvmet_subsys *subsys, u32 nsid);

  #define NVMET_QUEUE_SIZE       1024
  #define NVMET_NR_QUEUES                128
--
--



More information about the Linux-nvme mailing list