nvme: don't rely on the changed namespace list log
authorChristoph Hellwig <hch@lst.de>
Thu, 7 Jun 2018 11:47:33 +0000 (13:47 +0200)
committerChristoph Hellwig <hch@lst.de>
Wed, 13 Jun 2018 07:24:34 +0000 (09:24 +0200)
Don't optimize our namespace rescan based on the changed namespace list
log page as userspace might have changed the content through reading
it.

Suggested-by: Keith Busch <keith.busch@linux.intel.com>
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Keith Busch <keith.busch@linux.intel.com>
Reviewed-by: Hannes Reinecke <hare@suse.com>
drivers/nvme/host/core.c

index d6ca4598c0279d6871c163270250b229fb4ae8af..dee8e71baf6214ea2f14586b19ac5fd4b2de3687 100644 (file)
@@ -3197,40 +3197,28 @@ static void nvme_scan_ns_sequential(struct nvme_ctrl *ctrl, unsigned nn)
        nvme_remove_invalid_namespaces(ctrl, nn);
 }
 
-static bool nvme_scan_changed_ns_log(struct nvme_ctrl *ctrl)
+static void nvme_clear_changed_ns_log(struct nvme_ctrl *ctrl)
 {
        size_t log_size = NVME_MAX_CHANGED_NAMESPACES * sizeof(__le32);
        __le32 *log;
-       int error, i;
-       bool ret = false;
+       int error;
 
        log = kzalloc(log_size, GFP_KERNEL);
        if (!log)
-               return false;
+               return;
 
+       /*
+        * We need to read the log to clear the AEN, but we don't want to rely
+        * on it for the changed namespace information as userspace could have
+        * raced with us in reading the log page, which could cause us to miss
+        * updates.
+        */
        error = nvme_get_log(ctrl, NVME_LOG_CHANGED_NS, log, log_size);
-       if (error) {
+       if (error)
                dev_warn(ctrl->device,
                        "reading changed ns log failed: %d\n", error);
-               goto out_free_log;
-       }
-
-       if (log[0] == cpu_to_le32(0xffffffff))
-               goto out_free_log;
 
-       for (i = 0; i < NVME_MAX_CHANGED_NAMESPACES; i++) {
-               u32 nsid = le32_to_cpu(log[i]);
-
-               if (nsid == 0)
-                       break;
-               dev_info(ctrl->device, "rescanning namespace %d.\n", nsid);
-               nvme_validate_ns(ctrl, nsid);
-       }
-       ret = true;
-
-out_free_log:
        kfree(log);
-       return ret;
 }
 
 static void nvme_scan_work(struct work_struct *work)
@@ -3246,9 +3234,8 @@ static void nvme_scan_work(struct work_struct *work)
        WARN_ON_ONCE(!ctrl->tagset);
 
        if (test_and_clear_bit(NVME_AER_NOTICE_NS_CHANGED, &ctrl->events)) {
-               if (nvme_scan_changed_ns_log(ctrl))
-                       goto out_sort_namespaces;
                dev_info(ctrl->device, "rescanning namespaces.\n");
+               nvme_clear_changed_ns_log(ctrl);
        }
 
        if (nvme_identify_ctrl(ctrl, &id))
@@ -3263,7 +3250,6 @@ static void nvme_scan_work(struct work_struct *work)
        nvme_scan_ns_sequential(ctrl, nn);
 out_free_id:
        kfree(id);
-out_sort_namespaces:
        down_write(&ctrl->namespaces_rwsem);
        list_sort(NULL, &ctrl->namespaces, ns_cmp);
        up_write(&ctrl->namespaces_rwsem);