[PATCH 0/1] nvme: fix controller ioctl through ns_head

[PATCH 0/1] nvme: fix controller ioctl through ns_head

[Minwoo Im]

Hello,

This series is to fix controller ioctl though block device in the
multipath case.  This series simply reverts the commit 3557a4409701
("nvme: don't bother to look up a namespace for controller ioctls") to
make it consider the namespace attachment with controllers in the
subsystem.

Minwoo Im (1):
  Revert "nvme: don't bother to look up a namespace for controller
    ioctls"

 drivers/nvme/host/ioctl.c | 47 +++++++++++++++++++++------------------
 1 file changed, 25 insertions(+), 22 deletions(-)

-- 
2.27.0


_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

[PATCH 1/1] Revert "nvme: don't bother to look up a namespace for controller ioctls"

[Minwoo Im]

This reverts commit 3557a4409701a132e8f86ad234ac8cf6e97b052e.

If we simply retrieve namespace from the given subsystem, controller to
be retrieved might not be the one which is really attached to the given
namespace.  We should consider the namespace attachment with controllers
in the subsystem, so this patch simply reverts the previous commit.

Signed-off-by: Minwoo Im <minwoo.im.dev@gmail.com>
---
 drivers/nvme/host/ioctl.c | 47 +++++++++++++++++++++------------------
 1 file changed, 25 insertions(+), 22 deletions(-)

diff --git a/drivers/nvme/host/ioctl.c b/drivers/nvme/host/ioctl.c
index 8e05d65c9e93..9494e3d11e89 100644
--- a/drivers/nvme/host/ioctl.c
+++ b/drivers/nvme/host/ioctl.c
@@ -358,40 +358,43 @@ int nvme_ioctl(struct block_device *bdev, fmode_t mode,
 }
 
 #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, (void __user *)arg);
-	return nvme_ns_head_ns_ioctl(head, cmd, (void __user *)arg);
+		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 */
 
-- 
2.27.0


_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: [PATCH 1/1] Revert "nvme: don't bother to look up a namespace for controller ioctls"

[Minwoo Im]

On 21-04-22 16:07:35, Minwoo Im wrote:
> This reverts commit 3557a4409701a132e8f86ad234ac8cf6e97b052e.
> 
> If we simply retrieve namespace from the given subsystem, controller to
> be retrieved might not be the one which is really attached to the given
> namespace.  We should consider the namespace attachment with controllers
> in the subsystem, so this patch simply reverts the previous commit.
> 
> Signed-off-by: Minwoo Im <minwoo.im.dev@gmail.com>

Sorry, I missed the latest nvme-5.13 tree.  This will have conflicts
with the commit 2637baed7801 ("nvme: introduce generic per-namespace
chardev").

Please let me resend it.

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme