nvme: fix controller ioctl through ns_head
authorMinwoo Im <minwoo.im.dev@gmail.com>
Thu, 22 Apr 2021 08:04:07 +0000 (17:04 +0900)
committerChristoph Hellwig <hch@lst.de>
Tue, 4 May 2021 07:35:47 +0000 (09:35 +0200)
In multipath case, we should consider namespace attachment with
controllers in a subsystem when we find out the live controller for the
namespace.  This patch manually reverted the commit 3557a4409701
("nvme: don't bother to look up a namespace for controller ioctls") with
few more updates to nvme_ns_head_chr_ioctl which has been newly updated.

Fixes: 3557a4409701 ("nvme: don't bother to look up a namespace for
controller ioctls")
Cc: Christoph Hellwig <hch@lst.de>
Signed-off-by: Minwoo Im <minwoo.im.dev@gmail.com>
Signed-off-by: Christoph Hellwig <hch@lst.de>
drivers/nvme/host/core.c
drivers/nvme/host/ioctl.c
drivers/nvme/host/nvme.h

index b6f7815..c1c1964 100644 (file)
@@ -1999,28 +1999,6 @@ static const struct block_device_operations nvme_bdev_ops = {
        .pr_ops         = &nvme_pr_ops,
 };
 
-#ifdef CONFIG_NVME_MULTIPATH
-struct nvme_ctrl *nvme_find_get_live_ctrl(struct nvme_subsystem *subsys)
-{
-       struct nvme_ctrl *ctrl;
-       int ret;
-
-       ret = mutex_lock_killable(&nvme_subsystems_lock);
-       if (ret)
-               return ERR_PTR(ret);
-       list_for_each_entry(ctrl, &subsys->ctrls, subsys_entry) {
-               if (ctrl->state == NVME_CTRL_LIVE)
-                       goto found;
-       }
-       mutex_unlock(&nvme_subsystems_lock);
-       return ERR_PTR(-EWOULDBLOCK);
-found:
-       nvme_get_ctrl(ctrl);
-       mutex_unlock(&nvme_subsystems_lock);
-       return ctrl;
-}
-#endif /* CONFIG_NVME_MULTIPATH */
-
 static int nvme_wait_ready(struct nvme_ctrl *ctrl, u64 cap, bool enabled)
 {
        unsigned long timeout =
index 502f8e4..9557ead 100644 (file)
@@ -370,41 +370,45 @@ long nvme_ns_chr_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 }
 
 #ifdef CONFIG_NVME_MULTIPATH
-static int nvme_ns_head_ctrl_ioctl(struct nvme_ns_head *head,
-               unsigned int cmd, void __user *argp)
+static int nvme_ns_head_ctrl_ioctl(struct nvme_ns *ns, unsigned int cmd,
+               void __user *argp, struct nvme_ns_head *head, int srcu_idx)
 {
-       struct nvme_ctrl *ctrl = nvme_find_get_live_ctrl(head->subsys);
+       struct nvme_ctrl *ctrl = ns->ctrl;
        int ret;
 
-       if (IS_ERR(ctrl))
-               return PTR_ERR(ctrl);
-       ret = nvme_ctrl_ioctl(ctrl, cmd, argp);
-       nvme_put_ctrl(ctrl);
-       return ret;
-}
+       nvme_get_ctrl(ns->ctrl);
+       nvme_put_ns_from_disk(head, srcu_idx);
+       ret = nvme_ctrl_ioctl(ns->ctrl, cmd, argp);
 
-static int nvme_ns_head_ns_ioctl(struct nvme_ns_head *head,
-               unsigned int cmd, void __user *argp)
-{
-       int srcu_idx = srcu_read_lock(&head->srcu);
-       struct nvme_ns *ns = nvme_find_path(head);
-       int ret = -EWOULDBLOCK;
-
-       if (ns)
-               ret = nvme_ns_ioctl(ns, cmd, argp);
-       srcu_read_unlock(&head->srcu, srcu_idx);
+       nvme_put_ctrl(ctrl);
        return ret;
 }
 
 int nvme_ns_head_ioctl(struct block_device *bdev, fmode_t mode,
                unsigned int cmd, unsigned long arg)
 {
-       struct nvme_ns_head *head = bdev->bd_disk->private_data;
+       struct nvme_ns_head *head = NULL;
        void __user *argp = (void __user *)arg;
+       struct nvme_ns *ns;
+       int srcu_idx, ret;
+
+       ns = nvme_get_ns_from_disk(bdev->bd_disk, &head, &srcu_idx);
+       if (unlikely(!ns))
+               return -EWOULDBLOCK;
 
+       /*
+        * Handle ioctls that apply to the controller instead of the namespace
+        * seperately and drop the ns SRCU reference early.  This avoids a
+        * deadlock when deleting namespaces using the passthrough interface.
+        */
        if (is_ctrl_ioctl(cmd))
-               return nvme_ns_head_ctrl_ioctl(head, cmd, argp);
-       return nvme_ns_head_ns_ioctl(head, cmd, argp);
+               ret = nvme_ns_head_ctrl_ioctl(ns, cmd, argp, head, srcu_idx);
+       else {
+               ret = nvme_ns_ioctl(ns, cmd, argp);
+               nvme_put_ns_from_disk(head, srcu_idx);
+       }
+
+       return ret;
 }
 
 long nvme_ns_head_chr_ioctl(struct file *file, unsigned int cmd,
@@ -414,10 +418,23 @@ long nvme_ns_head_chr_ioctl(struct file *file, unsigned int cmd,
        struct nvme_ns_head *head =
                container_of(cdev, struct nvme_ns_head, cdev);
        void __user *argp = (void __user *)arg;
+       struct nvme_ns *ns;
+       int srcu_idx, ret;
+
+       srcu_idx = srcu_read_lock(&head->srcu);
+       ns = nvme_find_path(head);
+       if (!ns) {
+               srcu_read_unlock(&head->srcu, srcu_idx);
+               return -EWOULDBLOCK;
+       }
 
        if (is_ctrl_ioctl(cmd))
-               return nvme_ns_head_ctrl_ioctl(head, cmd, argp);
-       return nvme_ns_head_ns_ioctl(head, cmd, argp);
+               return nvme_ns_head_ctrl_ioctl(ns, cmd, argp, head, srcu_idx);
+
+       ret = nvme_ns_ioctl(ns, cmd, argp);
+       nvme_put_ns_from_disk(head, srcu_idx);
+
+       return ret;
 }
 #endif /* CONFIG_NVME_MULTIPATH */
 
index 773dde5..c1e086a 100644 (file)
@@ -664,7 +664,6 @@ struct nvme_ns *nvme_get_ns_from_disk(struct gendisk *disk,
 void nvme_put_ns_from_disk(struct nvme_ns_head *head, int idx);
 bool nvme_tryget_ns_head(struct nvme_ns_head *head);
 void nvme_put_ns_head(struct nvme_ns_head *head);
-struct nvme_ctrl *nvme_find_get_live_ctrl(struct nvme_subsystem *subsys);
 int nvme_cdev_add(struct cdev *cdev, struct device *cdev_device,
                const struct file_operations *fops, struct module *owner);
 void nvme_cdev_del(struct cdev *cdev, struct device *cdev_device);