All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH V6 0/2] nvme: enable char device per namespace
@ 2021-03-01 19:24 ` javier
  0 siblings, 0 replies; 34+ messages in thread
From: javier @ 2021-03-01 19:24 UTC (permalink / raw)
  To: linux-nvme
  Cc: linux-block, hch, kbusch, sagi, minwoo.im.dev, Javier González

From: Javier González <javier.gonz@samsung.com>

These two patches enable a char device per namespace, also through
multipath. It is possible to test this in QEMU using Keith's and Klaus'
tree in nvme-next

     http://git.infradead.org/qemu-nvme.git/shortlog/refs/heads/nvme-next


One question that came up while preparing V6 is what to do with the bdev
sysfs entries when the block device fails. On the one hand, these values
are useful for the char device, but on the other, they are in principle
only valid for the block device. We have 3 options: (i) clear sysfs when
bdev fails and require user-space to query the nvme device directly
through sysfs, (ii) maintain the bdev sysfs entries, and (iii) make char
device dedicated sysfs entries, which might be redundant when both the
char device and the block device are brought up. Thoughts?

Keith: Regarding nvme-cli support, once this is in place, we will send a
patch to nvme-cli so that this device is shown in verbose mode. Do you
have anu thoughts about this?

Changes since V5
  - Addressed style and naming comments from Christoph
  - Change the logic around nvme_update_ns_info() to (i) make the
    GENHD_FL_HIDDEN more explicit in the init logic, and (ii) to support
    an error path that also stops the char device from being created.
    This error path is not exercised currently.

Changes since V4
  - Added support for multipath (from Minwoo)
  - Fixed typo in commit message for sysfs naming
  - Rebase into nvme-5.12

Changes since V3
  - Use a dedicated ida for the generic handle
  - Do not abort namespace greation if the generic handle fails

Changes since V2:
  - Apply a number of naming and code structure improvements (from
    Christoph)
  - Use i_cdev to pull struct nvme_ns in the ioctl path instead of
    populating file->private_data (from Christoph)
  - Change char device and sysfs entries to /dev/nvme-generic-XcYnZ to
    follow the hidden device naming scheme (from Christoph and Keith)

Changes since V1:
  - Remove patches 1-3 which are already picked up by Christoph
  - Change the char device and sysfs entries to nvmeXnYc / c signals
    char device
  - Address Minwoo's comments on inline functions and style


Javier González (1):
  nvme: enable char device per namespace

Minwoo Im (1):
  nvme: allow open for nvme-generic char device

 drivers/nvme/host/core.c | 198 ++++++++++++++++++++++++++++++++++-----
 drivers/nvme/host/nvme.h |   5 +
 2 files changed, 182 insertions(+), 21 deletions(-)

-- 
2.17.1


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH V6 0/2] nvme: enable char device per namespace
@ 2021-03-01 19:24 ` javier
  0 siblings, 0 replies; 34+ messages in thread
From: javier @ 2021-03-01 19:24 UTC (permalink / raw)
  To: linux-nvme
  Cc: sagi, linux-block, minwoo.im.dev, kbusch, Javier González, hch

From: Javier González <javier.gonz@samsung.com>

These two patches enable a char device per namespace, also through
multipath. It is possible to test this in QEMU using Keith's and Klaus'
tree in nvme-next

     http://git.infradead.org/qemu-nvme.git/shortlog/refs/heads/nvme-next


One question that came up while preparing V6 is what to do with the bdev
sysfs entries when the block device fails. On the one hand, these values
are useful for the char device, but on the other, they are in principle
only valid for the block device. We have 3 options: (i) clear sysfs when
bdev fails and require user-space to query the nvme device directly
through sysfs, (ii) maintain the bdev sysfs entries, and (iii) make char
device dedicated sysfs entries, which might be redundant when both the
char device and the block device are brought up. Thoughts?

Keith: Regarding nvme-cli support, once this is in place, we will send a
patch to nvme-cli so that this device is shown in verbose mode. Do you
have anu thoughts about this?

Changes since V5
  - Addressed style and naming comments from Christoph
  - Change the logic around nvme_update_ns_info() to (i) make the
    GENHD_FL_HIDDEN more explicit in the init logic, and (ii) to support
    an error path that also stops the char device from being created.
    This error path is not exercised currently.

Changes since V4
  - Added support for multipath (from Minwoo)
  - Fixed typo in commit message for sysfs naming
  - Rebase into nvme-5.12

Changes since V3
  - Use a dedicated ida for the generic handle
  - Do not abort namespace greation if the generic handle fails

Changes since V2:
  - Apply a number of naming and code structure improvements (from
    Christoph)
  - Use i_cdev to pull struct nvme_ns in the ioctl path instead of
    populating file->private_data (from Christoph)
  - Change char device and sysfs entries to /dev/nvme-generic-XcYnZ to
    follow the hidden device naming scheme (from Christoph and Keith)

Changes since V1:
  - Remove patches 1-3 which are already picked up by Christoph
  - Change the char device and sysfs entries to nvmeXnYc / c signals
    char device
  - Address Minwoo's comments on inline functions and style


Javier González (1):
  nvme: enable char device per namespace

Minwoo Im (1):
  nvme: allow open for nvme-generic char device

 drivers/nvme/host/core.c | 198 ++++++++++++++++++++++++++++++++++-----
 drivers/nvme/host/nvme.h |   5 +
 2 files changed, 182 insertions(+), 21 deletions(-)

-- 
2.17.1


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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH V6 1/2] nvme: enable char device per namespace
  2021-03-01 19:24 ` javier
@ 2021-03-01 19:24   ` javier
  -1 siblings, 0 replies; 34+ messages in thread
From: javier @ 2021-03-01 19:24 UTC (permalink / raw)
  To: linux-nvme
  Cc: linux-block, hch, kbusch, sagi, minwoo.im.dev, Javier González

From: Javier González <javier.gonz@samsung.com>

Create a char device per NVMe namespace. This char device is always
initialized, independently of whether the features implemented by the
device are supported by the kernel. User-space can therefore always
issue IOCTLs to the NVMe driver using the char device.

The char device is presented as /dev/nvme-generic-XcYnZ. This naming
scheme follows the convention of the hidden device (nvmeXcYnZ). Support
for multipath will follow.

Signed-off-by: Javier González <javier.gonz@samsung.com>
Signed-off-by: Minwoo Im <minwoo.im.dev@gmail.com>
---
 drivers/nvme/host/core.c | 185 +++++++++++++++++++++++++++++++++++----
 drivers/nvme/host/nvme.h |   5 ++
 2 files changed, 174 insertions(+), 16 deletions(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index d77f3f26d8d3..b94609bc92f4 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -86,13 +86,31 @@ static DEFINE_MUTEX(nvme_subsystems_lock);
 
 static DEFINE_IDA(nvme_instance_ida);
 static dev_t nvme_ctrl_base_chr_devt;
+
+static DEFINE_IDA(nvme_gen_minor_ida);
+static dev_t nvme_ns_base_chr_devt;
 static struct class *nvme_class;
+static struct class *nvme_ns_class;
 static struct class *nvme_subsys_class;
 
 static void nvme_put_subsystem(struct nvme_subsystem *subsys);
 static void nvme_remove_invalid_namespaces(struct nvme_ctrl *ctrl,
 					   unsigned nsid);
 
+/*
+ * These two helpers check whether the given instance is per-namespace
+ * character device (generic device) or not.
+ */
+static inline bool nvme_dev_is_generic(struct device *dev)
+{
+	return dev->class == nvme_ns_class;
+}
+
+static inline bool nvme_ns_is_generic(struct nvme_ns *ns)
+{
+	return !!ns->minor;
+}
+
 /*
  * Prepare a queue for teardown.
  *
@@ -559,7 +577,10 @@ static void nvme_free_ns(struct kref *kref)
 
 	if (ns->ndev)
 		nvme_nvm_unregister(ns);
+	if (nvme_ns_is_generic(ns))
+		ida_simple_remove(&nvme_gen_minor_ida, ns->minor - 1);
 
+	cdev_device_del(&ns->cdev, &ns->cdev_device);
 	put_disk(ns->disk);
 	nvme_put_ns_head(ns->head);
 	nvme_put_ctrl(ns->ctrl);
@@ -1772,15 +1793,15 @@ static int nvme_handle_ctrl_ioctl(struct nvme_ns *ns, unsigned int cmd,
 	return ret;
 }
 
-static int nvme_ioctl(struct block_device *bdev, fmode_t mode,
-		unsigned int cmd, unsigned long arg)
+static int nvme_disk_ioctl(struct gendisk *disk, unsigned int cmd,
+			   unsigned long arg)
 {
 	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);
+	ns = nvme_get_ns_from_disk(disk, &head, &srcu_idx);
 	if (unlikely(!ns))
 		return -EWOULDBLOCK;
 
@@ -1817,6 +1838,12 @@ static int nvme_ioctl(struct block_device *bdev, fmode_t mode,
 	return ret;
 }
 
+static int nvme_ioctl(struct block_device *bdev, fmode_t mode,
+		      unsigned int cmd, unsigned long arg)
+{
+	return nvme_disk_ioctl(bdev->bd_disk, cmd, arg);
+}
+
 #ifdef CONFIG_COMPAT
 struct nvme_user_io32 {
 	__u8	opcode;
@@ -1858,10 +1885,8 @@ static int nvme_compat_ioctl(struct block_device *bdev, fmode_t mode,
 #define nvme_compat_ioctl	NULL
 #endif /* CONFIG_COMPAT */
 
-static int nvme_open(struct block_device *bdev, fmode_t mode)
+static int nvme_ns_open(struct nvme_ns *ns)
 {
-	struct nvme_ns *ns = bdev->bd_disk->private_data;
-
 #ifdef CONFIG_NVME_MULTIPATH
 	/* should never be called due to GENHD_FL_HIDDEN */
 	if (WARN_ON_ONCE(ns->head->disk))
@@ -1880,14 +1905,22 @@ static int nvme_open(struct block_device *bdev, fmode_t mode)
 	return -ENXIO;
 }
 
-static void nvme_release(struct gendisk *disk, fmode_t mode)
+static void nvme_ns_release(struct nvme_ns *ns)
 {
-	struct nvme_ns *ns = disk->private_data;
-
 	module_put(ns->ctrl->ops->module);
 	nvme_put_ns(ns);
 }
 
+static int nvme_open(struct block_device *bdev, fmode_t mode)
+{
+	return nvme_ns_open(bdev->bd_disk->private_data);
+}
+
+static void nvme_release(struct gendisk *disk, fmode_t mode)
+{
+	nvme_ns_release(disk->private_data);
+}
+
 static int nvme_getgeo(struct block_device *bdev, struct hd_geometry *geo)
 {
 	/* some standard values */
@@ -2205,6 +2238,8 @@ static int nvme_update_ns_info(struct nvme_ns *ns, struct nvme_id_ns *id)
 	unsigned lbaf = id->flbas & NVME_NS_FLBAS_LBA_MASK;
 	int ret;
 
+	ns->features |= NVME_NS_CHAR_SUPPORTED;
+
 	blk_mq_freeze_queue(ns->disk->queue);
 	ns->lba_shift = id->lbaf[lbaf].ds;
 	nvme_set_queue_limits(ns->ctrl, ns->queue);
@@ -2378,6 +2413,44 @@ static const struct block_device_operations nvme_bdev_ops = {
 	.pr_ops		= &nvme_pr_ops,
 };
 
+static inline struct nvme_ns *dev_to_ns(struct device *dev)
+{
+	return container_of(dev, struct nvme_ns, cdev_device);
+}
+
+static inline struct nvme_ns *cdev_to_ns(struct cdev *cdev)
+{
+	return container_of(cdev, struct nvme_ns, cdev);
+}
+
+static int nvme_cdev_open(struct inode *inode, struct file *file)
+{
+	return nvme_ns_open(cdev_to_ns(inode->i_cdev));
+}
+
+static int nvme_cdev_release(struct inode *inode, struct file *file)
+{
+	nvme_ns_release(cdev_to_ns(inode->i_cdev));
+
+	return 0;
+}
+
+static long nvme_cdev_ioctl(struct file *file, unsigned int cmd,
+			    unsigned long arg)
+{
+	struct nvme_ns *ns = cdev_to_ns(file->f_inode->i_cdev);
+
+	return nvme_disk_ioctl(ns->disk, cmd, arg);
+}
+
+static const struct file_operations nvme_cdev_fops = {
+	.owner		= THIS_MODULE,
+	.open		= nvme_cdev_open,
+	.release	= nvme_cdev_release,
+	.unlocked_ioctl	= nvme_cdev_ioctl,
+	.compat_ioctl	= compat_ptr_ioctl,
+};
+
 #ifdef CONFIG_NVME_MULTIPATH
 static int nvme_ns_head_open(struct block_device *bdev, fmode_t mode)
 {
@@ -3379,6 +3452,9 @@ static inline struct nvme_ns_head *dev_to_ns_head(struct device *dev)
 {
 	struct gendisk *disk = dev_to_disk(dev);
 
+	if (nvme_dev_is_generic(dev))
+		return dev_to_ns(dev)->head;
+
 	if (disk->fops == &nvme_bdev_ops)
 		return nvme_get_ns_from_dev(dev)->head;
 	else
@@ -3488,6 +3564,8 @@ static umode_t nvme_ns_id_attrs_are_visible(struct kobject *kobj,
 	}
 #ifdef CONFIG_NVME_MULTIPATH
 	if (a == &dev_attr_ana_grpid.attr || a == &dev_attr_ana_state.attr) {
+		if (nvme_dev_is_generic(dev))
+			return 0;
 		if (dev_to_disk(dev)->fops != &nvme_bdev_ops) /* per-path attr */
 			return 0;
 		if (!nvme_ctrl_use_ana(nvme_get_ns_from_dev(dev)->ctrl))
@@ -3510,6 +3588,11 @@ const struct attribute_group *nvme_ns_id_attr_groups[] = {
 	NULL,
 };
 
+const struct attribute_group *nvme_ns_char_id_attr_groups[] = {
+	&nvme_ns_id_attr_group,
+	NULL,
+};
+
 #define nvme_show_str_function(field)						\
 static ssize_t  field##_show(struct device *dev,				\
 			    struct device_attribute *attr, char *buf)		\
@@ -3902,6 +3985,47 @@ struct nvme_ns *nvme_find_get_ns(struct nvme_ctrl *ctrl, unsigned nsid)
 }
 EXPORT_SYMBOL_NS_GPL(nvme_find_get_ns, NVME_TARGET_PASSTHRU);
 
+static int nvme_alloc_chardev_ns(struct nvme_ctrl *ctrl, struct nvme_ns *ns)
+{
+	char cdisk_name[DISK_NAME_LEN];
+	int ret;
+
+	ret = ida_simple_get(&nvme_gen_minor_ida, 0, 0, GFP_KERNEL);
+	if (ret < 0)
+		return ret;
+
+	ns->minor = ret + 1;
+	device_initialize(&ns->cdev_device);
+	ns->cdev_device.devt = MKDEV(MAJOR(nvme_ns_base_chr_devt), ret);
+	ns->cdev_device.class = nvme_ns_class;
+	ns->cdev_device.parent = ctrl->device;
+	ns->cdev_device.groups = nvme_ns_char_id_attr_groups;
+	dev_set_drvdata(&ns->cdev_device, ns);
+
+	sprintf(cdisk_name, "nvme-generic-%dc%dn%d", ctrl->subsys->instance,
+		ctrl->instance, ns->head->instance);
+
+	ret = dev_set_name(&ns->cdev_device, "%s", cdisk_name);
+	if (ret)
+		goto put_ida;
+
+	cdev_init(&ns->cdev, &nvme_cdev_fops);
+	ns->cdev.owner = ctrl->ops->module;
+
+	ret = cdev_device_add(&ns->cdev, &ns->cdev_device);
+	if (ret)
+		goto free_kobj;
+
+	return ret;
+
+free_kobj:
+	kfree_const(ns->cdev_device.kobj.name);
+put_ida:
+	ida_simple_remove(&nvme_gen_minor_ida, ns->minor - 1);
+	ns->minor = 0;
+	return ret;
+}
+
 static void nvme_alloc_ns(struct nvme_ctrl *ctrl, unsigned nsid,
 		struct nvme_ns_ids *ids)
 {
@@ -3948,8 +4072,16 @@ static void nvme_alloc_ns(struct nvme_ctrl *ctrl, unsigned nsid,
 	memcpy(disk->disk_name, disk_name, DISK_NAME_LEN);
 	ns->disk = disk;
 
-	if (nvme_update_ns_info(ns, id))
-		goto out_put_disk;
+	/*
+	 * If the namespace update fails in a graceful manner, hide the block
+	 * device, but still allow for the namespace char device to be created.
+	 */
+	if (nvme_update_ns_info(ns, id)) {
+		if (ns->features & NVME_NS_CHAR_SUPPORTED)
+			ns->disk->flags |= GENHD_FL_HIDDEN;
+		else
+			goto out_put_disk;
+	}
 
 	if ((ctrl->quirks & NVME_QUIRK_LIGHTNVM) && id->vs[0] == 0x1) {
 		if (nvme_nvm_register(ns, disk_name, node)) {
@@ -3965,9 +4097,14 @@ static void nvme_alloc_ns(struct nvme_ctrl *ctrl, unsigned nsid,
 	nvme_get_ctrl(ctrl);
 
 	device_add_disk(ctrl->device, ns->disk, nvme_ns_id_attr_groups);
-
 	nvme_mpath_add_disk(ns, id);
 	nvme_fault_inject_init(&ns->fault_inject, ns->disk->disk_name);
+
+	if (nvme_alloc_chardev_ns(ctrl, ns))
+		dev_warn(ctrl->device,
+			"failed to create generic handle for nsid:%d\n",
+			nsid);
+
 	kfree(id);
 
 	return;
@@ -4780,23 +4917,38 @@ static int __init nvme_core_init(void)
 	if (result < 0)
 		goto destroy_delete_wq;
 
+	result = alloc_chrdev_region(&nvme_ns_base_chr_devt, 0,
+			NVME_MINORS, "nvmec");
+	if (result < 0)
+		goto unregister_dev_chrdev;
+
 	nvme_class = class_create(THIS_MODULE, "nvme");
 	if (IS_ERR(nvme_class)) {
 		result = PTR_ERR(nvme_class);
-		goto unregister_chrdev;
+		goto unregister_ns_chrdev;
 	}
 	nvme_class->dev_uevent = nvme_class_uevent;
 
+	nvme_ns_class = class_create(THIS_MODULE, "nvme-ns");
+	if (IS_ERR(nvme_ns_class)) {
+		result = PTR_ERR(nvme_ns_class);
+		goto destroy_dev_class;
+	}
+
 	nvme_subsys_class = class_create(THIS_MODULE, "nvme-subsystem");
 	if (IS_ERR(nvme_subsys_class)) {
 		result = PTR_ERR(nvme_subsys_class);
-		goto destroy_class;
+		goto destroy_ns_class;
 	}
 	return 0;
 
-destroy_class:
+destroy_ns_class:
+	class_destroy(nvme_ns_class);
+destroy_dev_class:
 	class_destroy(nvme_class);
-unregister_chrdev:
+unregister_ns_chrdev:
+	unregister_chrdev_region(nvme_ns_base_chr_devt, NVME_MINORS);
+unregister_dev_chrdev:
 	unregister_chrdev_region(nvme_ctrl_base_chr_devt, NVME_MINORS);
 destroy_delete_wq:
 	destroy_workqueue(nvme_delete_wq);
@@ -4812,6 +4964,7 @@ static void __exit nvme_core_exit(void)
 {
 	class_destroy(nvme_subsys_class);
 	class_destroy(nvme_class);
+	unregister_chrdev_region(nvme_ns_base_chr_devt, NVME_MINORS);
 	unregister_chrdev_region(nvme_ctrl_base_chr_devt, NVME_MINORS);
 	destroy_workqueue(nvme_delete_wq);
 	destroy_workqueue(nvme_reset_wq);
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index 07b34175c6ce..a145eaa7f86f 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -425,6 +425,7 @@ struct nvme_ns_head {
 enum nvme_ns_features {
 	NVME_NS_EXT_LBAS = 1 << 0, /* support extended LBA format */
 	NVME_NS_METADATA_SUPPORTED = 1 << 1, /* support getting generated md */
+	NVME_NS_CHAR_SUPPORTED = 1 << 2, /* support for char device */
 };
 
 struct nvme_ns {
@@ -442,6 +443,10 @@ struct nvme_ns {
 	struct kref kref;
 	struct nvme_ns_head *head;
 
+	struct device cdev_device;	/* char device */
+	struct cdev cdev;
+	int minor;
+
 	int lba_shift;
 	u16 ms;
 	u16 sgs;
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH V6 1/2] nvme: enable char device per namespace
@ 2021-03-01 19:24   ` javier
  0 siblings, 0 replies; 34+ messages in thread
From: javier @ 2021-03-01 19:24 UTC (permalink / raw)
  To: linux-nvme
  Cc: sagi, linux-block, minwoo.im.dev, kbusch, Javier González, hch

From: Javier González <javier.gonz@samsung.com>

Create a char device per NVMe namespace. This char device is always
initialized, independently of whether the features implemented by the
device are supported by the kernel. User-space can therefore always
issue IOCTLs to the NVMe driver using the char device.

The char device is presented as /dev/nvme-generic-XcYnZ. This naming
scheme follows the convention of the hidden device (nvmeXcYnZ). Support
for multipath will follow.

Signed-off-by: Javier González <javier.gonz@samsung.com>
Signed-off-by: Minwoo Im <minwoo.im.dev@gmail.com>
---
 drivers/nvme/host/core.c | 185 +++++++++++++++++++++++++++++++++++----
 drivers/nvme/host/nvme.h |   5 ++
 2 files changed, 174 insertions(+), 16 deletions(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index d77f3f26d8d3..b94609bc92f4 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -86,13 +86,31 @@ static DEFINE_MUTEX(nvme_subsystems_lock);
 
 static DEFINE_IDA(nvme_instance_ida);
 static dev_t nvme_ctrl_base_chr_devt;
+
+static DEFINE_IDA(nvme_gen_minor_ida);
+static dev_t nvme_ns_base_chr_devt;
 static struct class *nvme_class;
+static struct class *nvme_ns_class;
 static struct class *nvme_subsys_class;
 
 static void nvme_put_subsystem(struct nvme_subsystem *subsys);
 static void nvme_remove_invalid_namespaces(struct nvme_ctrl *ctrl,
 					   unsigned nsid);
 
+/*
+ * These two helpers check whether the given instance is per-namespace
+ * character device (generic device) or not.
+ */
+static inline bool nvme_dev_is_generic(struct device *dev)
+{
+	return dev->class == nvme_ns_class;
+}
+
+static inline bool nvme_ns_is_generic(struct nvme_ns *ns)
+{
+	return !!ns->minor;
+}
+
 /*
  * Prepare a queue for teardown.
  *
@@ -559,7 +577,10 @@ static void nvme_free_ns(struct kref *kref)
 
 	if (ns->ndev)
 		nvme_nvm_unregister(ns);
+	if (nvme_ns_is_generic(ns))
+		ida_simple_remove(&nvme_gen_minor_ida, ns->minor - 1);
 
+	cdev_device_del(&ns->cdev, &ns->cdev_device);
 	put_disk(ns->disk);
 	nvme_put_ns_head(ns->head);
 	nvme_put_ctrl(ns->ctrl);
@@ -1772,15 +1793,15 @@ static int nvme_handle_ctrl_ioctl(struct nvme_ns *ns, unsigned int cmd,
 	return ret;
 }
 
-static int nvme_ioctl(struct block_device *bdev, fmode_t mode,
-		unsigned int cmd, unsigned long arg)
+static int nvme_disk_ioctl(struct gendisk *disk, unsigned int cmd,
+			   unsigned long arg)
 {
 	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);
+	ns = nvme_get_ns_from_disk(disk, &head, &srcu_idx);
 	if (unlikely(!ns))
 		return -EWOULDBLOCK;
 
@@ -1817,6 +1838,12 @@ static int nvme_ioctl(struct block_device *bdev, fmode_t mode,
 	return ret;
 }
 
+static int nvme_ioctl(struct block_device *bdev, fmode_t mode,
+		      unsigned int cmd, unsigned long arg)
+{
+	return nvme_disk_ioctl(bdev->bd_disk, cmd, arg);
+}
+
 #ifdef CONFIG_COMPAT
 struct nvme_user_io32 {
 	__u8	opcode;
@@ -1858,10 +1885,8 @@ static int nvme_compat_ioctl(struct block_device *bdev, fmode_t mode,
 #define nvme_compat_ioctl	NULL
 #endif /* CONFIG_COMPAT */
 
-static int nvme_open(struct block_device *bdev, fmode_t mode)
+static int nvme_ns_open(struct nvme_ns *ns)
 {
-	struct nvme_ns *ns = bdev->bd_disk->private_data;
-
 #ifdef CONFIG_NVME_MULTIPATH
 	/* should never be called due to GENHD_FL_HIDDEN */
 	if (WARN_ON_ONCE(ns->head->disk))
@@ -1880,14 +1905,22 @@ static int nvme_open(struct block_device *bdev, fmode_t mode)
 	return -ENXIO;
 }
 
-static void nvme_release(struct gendisk *disk, fmode_t mode)
+static void nvme_ns_release(struct nvme_ns *ns)
 {
-	struct nvme_ns *ns = disk->private_data;
-
 	module_put(ns->ctrl->ops->module);
 	nvme_put_ns(ns);
 }
 
+static int nvme_open(struct block_device *bdev, fmode_t mode)
+{
+	return nvme_ns_open(bdev->bd_disk->private_data);
+}
+
+static void nvme_release(struct gendisk *disk, fmode_t mode)
+{
+	nvme_ns_release(disk->private_data);
+}
+
 static int nvme_getgeo(struct block_device *bdev, struct hd_geometry *geo)
 {
 	/* some standard values */
@@ -2205,6 +2238,8 @@ static int nvme_update_ns_info(struct nvme_ns *ns, struct nvme_id_ns *id)
 	unsigned lbaf = id->flbas & NVME_NS_FLBAS_LBA_MASK;
 	int ret;
 
+	ns->features |= NVME_NS_CHAR_SUPPORTED;
+
 	blk_mq_freeze_queue(ns->disk->queue);
 	ns->lba_shift = id->lbaf[lbaf].ds;
 	nvme_set_queue_limits(ns->ctrl, ns->queue);
@@ -2378,6 +2413,44 @@ static const struct block_device_operations nvme_bdev_ops = {
 	.pr_ops		= &nvme_pr_ops,
 };
 
+static inline struct nvme_ns *dev_to_ns(struct device *dev)
+{
+	return container_of(dev, struct nvme_ns, cdev_device);
+}
+
+static inline struct nvme_ns *cdev_to_ns(struct cdev *cdev)
+{
+	return container_of(cdev, struct nvme_ns, cdev);
+}
+
+static int nvme_cdev_open(struct inode *inode, struct file *file)
+{
+	return nvme_ns_open(cdev_to_ns(inode->i_cdev));
+}
+
+static int nvme_cdev_release(struct inode *inode, struct file *file)
+{
+	nvme_ns_release(cdev_to_ns(inode->i_cdev));
+
+	return 0;
+}
+
+static long nvme_cdev_ioctl(struct file *file, unsigned int cmd,
+			    unsigned long arg)
+{
+	struct nvme_ns *ns = cdev_to_ns(file->f_inode->i_cdev);
+
+	return nvme_disk_ioctl(ns->disk, cmd, arg);
+}
+
+static const struct file_operations nvme_cdev_fops = {
+	.owner		= THIS_MODULE,
+	.open		= nvme_cdev_open,
+	.release	= nvme_cdev_release,
+	.unlocked_ioctl	= nvme_cdev_ioctl,
+	.compat_ioctl	= compat_ptr_ioctl,
+};
+
 #ifdef CONFIG_NVME_MULTIPATH
 static int nvme_ns_head_open(struct block_device *bdev, fmode_t mode)
 {
@@ -3379,6 +3452,9 @@ static inline struct nvme_ns_head *dev_to_ns_head(struct device *dev)
 {
 	struct gendisk *disk = dev_to_disk(dev);
 
+	if (nvme_dev_is_generic(dev))
+		return dev_to_ns(dev)->head;
+
 	if (disk->fops == &nvme_bdev_ops)
 		return nvme_get_ns_from_dev(dev)->head;
 	else
@@ -3488,6 +3564,8 @@ static umode_t nvme_ns_id_attrs_are_visible(struct kobject *kobj,
 	}
 #ifdef CONFIG_NVME_MULTIPATH
 	if (a == &dev_attr_ana_grpid.attr || a == &dev_attr_ana_state.attr) {
+		if (nvme_dev_is_generic(dev))
+			return 0;
 		if (dev_to_disk(dev)->fops != &nvme_bdev_ops) /* per-path attr */
 			return 0;
 		if (!nvme_ctrl_use_ana(nvme_get_ns_from_dev(dev)->ctrl))
@@ -3510,6 +3588,11 @@ const struct attribute_group *nvme_ns_id_attr_groups[] = {
 	NULL,
 };
 
+const struct attribute_group *nvme_ns_char_id_attr_groups[] = {
+	&nvme_ns_id_attr_group,
+	NULL,
+};
+
 #define nvme_show_str_function(field)						\
 static ssize_t  field##_show(struct device *dev,				\
 			    struct device_attribute *attr, char *buf)		\
@@ -3902,6 +3985,47 @@ struct nvme_ns *nvme_find_get_ns(struct nvme_ctrl *ctrl, unsigned nsid)
 }
 EXPORT_SYMBOL_NS_GPL(nvme_find_get_ns, NVME_TARGET_PASSTHRU);
 
+static int nvme_alloc_chardev_ns(struct nvme_ctrl *ctrl, struct nvme_ns *ns)
+{
+	char cdisk_name[DISK_NAME_LEN];
+	int ret;
+
+	ret = ida_simple_get(&nvme_gen_minor_ida, 0, 0, GFP_KERNEL);
+	if (ret < 0)
+		return ret;
+
+	ns->minor = ret + 1;
+	device_initialize(&ns->cdev_device);
+	ns->cdev_device.devt = MKDEV(MAJOR(nvme_ns_base_chr_devt), ret);
+	ns->cdev_device.class = nvme_ns_class;
+	ns->cdev_device.parent = ctrl->device;
+	ns->cdev_device.groups = nvme_ns_char_id_attr_groups;
+	dev_set_drvdata(&ns->cdev_device, ns);
+
+	sprintf(cdisk_name, "nvme-generic-%dc%dn%d", ctrl->subsys->instance,
+		ctrl->instance, ns->head->instance);
+
+	ret = dev_set_name(&ns->cdev_device, "%s", cdisk_name);
+	if (ret)
+		goto put_ida;
+
+	cdev_init(&ns->cdev, &nvme_cdev_fops);
+	ns->cdev.owner = ctrl->ops->module;
+
+	ret = cdev_device_add(&ns->cdev, &ns->cdev_device);
+	if (ret)
+		goto free_kobj;
+
+	return ret;
+
+free_kobj:
+	kfree_const(ns->cdev_device.kobj.name);
+put_ida:
+	ida_simple_remove(&nvme_gen_minor_ida, ns->minor - 1);
+	ns->minor = 0;
+	return ret;
+}
+
 static void nvme_alloc_ns(struct nvme_ctrl *ctrl, unsigned nsid,
 		struct nvme_ns_ids *ids)
 {
@@ -3948,8 +4072,16 @@ static void nvme_alloc_ns(struct nvme_ctrl *ctrl, unsigned nsid,
 	memcpy(disk->disk_name, disk_name, DISK_NAME_LEN);
 	ns->disk = disk;
 
-	if (nvme_update_ns_info(ns, id))
-		goto out_put_disk;
+	/*
+	 * If the namespace update fails in a graceful manner, hide the block
+	 * device, but still allow for the namespace char device to be created.
+	 */
+	if (nvme_update_ns_info(ns, id)) {
+		if (ns->features & NVME_NS_CHAR_SUPPORTED)
+			ns->disk->flags |= GENHD_FL_HIDDEN;
+		else
+			goto out_put_disk;
+	}
 
 	if ((ctrl->quirks & NVME_QUIRK_LIGHTNVM) && id->vs[0] == 0x1) {
 		if (nvme_nvm_register(ns, disk_name, node)) {
@@ -3965,9 +4097,14 @@ static void nvme_alloc_ns(struct nvme_ctrl *ctrl, unsigned nsid,
 	nvme_get_ctrl(ctrl);
 
 	device_add_disk(ctrl->device, ns->disk, nvme_ns_id_attr_groups);
-
 	nvme_mpath_add_disk(ns, id);
 	nvme_fault_inject_init(&ns->fault_inject, ns->disk->disk_name);
+
+	if (nvme_alloc_chardev_ns(ctrl, ns))
+		dev_warn(ctrl->device,
+			"failed to create generic handle for nsid:%d\n",
+			nsid);
+
 	kfree(id);
 
 	return;
@@ -4780,23 +4917,38 @@ static int __init nvme_core_init(void)
 	if (result < 0)
 		goto destroy_delete_wq;
 
+	result = alloc_chrdev_region(&nvme_ns_base_chr_devt, 0,
+			NVME_MINORS, "nvmec");
+	if (result < 0)
+		goto unregister_dev_chrdev;
+
 	nvme_class = class_create(THIS_MODULE, "nvme");
 	if (IS_ERR(nvme_class)) {
 		result = PTR_ERR(nvme_class);
-		goto unregister_chrdev;
+		goto unregister_ns_chrdev;
 	}
 	nvme_class->dev_uevent = nvme_class_uevent;
 
+	nvme_ns_class = class_create(THIS_MODULE, "nvme-ns");
+	if (IS_ERR(nvme_ns_class)) {
+		result = PTR_ERR(nvme_ns_class);
+		goto destroy_dev_class;
+	}
+
 	nvme_subsys_class = class_create(THIS_MODULE, "nvme-subsystem");
 	if (IS_ERR(nvme_subsys_class)) {
 		result = PTR_ERR(nvme_subsys_class);
-		goto destroy_class;
+		goto destroy_ns_class;
 	}
 	return 0;
 
-destroy_class:
+destroy_ns_class:
+	class_destroy(nvme_ns_class);
+destroy_dev_class:
 	class_destroy(nvme_class);
-unregister_chrdev:
+unregister_ns_chrdev:
+	unregister_chrdev_region(nvme_ns_base_chr_devt, NVME_MINORS);
+unregister_dev_chrdev:
 	unregister_chrdev_region(nvme_ctrl_base_chr_devt, NVME_MINORS);
 destroy_delete_wq:
 	destroy_workqueue(nvme_delete_wq);
@@ -4812,6 +4964,7 @@ static void __exit nvme_core_exit(void)
 {
 	class_destroy(nvme_subsys_class);
 	class_destroy(nvme_class);
+	unregister_chrdev_region(nvme_ns_base_chr_devt, NVME_MINORS);
 	unregister_chrdev_region(nvme_ctrl_base_chr_devt, NVME_MINORS);
 	destroy_workqueue(nvme_delete_wq);
 	destroy_workqueue(nvme_reset_wq);
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index 07b34175c6ce..a145eaa7f86f 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -425,6 +425,7 @@ struct nvme_ns_head {
 enum nvme_ns_features {
 	NVME_NS_EXT_LBAS = 1 << 0, /* support extended LBA format */
 	NVME_NS_METADATA_SUPPORTED = 1 << 1, /* support getting generated md */
+	NVME_NS_CHAR_SUPPORTED = 1 << 2, /* support for char device */
 };
 
 struct nvme_ns {
@@ -442,6 +443,10 @@ struct nvme_ns {
 	struct kref kref;
 	struct nvme_ns_head *head;
 
+	struct device cdev_device;	/* char device */
+	struct cdev cdev;
+	int minor;
+
 	int lba_shift;
 	u16 ms;
 	u16 sgs;
-- 
2.17.1


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

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH V6 2/2] nvme: allow open for nvme-generic char device
  2021-03-01 19:24 ` javier
@ 2021-03-01 19:24   ` javier
  -1 siblings, 0 replies; 34+ messages in thread
From: javier @ 2021-03-01 19:24 UTC (permalink / raw)
  To: linux-nvme; +Cc: linux-block, hch, kbusch, sagi, minwoo.im.dev

From: Minwoo Im <minwoo.im.dev@gmail.com>

Keep rejecting the hidden device access via open, but allow cases
through the nvme-generic char device by moving check for head namespace
to nvme_open() from nvme_ns_open().

Signed-off-by: Minwoo Im <minwoo.im.dev@gmail.com>
---
 drivers/nvme/host/core.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index b94609bc92f4..ed66ff33a85a 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -1887,11 +1887,6 @@ static int nvme_compat_ioctl(struct block_device *bdev, fmode_t mode,
 
 static int nvme_ns_open(struct nvme_ns *ns)
 {
-#ifdef CONFIG_NVME_MULTIPATH
-	/* should never be called due to GENHD_FL_HIDDEN */
-	if (WARN_ON_ONCE(ns->head->disk))
-		goto fail;
-#endif
 	if (!kref_get_unless_zero(&ns->kref))
 		goto fail;
 	if (!try_module_get(ns->ctrl->ops->module))
@@ -1913,7 +1908,15 @@ static void nvme_ns_release(struct nvme_ns *ns)
 
 static int nvme_open(struct block_device *bdev, fmode_t mode)
 {
-	return nvme_ns_open(bdev->bd_disk->private_data);
+	struct nvme_ns *ns = bdev->bd_disk->private_data;
+
+#ifdef CONFIG_NVME_MULTIPATH
+	/* should never be called due to GENHD_FL_HIDDEN */
+	if (WARN_ON_ONCE(ns->head->disk))
+		return -ENXIO;
+#endif
+
+	return nvme_ns_open(ns);
 }
 
 static void nvme_release(struct gendisk *disk, fmode_t mode)
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH V6 2/2] nvme: allow open for nvme-generic char device
@ 2021-03-01 19:24   ` javier
  0 siblings, 0 replies; 34+ messages in thread
From: javier @ 2021-03-01 19:24 UTC (permalink / raw)
  To: linux-nvme; +Cc: linux-block, kbusch, minwoo.im.dev, hch, sagi

From: Minwoo Im <minwoo.im.dev@gmail.com>

Keep rejecting the hidden device access via open, but allow cases
through the nvme-generic char device by moving check for head namespace
to nvme_open() from nvme_ns_open().

Signed-off-by: Minwoo Im <minwoo.im.dev@gmail.com>
---
 drivers/nvme/host/core.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index b94609bc92f4..ed66ff33a85a 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -1887,11 +1887,6 @@ static int nvme_compat_ioctl(struct block_device *bdev, fmode_t mode,
 
 static int nvme_ns_open(struct nvme_ns *ns)
 {
-#ifdef CONFIG_NVME_MULTIPATH
-	/* should never be called due to GENHD_FL_HIDDEN */
-	if (WARN_ON_ONCE(ns->head->disk))
-		goto fail;
-#endif
 	if (!kref_get_unless_zero(&ns->kref))
 		goto fail;
 	if (!try_module_get(ns->ctrl->ops->module))
@@ -1913,7 +1908,15 @@ static void nvme_ns_release(struct nvme_ns *ns)
 
 static int nvme_open(struct block_device *bdev, fmode_t mode)
 {
-	return nvme_ns_open(bdev->bd_disk->private_data);
+	struct nvme_ns *ns = bdev->bd_disk->private_data;
+
+#ifdef CONFIG_NVME_MULTIPATH
+	/* should never be called due to GENHD_FL_HIDDEN */
+	if (WARN_ON_ONCE(ns->head->disk))
+		return -ENXIO;
+#endif
+
+	return nvme_ns_open(ns);
 }
 
 static void nvme_release(struct gendisk *disk, fmode_t mode)
-- 
2.17.1


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

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* Re: [PATCH V6 1/2] nvme: enable char device per namespace
  2021-03-01 19:24   ` javier
@ 2021-03-03  9:10     ` Christoph Hellwig
  -1 siblings, 0 replies; 34+ messages in thread
From: Christoph Hellwig @ 2021-03-03  9:10 UTC (permalink / raw)
  To: javier
  Cc: linux-nvme, linux-block, hch, kbusch, sagi, minwoo.im.dev,
	Javier González

On Mon, Mar 01, 2021 at 08:24:51PM +0100, javier@javigon.com wrote:
> From: Javier González <javier.gonz@samsung.com>
> 
> Create a char device per NVMe namespace. This char device is always
> initialized, independently of whether the features implemented by the
> device are supported by the kernel. User-space can therefore always
> issue IOCTLs to the NVMe driver using the char device.
> 
> The char device is presented as /dev/nvme-generic-XcYnZ. This naming
> scheme follows the convention of the hidden device (nvmeXcYnZ). Support
> for multipath will follow.

So I'm a little worried about the "support for multipath will follow" as
this has implications for the naming scheme, and our policy of how we
allow access to a namespace.

Ignoring some of the deprecated historic mistakes I think the policy
should be:

 - admin commands that often are controller specific should usually
   go to a controller-specific device, the existing /dev/nvmeX
   devices
 - I/O commands and admin command that do specific a nsid should go
   through a per-namespace node that is multipath aware and not
   controller specific

Which also makes me wonder about patch 2 in the series that seems
somewhat dangerous.   Can we clearly state the policy implemented?

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH V6 1/2] nvme: enable char device per namespace
@ 2021-03-03  9:10     ` Christoph Hellwig
  0 siblings, 0 replies; 34+ messages in thread
From: Christoph Hellwig @ 2021-03-03  9:10 UTC (permalink / raw)
  To: javier
  Cc: linux-nvme, linux-block, hch, kbusch, sagi, minwoo.im.dev,
	Javier González

On Mon, Mar 01, 2021 at 08:24:51PM +0100, javier@javigon.com wrote:
> From: Javier González <javier.gonz@samsung.com>
> 
> Create a char device per NVMe namespace. This char device is always
> initialized, independently of whether the features implemented by the
> device are supported by the kernel. User-space can therefore always
> issue IOCTLs to the NVMe driver using the char device.
> 
> The char device is presented as /dev/nvme-generic-XcYnZ. This naming
> scheme follows the convention of the hidden device (nvmeXcYnZ). Support
> for multipath will follow.

So I'm a little worried about the "support for multipath will follow" as
this has implications for the naming scheme, and our policy of how we
allow access to a namespace.

Ignoring some of the deprecated historic mistakes I think the policy
should be:

 - admin commands that often are controller specific should usually
   go to a controller-specific device, the existing /dev/nvmeX
   devices
 - I/O commands and admin command that do specific a nsid should go
   through a per-namespace node that is multipath aware and not
   controller specific

Which also makes me wonder about patch 2 in the series that seems
somewhat dangerous.   Can we clearly state the policy implemented?

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH V6 1/2] nvme: enable char device per namespace
  2021-03-03  9:10     ` Christoph Hellwig
@ 2021-03-03 10:02       ` Javier González
  -1 siblings, 0 replies; 34+ messages in thread
From: Javier González @ 2021-03-03 10:02 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: linux-nvme, linux-block, kbusch, sagi, minwoo.im.dev

On 03.03.2021 10:10, Christoph Hellwig wrote:
>On Mon, Mar 01, 2021 at 08:24:51PM +0100, javier@javigon.com wrote:
>> From: Javier González <javier.gonz@samsung.com>
>>
>> Create a char device per NVMe namespace. This char device is always
>> initialized, independently of whether the features implemented by the
>> device are supported by the kernel. User-space can therefore always
>> issue IOCTLs to the NVMe driver using the char device.
>>
>> The char device is presented as /dev/nvme-generic-XcYnZ. This naming
>> scheme follows the convention of the hidden device (nvmeXcYnZ). Support
>> for multipath will follow.
>
>So I'm a little worried about the "support for multipath will follow" as
>this has implications for the naming scheme, and our policy of how we
>allow access to a namespace.

I maintained the message to follow the original commit. In this
patchset multipath is implemented. I maintains the same naming scheme.

We can remove this comment as multipath is supported in the existing
series.

>
>Ignoring some of the deprecated historic mistakes I think the policy
>should be:
>
> - admin commands that often are controller specific should usually
>   go to a controller-specific device, the existing /dev/nvmeX
>   devices
> - I/O commands and admin command that do specific a nsid should go
>   through a per-namespace node that is multipath aware and not
>   controller specific

Sounds good.

The current implementation re-routes IOCTLs to the char device through
the existing bdev IOCTLs, so I believe we follow this policy already. We
basically default to current behavior.

And I assume that for legacy, namespace IOCTLs to the controller will
still be routed to the namespace (assuming a single namespace).

>Which also makes me wonder about patch 2 in the series that seems
>somewhat dangerous.   Can we clearly state the policy implemented?

Patch 2 is the one that exposes the existing logic for multipath. How do
you think we should do it instead?

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH V6 1/2] nvme: enable char device per namespace
@ 2021-03-03 10:02       ` Javier González
  0 siblings, 0 replies; 34+ messages in thread
From: Javier González @ 2021-03-03 10:02 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: linux-nvme, linux-block, kbusch, sagi, minwoo.im.dev

On 03.03.2021 10:10, Christoph Hellwig wrote:
>On Mon, Mar 01, 2021 at 08:24:51PM +0100, javier@javigon.com wrote:
>> From: Javier González <javier.gonz@samsung.com>
>>
>> Create a char device per NVMe namespace. This char device is always
>> initialized, independently of whether the features implemented by the
>> device are supported by the kernel. User-space can therefore always
>> issue IOCTLs to the NVMe driver using the char device.
>>
>> The char device is presented as /dev/nvme-generic-XcYnZ. This naming
>> scheme follows the convention of the hidden device (nvmeXcYnZ). Support
>> for multipath will follow.
>
>So I'm a little worried about the "support for multipath will follow" as
>this has implications for the naming scheme, and our policy of how we
>allow access to a namespace.

I maintained the message to follow the original commit. In this
patchset multipath is implemented. I maintains the same naming scheme.

We can remove this comment as multipath is supported in the existing
series.

>
>Ignoring some of the deprecated historic mistakes I think the policy
>should be:
>
> - admin commands that often are controller specific should usually
>   go to a controller-specific device, the existing /dev/nvmeX
>   devices
> - I/O commands and admin command that do specific a nsid should go
>   through a per-namespace node that is multipath aware and not
>   controller specific

Sounds good.

The current implementation re-routes IOCTLs to the char device through
the existing bdev IOCTLs, so I believe we follow this policy already. We
basically default to current behavior.

And I assume that for legacy, namespace IOCTLs to the controller will
still be routed to the namespace (assuming a single namespace).

>Which also makes me wonder about patch 2 in the series that seems
>somewhat dangerous.   Can we clearly state the policy implemented?

Patch 2 is the one that exposes the existing logic for multipath. How do
you think we should do it instead?

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH V6 1/2] nvme: enable char device per namespace
  2021-03-03 10:02       ` Javier González
@ 2021-03-09 11:31         ` Christoph Hellwig
  -1 siblings, 0 replies; 34+ messages in thread
From: Christoph Hellwig @ 2021-03-09 11:31 UTC (permalink / raw)
  To: Javier González
  Cc: Christoph Hellwig, linux-nvme, linux-block, kbusch, sagi, minwoo.im.dev

On Wed, Mar 03, 2021 at 11:02:12AM +0100, Javier González wrote:
>> Ignoring some of the deprecated historic mistakes I think the policy
>> should be:
>>
>> - admin commands that often are controller specific should usually
>>   go to a controller-specific device, the existing /dev/nvmeX
>>   devices
>> - I/O commands and admin command that do specific a nsid should go
>>   through a per-namespace node that is multipath aware and not
>>   controller specific
>
> Sounds good.
>
> The current implementation re-routes IOCTLs to the char device through
> the existing bdev IOCTLs, so I believe we follow this policy already. We
> basically default to current behavior.
>
> And I assume that for legacy, namespace IOCTLs to the controller will
> still be routed to the namespace (assuming a single namespace).
>
>> Which also makes me wonder about patch 2 in the series that seems
>> somewhat dangerous.   Can we clearly state the policy implemented?
>
> Patch 2 is the one that exposes the existing logic for multipath. How do
> you think we should do it instead?

So trying to follow the code:

 - nvme_cdev_fops implements file operations that directly on a nvme_ns,
   so they are path specific
 - we allow opening them even for a hidden controller
 - there does not seem to be a char device node for ns_head at all.

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH V6 1/2] nvme: enable char device per namespace
@ 2021-03-09 11:31         ` Christoph Hellwig
  0 siblings, 0 replies; 34+ messages in thread
From: Christoph Hellwig @ 2021-03-09 11:31 UTC (permalink / raw)
  To: Javier González
  Cc: Christoph Hellwig, linux-nvme, linux-block, kbusch, sagi, minwoo.im.dev

On Wed, Mar 03, 2021 at 11:02:12AM +0100, Javier González wrote:
>> Ignoring some of the deprecated historic mistakes I think the policy
>> should be:
>>
>> - admin commands that often are controller specific should usually
>>   go to a controller-specific device, the existing /dev/nvmeX
>>   devices
>> - I/O commands and admin command that do specific a nsid should go
>>   through a per-namespace node that is multipath aware and not
>>   controller specific
>
> Sounds good.
>
> The current implementation re-routes IOCTLs to the char device through
> the existing bdev IOCTLs, so I believe we follow this policy already. We
> basically default to current behavior.
>
> And I assume that for legacy, namespace IOCTLs to the controller will
> still be routed to the namespace (assuming a single namespace).
>
>> Which also makes me wonder about patch 2 in the series that seems
>> somewhat dangerous.   Can we clearly state the policy implemented?
>
> Patch 2 is the one that exposes the existing logic for multipath. How do
> you think we should do it instead?

So trying to follow the code:

 - nvme_cdev_fops implements file operations that directly on a nvme_ns,
   so they are path specific
 - we allow opening them even for a hidden controller
 - there does not seem to be a char device node for ns_head at all.

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH V6 1/2] nvme: enable char device per namespace
  2021-03-09 11:31         ` Christoph Hellwig
@ 2021-03-09 12:42           ` Javier González
  -1 siblings, 0 replies; 34+ messages in thread
From: Javier González @ 2021-03-09 12:42 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: linux-nvme, linux-block, kbusch, sagi, minwoo.im.dev

On 09.03.2021 12:31, Christoph Hellwig wrote:
>On Wed, Mar 03, 2021 at 11:02:12AM +0100, Javier González wrote:
>>> Ignoring some of the deprecated historic mistakes I think the policy
>>> should be:
>>>
>>> - admin commands that often are controller specific should usually
>>>   go to a controller-specific device, the existing /dev/nvmeX
>>>   devices
>>> - I/O commands and admin command that do specific a nsid should go
>>>   through a per-namespace node that is multipath aware and not
>>>   controller specific
>>
>> Sounds good.
>>
>> The current implementation re-routes IOCTLs to the char device through
>> the existing bdev IOCTLs, so I believe we follow this policy already. We
>> basically default to current behavior.
>>
>> And I assume that for legacy, namespace IOCTLs to the controller will
>> still be routed to the namespace (assuming a single namespace).
>>
>>> Which also makes me wonder about patch 2 in the series that seems
>>> somewhat dangerous.   Can we clearly state the policy implemented?
>>
>> Patch 2 is the one that exposes the existing logic for multipath. How do
>> you think we should do it instead?
>
>So trying to follow the code:
>
> - nvme_cdev_fops implements file operations that directly on a nvme_ns,
>   so they are path specific

This is correct.

> - we allow opening them even for a hidden controller

This is also correct.

> - there does not seem to be a char device node for ns_head at all.

Also correct.

We tried to keep it simple in the first iteration. Am I understanding
that you see necessary to have per ns_head char devices?


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH V6 1/2] nvme: enable char device per namespace
@ 2021-03-09 12:42           ` Javier González
  0 siblings, 0 replies; 34+ messages in thread
From: Javier González @ 2021-03-09 12:42 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: linux-nvme, linux-block, kbusch, sagi, minwoo.im.dev

On 09.03.2021 12:31, Christoph Hellwig wrote:
>On Wed, Mar 03, 2021 at 11:02:12AM +0100, Javier González wrote:
>>> Ignoring some of the deprecated historic mistakes I think the policy
>>> should be:
>>>
>>> - admin commands that often are controller specific should usually
>>>   go to a controller-specific device, the existing /dev/nvmeX
>>>   devices
>>> - I/O commands and admin command that do specific a nsid should go
>>>   through a per-namespace node that is multipath aware and not
>>>   controller specific
>>
>> Sounds good.
>>
>> The current implementation re-routes IOCTLs to the char device through
>> the existing bdev IOCTLs, so I believe we follow this policy already. We
>> basically default to current behavior.
>>
>> And I assume that for legacy, namespace IOCTLs to the controller will
>> still be routed to the namespace (assuming a single namespace).
>>
>>> Which also makes me wonder about patch 2 in the series that seems
>>> somewhat dangerous.   Can we clearly state the policy implemented?
>>
>> Patch 2 is the one that exposes the existing logic for multipath. How do
>> you think we should do it instead?
>
>So trying to follow the code:
>
> - nvme_cdev_fops implements file operations that directly on a nvme_ns,
>   so they are path specific

This is correct.

> - we allow opening them even for a hidden controller

This is also correct.

> - there does not seem to be a char device node for ns_head at all.

Also correct.

We tried to keep it simple in the first iteration. Am I understanding
that you see necessary to have per ns_head char devices?


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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH V6 1/2] nvme: enable char device per namespace
  2021-03-09 12:42           ` Javier González
@ 2021-03-09 15:05             ` Christoph Hellwig
  -1 siblings, 0 replies; 34+ messages in thread
From: Christoph Hellwig @ 2021-03-09 15:05 UTC (permalink / raw)
  To: Javier González
  Cc: Christoph Hellwig, linux-nvme, linux-block, kbusch, sagi, minwoo.im.dev

On Tue, Mar 09, 2021 at 01:42:23PM +0100, Javier González wrote:
>> - nvme_cdev_fops implements file operations that directly on a nvme_ns,
>>   so they are path specific
>
> This is correct.
>
>> - we allow opening them even for a hidden controller
>
> This is also correct.
>
>> - there does not seem to be a char device node for ns_head at all.
>
> Also correct.
>
> We tried to keep it simple in the first iteration. Am I understanding
> that you see necessary to have per ns_head char devices?

That would be my understanding of "multipath support" for this character
device, yes.  Especially as hiding the individual char devices for the
hidden controllers once they are initially exposed would be an ABI break.

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH V6 1/2] nvme: enable char device per namespace
@ 2021-03-09 15:05             ` Christoph Hellwig
  0 siblings, 0 replies; 34+ messages in thread
From: Christoph Hellwig @ 2021-03-09 15:05 UTC (permalink / raw)
  To: Javier González
  Cc: Christoph Hellwig, linux-nvme, linux-block, kbusch, sagi, minwoo.im.dev

On Tue, Mar 09, 2021 at 01:42:23PM +0100, Javier González wrote:
>> - nvme_cdev_fops implements file operations that directly on a nvme_ns,
>>   so they are path specific
>
> This is correct.
>
>> - we allow opening them even for a hidden controller
>
> This is also correct.
>
>> - there does not seem to be a char device node for ns_head at all.
>
> Also correct.
>
> We tried to keep it simple in the first iteration. Am I understanding
> that you see necessary to have per ns_head char devices?

That would be my understanding of "multipath support" for this character
device, yes.  Especially as hiding the individual char devices for the
hidden controllers once they are initially exposed would be an ABI break.

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH V6 1/2] nvme: enable char device per namespace
  2021-03-09 15:05             ` Christoph Hellwig
@ 2021-03-09 21:18               ` Javier González
  -1 siblings, 0 replies; 34+ messages in thread
From: Javier González @ 2021-03-09 21:18 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: linux-nvme, linux-block, kbusch, sagi, minwoo.im.dev

On 09.03.2021 16:05, Christoph Hellwig wrote:
>On Tue, Mar 09, 2021 at 01:42:23PM +0100, Javier González wrote:
>>> - nvme_cdev_fops implements file operations that directly on a nvme_ns,
>>>   so they are path specific
>>
>> This is correct.
>>
>>> - we allow opening them even for a hidden controller
>>
>> This is also correct.
>>
>>> - there does not seem to be a char device node for ns_head at all.
>>
>> Also correct.
>>
>> We tried to keep it simple in the first iteration. Am I understanding
>> that you see necessary to have per ns_head char devices?
>
>That would be my understanding of "multipath support" for this character
>device, yes.  Especially as hiding the individual char devices for the
>hidden controllers once they are initially exposed would be an ABI break.

Ok. Will look into it for the next iteration.

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH V6 1/2] nvme: enable char device per namespace
@ 2021-03-09 21:18               ` Javier González
  0 siblings, 0 replies; 34+ messages in thread
From: Javier González @ 2021-03-09 21:18 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: linux-nvme, linux-block, kbusch, sagi, minwoo.im.dev

On 09.03.2021 16:05, Christoph Hellwig wrote:
>On Tue, Mar 09, 2021 at 01:42:23PM +0100, Javier González wrote:
>>> - nvme_cdev_fops implements file operations that directly on a nvme_ns,
>>>   so they are path specific
>>
>> This is correct.
>>
>>> - we allow opening them even for a hidden controller
>>
>> This is also correct.
>>
>>> - there does not seem to be a char device node for ns_head at all.
>>
>> Also correct.
>>
>> We tried to keep it simple in the first iteration. Am I understanding
>> that you see necessary to have per ns_head char devices?
>
>That would be my understanding of "multipath support" for this character
>device, yes.  Especially as hiding the individual char devices for the
>hidden controllers once they are initially exposed would be an ABI break.

Ok. Will look into it for the next iteration.

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH V6 1/2] nvme: enable char device per namespace
  2021-03-01 19:24   ` javier
@ 2021-03-24 12:29     ` Niklas Cassel
  -1 siblings, 0 replies; 34+ messages in thread
From: Niklas Cassel @ 2021-03-24 12:29 UTC (permalink / raw)
  To: javier
  Cc: linux-nvme, sagi, linux-block, minwoo.im.dev, kbusch,
	Javier González, hch

On Mon, Mar 01, 2021 at 08:24:51PM +0100, javier@javigon.com wrote:
> From: Javier González <javier.gonz@samsung.com>
> 
> Create a char device per NVMe namespace. This char device is always
> initialized, independently of whether the features implemented by the
> device are supported by the kernel. User-space can therefore always
> issue IOCTLs to the NVMe driver using the char device.
> 
> The char device is presented as /dev/nvme-generic-XcYnZ. This naming
> scheme follows the convention of the hidden device (nvmeXcYnZ). Support
> for multipath will follow.
> 

Hello all,

Looking at the discussion that led up to the current design:
https://lore.kernel.org/linux-block/20201102185851.GA21349@lst.de/

Keith initially suggested:

a) Set up the existing controller character device with a generic
   disk-less request_queue to the IO queues accepting IO commands to
   arbitrary NSIDs.

However Christoph replied:

The problem with a) is that it can't be used to give users or groups
access to just one namespaces, so it causes a real access control
nightmare.

c) Each namespace gets its own character device, period.



However, testing this patch series out:

crw------- 1 root root 249,   0 Mar 24 11:32 /dev/nvme-generic-0c0n1
crw------- 1 root root 249,   1 Mar 24 11:32 /dev/nvme-generic-0c0n2
crw------- 1 root root 250,   0 Mar 24 11:32 /dev/nvme0
brw-rw---- 1 root disk 259,   1 Mar 24 11:32 /dev/nvme0n2

NSID1 has been rejected (because of ZNS ZOC, which kernel does not support).

However, if I use the new char device for NSID1, but specify NSID2 to nvme-cli:

sudo nvme write-zeroes -s 0 -c 0 --namespace-id=2 /dev/nvme-generic-0c0n1

I was still allowed to write to NSID2:

sudo nvme zns report-zones -d 1 /dev/nvme0n2
SLBA: 0x0        WP: 0x1        Cap: 0x3e000    State: IMP_OPENED   Type: SEQWRITE_REQ   Attrs: 0x0

Should this really be allowed?

I was under the impression that Christoph's argument for implementing per
namespace char devices, was that you should be able to do access control.
Doesn't that mean that for the new char devices, we need to reject ioctls
that specify a nvme_passthru_cmd.nsid != the NSID that the char device
represents?


Although, this is not really something new, as we already have the same
behavior when it comes ioctls and the block devices. Perhaps we want to
add the same verification there?

Regardless if we want to add a verification for block devices or not,
it just seemed to me that the whole argument for introducing new char
devices was to allow access control per namespace, which doesn't seem
to have been taken into account, but perhaps I'm missing something.


Kind regards,
Niklas

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH V6 1/2] nvme: enable char device per namespace
@ 2021-03-24 12:29     ` Niklas Cassel
  0 siblings, 0 replies; 34+ messages in thread
From: Niklas Cassel @ 2021-03-24 12:29 UTC (permalink / raw)
  To: javier
  Cc: linux-nvme, sagi, linux-block, minwoo.im.dev, kbusch,
	Javier González, hch

On Mon, Mar 01, 2021 at 08:24:51PM +0100, javier@javigon.com wrote:
> From: Javier González <javier.gonz@samsung.com>
> 
> Create a char device per NVMe namespace. This char device is always
> initialized, independently of whether the features implemented by the
> device are supported by the kernel. User-space can therefore always
> issue IOCTLs to the NVMe driver using the char device.
> 
> The char device is presented as /dev/nvme-generic-XcYnZ. This naming
> scheme follows the convention of the hidden device (nvmeXcYnZ). Support
> for multipath will follow.
> 

Hello all,

Looking at the discussion that led up to the current design:
https://lore.kernel.org/linux-block/20201102185851.GA21349@lst.de/

Keith initially suggested:

a) Set up the existing controller character device with a generic
   disk-less request_queue to the IO queues accepting IO commands to
   arbitrary NSIDs.

However Christoph replied:

The problem with a) is that it can't be used to give users or groups
access to just one namespaces, so it causes a real access control
nightmare.

c) Each namespace gets its own character device, period.



However, testing this patch series out:

crw------- 1 root root 249,   0 Mar 24 11:32 /dev/nvme-generic-0c0n1
crw------- 1 root root 249,   1 Mar 24 11:32 /dev/nvme-generic-0c0n2
crw------- 1 root root 250,   0 Mar 24 11:32 /dev/nvme0
brw-rw---- 1 root disk 259,   1 Mar 24 11:32 /dev/nvme0n2

NSID1 has been rejected (because of ZNS ZOC, which kernel does not support).

However, if I use the new char device for NSID1, but specify NSID2 to nvme-cli:

sudo nvme write-zeroes -s 0 -c 0 --namespace-id=2 /dev/nvme-generic-0c0n1

I was still allowed to write to NSID2:

sudo nvme zns report-zones -d 1 /dev/nvme0n2
SLBA: 0x0        WP: 0x1        Cap: 0x3e000    State: IMP_OPENED   Type: SEQWRITE_REQ   Attrs: 0x0

Should this really be allowed?

I was under the impression that Christoph's argument for implementing per
namespace char devices, was that you should be able to do access control.
Doesn't that mean that for the new char devices, we need to reject ioctls
that specify a nvme_passthru_cmd.nsid != the NSID that the char device
represents?


Although, this is not really something new, as we already have the same
behavior when it comes ioctls and the block devices. Perhaps we want to
add the same verification there?

Regardless if we want to add a verification for block devices or not,
it just seemed to me that the whole argument for introducing new char
devices was to allow access control per namespace, which doesn't seem
to have been taken into account, but perhaps I'm missing something.


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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH V6 1/2] nvme: enable char device per namespace
  2021-03-24 12:29     ` Niklas Cassel
@ 2021-03-25  2:09       ` Minwoo Im
  -1 siblings, 0 replies; 34+ messages in thread
From: Minwoo Im @ 2021-03-25  2:09 UTC (permalink / raw)
  To: Niklas Cassel
  Cc: javier, linux-nvme, sagi, linux-block, kbusch, Javier González, hch

On 21-03-24 12:29:32, Niklas Cassel wrote:
> On Mon, Mar 01, 2021 at 08:24:51PM +0100, javier@javigon.com wrote:
> > From: Javier González <javier.gonz@samsung.com>
> > 
> > Create a char device per NVMe namespace. This char device is always
> > initialized, independently of whether the features implemented by the
> > device are supported by the kernel. User-space can therefore always
> > issue IOCTLs to the NVMe driver using the char device.
> > 
> > The char device is presented as /dev/nvme-generic-XcYnZ. This naming
> > scheme follows the convention of the hidden device (nvmeXcYnZ). Support
> > for multipath will follow.
> > 
> 
> Hello all,
> 
> Looking at the discussion that led up to the current design:
> https://lore.kernel.org/linux-block/20201102185851.GA21349@lst.de/
> 
> Keith initially suggested:
> 
> a) Set up the existing controller character device with a generic
>    disk-less request_queue to the IO queues accepting IO commands to
>    arbitrary NSIDs.
> 
> However Christoph replied:
> 
> The problem with a) is that it can't be used to give users or groups
> access to just one namespaces, so it causes a real access control
> nightmare.
> 
> c) Each namespace gets its own character device, period.

Thanks for summarizing this up!

> However, testing this patch series out:
> 
> crw------- 1 root root 249,   0 Mar 24 11:32 /dev/nvme-generic-0c0n1
> crw------- 1 root root 249,   1 Mar 24 11:32 /dev/nvme-generic-0c0n2
> crw------- 1 root root 250,   0 Mar 24 11:32 /dev/nvme0
> brw-rw---- 1 root disk 259,   1 Mar 24 11:32 /dev/nvme0n2
> 
> NSID1 has been rejected (because of ZNS ZOC, which kernel does not support).
> 
> However, if I use the new char device for NSID1, but specify NSID2 to nvme-cli:
> 
> sudo nvme write-zeroes -s 0 -c 0 --namespace-id=2 /dev/nvme-generic-0c0n1
> 
> I was still allowed to write to NSID2:
> 
> sudo nvme zns report-zones -d 1 /dev/nvme0n2
> SLBA: 0x0        WP: 0x1        Cap: 0x3e000    State: IMP_OPENED   Type: SEQWRITE_REQ   Attrs: 0x0
> 
> Should this really be allowed?

I think this should not be allowed at all.  Thanks for the testing!

> 
> I was under the impression that Christoph's argument for implementing per
> namespace char devices, was that you should be able to do access control.
> Doesn't that mean that for the new char devices, we need to reject ioctls
> that specify a nvme_passthru_cmd.nsid != the NSID that the char device
> represents?
> 
> 
> Although, this is not really something new, as we already have the same
> behavior when it comes ioctls and the block devices. Perhaps we want to
> add the same verification there?

I think there should be verifications.

> 
> Regardless if we want to add a verification for block devices or not,
> it just seemed to me that the whole argument for introducing new char
> devices was to allow access control per namespace, which doesn't seem
> to have been taken into account, but perhaps I'm missing something.

Any other points that you think it's not been taken account?  I think it
should map to previous blkdev operations, but with some verfications
there.  It would be great if you can share any other points supposed to
be supported here :)

> Kind regards,
> Niklas

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH V6 1/2] nvme: enable char device per namespace
@ 2021-03-25  2:09       ` Minwoo Im
  0 siblings, 0 replies; 34+ messages in thread
From: Minwoo Im @ 2021-03-25  2:09 UTC (permalink / raw)
  To: Niklas Cassel
  Cc: javier, linux-nvme, sagi, linux-block, kbusch, Javier González, hch

On 21-03-24 12:29:32, Niklas Cassel wrote:
> On Mon, Mar 01, 2021 at 08:24:51PM +0100, javier@javigon.com wrote:
> > From: Javier González <javier.gonz@samsung.com>
> > 
> > Create a char device per NVMe namespace. This char device is always
> > initialized, independently of whether the features implemented by the
> > device are supported by the kernel. User-space can therefore always
> > issue IOCTLs to the NVMe driver using the char device.
> > 
> > The char device is presented as /dev/nvme-generic-XcYnZ. This naming
> > scheme follows the convention of the hidden device (nvmeXcYnZ). Support
> > for multipath will follow.
> > 
> 
> Hello all,
> 
> Looking at the discussion that led up to the current design:
> https://lore.kernel.org/linux-block/20201102185851.GA21349@lst.de/
> 
> Keith initially suggested:
> 
> a) Set up the existing controller character device with a generic
>    disk-less request_queue to the IO queues accepting IO commands to
>    arbitrary NSIDs.
> 
> However Christoph replied:
> 
> The problem with a) is that it can't be used to give users or groups
> access to just one namespaces, so it causes a real access control
> nightmare.
> 
> c) Each namespace gets its own character device, period.

Thanks for summarizing this up!

> However, testing this patch series out:
> 
> crw------- 1 root root 249,   0 Mar 24 11:32 /dev/nvme-generic-0c0n1
> crw------- 1 root root 249,   1 Mar 24 11:32 /dev/nvme-generic-0c0n2
> crw------- 1 root root 250,   0 Mar 24 11:32 /dev/nvme0
> brw-rw---- 1 root disk 259,   1 Mar 24 11:32 /dev/nvme0n2
> 
> NSID1 has been rejected (because of ZNS ZOC, which kernel does not support).
> 
> However, if I use the new char device for NSID1, but specify NSID2 to nvme-cli:
> 
> sudo nvme write-zeroes -s 0 -c 0 --namespace-id=2 /dev/nvme-generic-0c0n1
> 
> I was still allowed to write to NSID2:
> 
> sudo nvme zns report-zones -d 1 /dev/nvme0n2
> SLBA: 0x0        WP: 0x1        Cap: 0x3e000    State: IMP_OPENED   Type: SEQWRITE_REQ   Attrs: 0x0
> 
> Should this really be allowed?

I think this should not be allowed at all.  Thanks for the testing!

> 
> I was under the impression that Christoph's argument for implementing per
> namespace char devices, was that you should be able to do access control.
> Doesn't that mean that for the new char devices, we need to reject ioctls
> that specify a nvme_passthru_cmd.nsid != the NSID that the char device
> represents?
> 
> 
> Although, this is not really something new, as we already have the same
> behavior when it comes ioctls and the block devices. Perhaps we want to
> add the same verification there?

I think there should be verifications.

> 
> Regardless if we want to add a verification for block devices or not,
> it just seemed to me that the whole argument for introducing new char
> devices was to allow access control per namespace, which doesn't seem
> to have been taken into account, but perhaps I'm missing something.

Any other points that you think it's not been taken account?  I think it
should map to previous blkdev operations, but with some verfications
there.  It would be great if you can share any other points supposed to
be supported here :)

> Kind regards,
> Niklas

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH V6 1/2] nvme: enable char device per namespace
  2021-03-25  2:09       ` Minwoo Im
@ 2021-03-25  8:26         ` hch
  -1 siblings, 0 replies; 34+ messages in thread
From: hch @ 2021-03-25  8:26 UTC (permalink / raw)
  To: Minwoo Im
  Cc: Niklas Cassel, javier, linux-nvme, sagi, linux-block, kbusch,
	Javier González, hch

On Thu, Mar 25, 2021 at 11:09:51AM +0900, Minwoo Im wrote:
> > I was still allowed to write to NSID2:
> > 
> > sudo nvme zns report-zones -d 1 /dev/nvme0n2
> > SLBA: 0x0        WP: 0x1        Cap: 0x3e000    State: IMP_OPENED   Type: SEQWRITE_REQ   Attrs: 0x0
> > 
> > Should this really be allowed?
> 
> I think this should not be allowed at all.  Thanks for the testing!

It should not be allowed, but it seems like a pre-existing problem
as nvme_user_cmd does not verify the nsid.

> > I was under the impression that Christoph's argument for implementing per
> > namespace char devices, was that you should be able to do access control.
> > Doesn't that mean that for the new char devices, we need to reject ioctls
> > that specify a nvme_passthru_cmd.nsid != the NSID that the char device
> > represents?
> > 
> > 
> > Although, this is not really something new, as we already have the same
> > behavior when it comes ioctls and the block devices. Perhaps we want to
> > add the same verification there?
> 
> I think there should be verifications.

Yes.

> > Regardless if we want to add a verification for block devices or not,
> > it just seemed to me that the whole argument for introducing new char
> > devices was to allow access control per namespace, which doesn't seem
> > to have been taken into account, but perhaps I'm missing something.
> 
> Any other points that you think it's not been taken account?  I think it
> should map to previous blkdev operations, but with some verfications
> there.  It would be great if you can share any other points supposed to
> be supported here :)

Agreed.

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH V6 1/2] nvme: enable char device per namespace
@ 2021-03-25  8:26         ` hch
  0 siblings, 0 replies; 34+ messages in thread
From: hch @ 2021-03-25  8:26 UTC (permalink / raw)
  To: Minwoo Im
  Cc: Niklas Cassel, javier, linux-nvme, sagi, linux-block, kbusch,
	Javier González, hch

On Thu, Mar 25, 2021 at 11:09:51AM +0900, Minwoo Im wrote:
> > I was still allowed to write to NSID2:
> > 
> > sudo nvme zns report-zones -d 1 /dev/nvme0n2
> > SLBA: 0x0        WP: 0x1        Cap: 0x3e000    State: IMP_OPENED   Type: SEQWRITE_REQ   Attrs: 0x0
> > 
> > Should this really be allowed?
> 
> I think this should not be allowed at all.  Thanks for the testing!

It should not be allowed, but it seems like a pre-existing problem
as nvme_user_cmd does not verify the nsid.

> > I was under the impression that Christoph's argument for implementing per
> > namespace char devices, was that you should be able to do access control.
> > Doesn't that mean that for the new char devices, we need to reject ioctls
> > that specify a nvme_passthru_cmd.nsid != the NSID that the char device
> > represents?
> > 
> > 
> > Although, this is not really something new, as we already have the same
> > behavior when it comes ioctls and the block devices. Perhaps we want to
> > add the same verification there?
> 
> I think there should be verifications.

Yes.

> > Regardless if we want to add a verification for block devices or not,
> > it just seemed to me that the whole argument for introducing new char
> > devices was to allow access control per namespace, which doesn't seem
> > to have been taken into account, but perhaps I'm missing something.
> 
> Any other points that you think it's not been taken account?  I think it
> should map to previous blkdev operations, but with some verfications
> there.  It would be great if you can share any other points supposed to
> be supported here :)

Agreed.

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH V6 1/2] nvme: enable char device per namespace
  2021-03-25  8:26         ` hch
@ 2021-03-25  8:39           ` Niklas Cassel
  -1 siblings, 0 replies; 34+ messages in thread
From: Niklas Cassel @ 2021-03-25  8:39 UTC (permalink / raw)
  To: hch
  Cc: Minwoo Im, javier, linux-nvme, sagi, linux-block, kbusch,
	Javier González

On Thu, Mar 25, 2021 at 09:26:47AM +0100, hch@lst.de wrote:
> On Thu, Mar 25, 2021 at 11:09:51AM +0900, Minwoo Im wrote:
> > > I was still allowed to write to NSID2:
> > > 
> > > sudo nvme zns report-zones -d 1 /dev/nvme0n2
> > > SLBA: 0x0        WP: 0x1        Cap: 0x3e000    State: IMP_OPENED   Type: SEQWRITE_REQ   Attrs: 0x0
> > > 
> > > Should this really be allowed?
> > 
> > I think this should not be allowed at all.  Thanks for the testing!
> 
> It should not be allowed, but it seems like a pre-existing problem
> as nvme_user_cmd does not verify the nsid.
> 
> > > I was under the impression that Christoph's argument for implementing per
> > > namespace char devices, was that you should be able to do access control.
> > > Doesn't that mean that for the new char devices, we need to reject ioctls
> > > that specify a nvme_passthru_cmd.nsid != the NSID that the char device
> > > represents?
> > > 
> > > 
> > > Although, this is not really something new, as we already have the same
> > > behavior when it comes ioctls and the block devices. Perhaps we want to
> > > add the same verification there?
> > 
> > I think there should be verifications.
> 
> Yes.

Thanks Minwoo, Christoph,

I'll cook up a patch based on nvme/nvme-5.13.


Kind regards,
Niklas

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH V6 1/2] nvme: enable char device per namespace
@ 2021-03-25  8:39           ` Niklas Cassel
  0 siblings, 0 replies; 34+ messages in thread
From: Niklas Cassel @ 2021-03-25  8:39 UTC (permalink / raw)
  To: hch
  Cc: Minwoo Im, javier, linux-nvme, sagi, linux-block, kbusch,
	Javier González

On Thu, Mar 25, 2021 at 09:26:47AM +0100, hch@lst.de wrote:
> On Thu, Mar 25, 2021 at 11:09:51AM +0900, Minwoo Im wrote:
> > > I was still allowed to write to NSID2:
> > > 
> > > sudo nvme zns report-zones -d 1 /dev/nvme0n2
> > > SLBA: 0x0        WP: 0x1        Cap: 0x3e000    State: IMP_OPENED   Type: SEQWRITE_REQ   Attrs: 0x0
> > > 
> > > Should this really be allowed?
> > 
> > I think this should not be allowed at all.  Thanks for the testing!
> 
> It should not be allowed, but it seems like a pre-existing problem
> as nvme_user_cmd does not verify the nsid.
> 
> > > I was under the impression that Christoph's argument for implementing per
> > > namespace char devices, was that you should be able to do access control.
> > > Doesn't that mean that for the new char devices, we need to reject ioctls
> > > that specify a nvme_passthru_cmd.nsid != the NSID that the char device
> > > represents?
> > > 
> > > 
> > > Although, this is not really something new, as we already have the same
> > > behavior when it comes ioctls and the block devices. Perhaps we want to
> > > add the same verification there?
> > 
> > I think there should be verifications.
> 
> Yes.

Thanks Minwoo, Christoph,

I'll cook up a patch based on nvme/nvme-5.13.


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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH V6 1/2] nvme: enable char device per namespace
  2021-03-25  8:39           ` Niklas Cassel
@ 2021-03-25  9:34             ` Minwoo Im
  -1 siblings, 0 replies; 34+ messages in thread
From: Minwoo Im @ 2021-03-25  9:34 UTC (permalink / raw)
  To: Niklas Cassel
  Cc: hch, javier, linux-nvme, sagi, linux-block, kbusch, Javier González

On 21-03-25 08:39:40, Niklas Cassel wrote:
> On Thu, Mar 25, 2021 at 09:26:47AM +0100, hch@lst.de wrote:
> > On Thu, Mar 25, 2021 at 11:09:51AM +0900, Minwoo Im wrote:
> > > > I was still allowed to write to NSID2:
> > > > 
> > > > sudo nvme zns report-zones -d 1 /dev/nvme0n2
> > > > SLBA: 0x0        WP: 0x1        Cap: 0x3e000    State: IMP_OPENED   Type: SEQWRITE_REQ   Attrs: 0x0
> > > > 
> > > > Should this really be allowed?
> > > 
> > > I think this should not be allowed at all.  Thanks for the testing!
> > 
> > It should not be allowed, but it seems like a pre-existing problem
> > as nvme_user_cmd does not verify the nsid.
> > 
> > > > I was under the impression that Christoph's argument for implementing per
> > > > namespace char devices, was that you should be able to do access control.
> > > > Doesn't that mean that for the new char devices, we need to reject ioctls
> > > > that specify a nvme_passthru_cmd.nsid != the NSID that the char device
> > > > represents?
> > > > 
> > > > 
> > > > Although, this is not really something new, as we already have the same
> > > > behavior when it comes ioctls and the block devices. Perhaps we want to
> > > > add the same verification there?
> > > 
> > > I think there should be verifications.
> > 
> > Yes.
> 
> Thanks Minwoo, Christoph,
> 
> I'll cook up a patch based on nvme/nvme-5.13.

Just FYI: Actually Javier and I am working on this patch of the next
version and have plan to post it on this weekend maybe :)

Thanks!

> 
> 
> Kind regards,
> Niklas

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH V6 1/2] nvme: enable char device per namespace
@ 2021-03-25  9:34             ` Minwoo Im
  0 siblings, 0 replies; 34+ messages in thread
From: Minwoo Im @ 2021-03-25  9:34 UTC (permalink / raw)
  To: Niklas Cassel
  Cc: hch, javier, linux-nvme, sagi, linux-block, kbusch, Javier González

On 21-03-25 08:39:40, Niklas Cassel wrote:
> On Thu, Mar 25, 2021 at 09:26:47AM +0100, hch@lst.de wrote:
> > On Thu, Mar 25, 2021 at 11:09:51AM +0900, Minwoo Im wrote:
> > > > I was still allowed to write to NSID2:
> > > > 
> > > > sudo nvme zns report-zones -d 1 /dev/nvme0n2
> > > > SLBA: 0x0        WP: 0x1        Cap: 0x3e000    State: IMP_OPENED   Type: SEQWRITE_REQ   Attrs: 0x0
> > > > 
> > > > Should this really be allowed?
> > > 
> > > I think this should not be allowed at all.  Thanks for the testing!
> > 
> > It should not be allowed, but it seems like a pre-existing problem
> > as nvme_user_cmd does not verify the nsid.
> > 
> > > > I was under the impression that Christoph's argument for implementing per
> > > > namespace char devices, was that you should be able to do access control.
> > > > Doesn't that mean that for the new char devices, we need to reject ioctls
> > > > that specify a nvme_passthru_cmd.nsid != the NSID that the char device
> > > > represents?
> > > > 
> > > > 
> > > > Although, this is not really something new, as we already have the same
> > > > behavior when it comes ioctls and the block devices. Perhaps we want to
> > > > add the same verification there?
> > > 
> > > I think there should be verifications.
> > 
> > Yes.
> 
> Thanks Minwoo, Christoph,
> 
> I'll cook up a patch based on nvme/nvme-5.13.

Just FYI: Actually Javier and I am working on this patch of the next
version and have plan to post it on this weekend maybe :)

Thanks!

> 
> 
> Kind regards,
> Niklas

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH V6 1/2] nvme: enable char device per namespace
  2021-03-01 19:24   ` javier
@ 2021-03-25 12:25     ` Niklas Cassel
  -1 siblings, 0 replies; 34+ messages in thread
From: Niklas Cassel @ 2021-03-25 12:25 UTC (permalink / raw)
  To: javier
  Cc: linux-nvme, sagi, linux-block, minwoo.im.dev, kbusch,
	Javier González, hch

On Mon, Mar 01, 2021 at 08:24:51PM +0100, javier@javigon.com wrote:
> From: Javier González <javier.gonz@samsung.com>
> 
> Create a char device per NVMe namespace. This char device is always
> initialized, independently of whether the features implemented by the
> device are supported by the kernel. User-space can therefore always
> issue IOCTLs to the NVMe driver using the char device.
> 
> The char device is presented as /dev/nvme-generic-XcYnZ. This naming
> scheme follows the convention of the hidden device (nvmeXcYnZ). Support
> for multipath will follow.

Do we perhaps want to put these new character devices inside a subdir?
e.g. /dev/nvme/nvme-generic-XcYnZ ?

Otherwise it feels like doing such a simple thing as ls -al /dev/nvme*
will show a lot of devices because of these new specialized char devices.


Kind regards,
Niklas

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH V6 1/2] nvme: enable char device per namespace
@ 2021-03-25 12:25     ` Niklas Cassel
  0 siblings, 0 replies; 34+ messages in thread
From: Niklas Cassel @ 2021-03-25 12:25 UTC (permalink / raw)
  To: javier
  Cc: linux-nvme, sagi, linux-block, minwoo.im.dev, kbusch,
	Javier González, hch

On Mon, Mar 01, 2021 at 08:24:51PM +0100, javier@javigon.com wrote:
> From: Javier González <javier.gonz@samsung.com>
> 
> Create a char device per NVMe namespace. This char device is always
> initialized, independently of whether the features implemented by the
> device are supported by the kernel. User-space can therefore always
> issue IOCTLs to the NVMe driver using the char device.
> 
> The char device is presented as /dev/nvme-generic-XcYnZ. This naming
> scheme follows the convention of the hidden device (nvmeXcYnZ). Support
> for multipath will follow.

Do we perhaps want to put these new character devices inside a subdir?
e.g. /dev/nvme/nvme-generic-XcYnZ ?

Otherwise it feels like doing such a simple thing as ls -al /dev/nvme*
will show a lot of devices because of these new specialized char devices.


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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH V6 1/2] nvme: enable char device per namespace
  2021-03-25 12:25     ` Niklas Cassel
@ 2021-03-25 12:33       ` Minwoo Im
  -1 siblings, 0 replies; 34+ messages in thread
From: Minwoo Im @ 2021-03-25 12:33 UTC (permalink / raw)
  To: Niklas Cassel
  Cc: javier, linux-nvme, sagi, linux-block, kbusch, Javier González, hch

On 21-03-25 12:25:24, Niklas Cassel wrote:
> On Mon, Mar 01, 2021 at 08:24:51PM +0100, javier@javigon.com wrote:
> > From: Javier González <javier.gonz@samsung.com>
> > 
> > Create a char device per NVMe namespace. This char device is always
> > initialized, independently of whether the features implemented by the
> > device are supported by the kernel. User-space can therefore always
> > issue IOCTLs to the NVMe driver using the char device.
> > 
> > The char device is presented as /dev/nvme-generic-XcYnZ. This naming
> > scheme follows the convention of the hidden device (nvmeXcYnZ). Support
> > for multipath will follow.
> 
> Do we perhaps want to put these new character devices inside a subdir?
> e.g. /dev/nvme/nvme-generic-XcYnZ ?
> 
> Otherwise it feels like doing such a simple thing as ls -al /dev/nvme*
> will show a lot of devices because of these new specialized char devices.

Good point.  If we have thousands of namespaces, it will create 2 times
of the blkdevs.  I would hear what maintainers say about this :)

FYI: new version has been posted with V1 by re-create this series with
Javier.  Please have a discuss there:

    https://lore.kernel.org/linux-nvme/20210325123048.94784-1-minwoo.im.dev@gmail.com/T/#u

Thanks!

> 
> 
> Kind regards,
> Niklas

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH V6 1/2] nvme: enable char device per namespace
@ 2021-03-25 12:33       ` Minwoo Im
  0 siblings, 0 replies; 34+ messages in thread
From: Minwoo Im @ 2021-03-25 12:33 UTC (permalink / raw)
  To: Niklas Cassel
  Cc: javier, linux-nvme, sagi, linux-block, kbusch, Javier González, hch

On 21-03-25 12:25:24, Niklas Cassel wrote:
> On Mon, Mar 01, 2021 at 08:24:51PM +0100, javier@javigon.com wrote:
> > From: Javier González <javier.gonz@samsung.com>
> > 
> > Create a char device per NVMe namespace. This char device is always
> > initialized, independently of whether the features implemented by the
> > device are supported by the kernel. User-space can therefore always
> > issue IOCTLs to the NVMe driver using the char device.
> > 
> > The char device is presented as /dev/nvme-generic-XcYnZ. This naming
> > scheme follows the convention of the hidden device (nvmeXcYnZ). Support
> > for multipath will follow.
> 
> Do we perhaps want to put these new character devices inside a subdir?
> e.g. /dev/nvme/nvme-generic-XcYnZ ?
> 
> Otherwise it feels like doing such a simple thing as ls -al /dev/nvme*
> will show a lot of devices because of these new specialized char devices.

Good point.  If we have thousands of namespaces, it will create 2 times
of the blkdevs.  I would hear what maintainers say about this :)

FYI: new version has been posted with V1 by re-create this series with
Javier.  Please have a discuss there:

    https://lore.kernel.org/linux-nvme/20210325123048.94784-1-minwoo.im.dev@gmail.com/T/#u

Thanks!

> 
> 
> Kind regards,
> Niklas

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH V6 1/2] nvme: enable char device per namespace
  2021-03-25 12:25     ` Niklas Cassel
@ 2021-03-25 15:14       ` Keith Busch
  -1 siblings, 0 replies; 34+ messages in thread
From: Keith Busch @ 2021-03-25 15:14 UTC (permalink / raw)
  To: Niklas Cassel
  Cc: javier, linux-nvme, sagi, linux-block, minwoo.im.dev,
	Javier González, hch

On Thu, Mar 25, 2021 at 12:25:24PM +0000, Niklas Cassel wrote:
> On Mon, Mar 01, 2021 at 08:24:51PM +0100, javier@javigon.com wrote:
> > From: Javier González <javier.gonz@samsung.com>
> > 
> > Create a char device per NVMe namespace. This char device is always
> > initialized, independently of whether the features implemented by the
> > device are supported by the kernel. User-space can therefore always
> > issue IOCTLs to the NVMe driver using the char device.
> > 
> > The char device is presented as /dev/nvme-generic-XcYnZ. This naming
> > scheme follows the convention of the hidden device (nvmeXcYnZ). Support
> > for multipath will follow.
> 
> Do we perhaps want to put these new character devices inside a subdir?
> e.g. /dev/nvme/nvme-generic-XcYnZ ?

I actually suggested the same hierarchy, but that was rejected.
 
> Otherwise it feels like doing such a simple thing as ls -al /dev/nvme*
> will show a lot of devices because of these new specialized char devices.

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH V6 1/2] nvme: enable char device per namespace
@ 2021-03-25 15:14       ` Keith Busch
  0 siblings, 0 replies; 34+ messages in thread
From: Keith Busch @ 2021-03-25 15:14 UTC (permalink / raw)
  To: Niklas Cassel
  Cc: javier, linux-nvme, sagi, linux-block, minwoo.im.dev,
	Javier González, hch

On Thu, Mar 25, 2021 at 12:25:24PM +0000, Niklas Cassel wrote:
> On Mon, Mar 01, 2021 at 08:24:51PM +0100, javier@javigon.com wrote:
> > From: Javier González <javier.gonz@samsung.com>
> > 
> > Create a char device per NVMe namespace. This char device is always
> > initialized, independently of whether the features implemented by the
> > device are supported by the kernel. User-space can therefore always
> > issue IOCTLs to the NVMe driver using the char device.
> > 
> > The char device is presented as /dev/nvme-generic-XcYnZ. This naming
> > scheme follows the convention of the hidden device (nvmeXcYnZ). Support
> > for multipath will follow.
> 
> Do we perhaps want to put these new character devices inside a subdir?
> e.g. /dev/nvme/nvme-generic-XcYnZ ?

I actually suggested the same hierarchy, but that was rejected.
 
> Otherwise it feels like doing such a simple thing as ls -al /dev/nvme*
> will show a lot of devices because of these new specialized char devices.

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

end of thread, other threads:[~2021-03-25 15:15 UTC | newest]

Thread overview: 34+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-01 19:24 [PATCH V6 0/2] nvme: enable char device per namespace javier
2021-03-01 19:24 ` javier
2021-03-01 19:24 ` [PATCH V6 1/2] " javier
2021-03-01 19:24   ` javier
2021-03-03  9:10   ` Christoph Hellwig
2021-03-03  9:10     ` Christoph Hellwig
2021-03-03 10:02     ` Javier González
2021-03-03 10:02       ` Javier González
2021-03-09 11:31       ` Christoph Hellwig
2021-03-09 11:31         ` Christoph Hellwig
2021-03-09 12:42         ` Javier González
2021-03-09 12:42           ` Javier González
2021-03-09 15:05           ` Christoph Hellwig
2021-03-09 15:05             ` Christoph Hellwig
2021-03-09 21:18             ` Javier González
2021-03-09 21:18               ` Javier González
2021-03-24 12:29   ` Niklas Cassel
2021-03-24 12:29     ` Niklas Cassel
2021-03-25  2:09     ` Minwoo Im
2021-03-25  2:09       ` Minwoo Im
2021-03-25  8:26       ` hch
2021-03-25  8:26         ` hch
2021-03-25  8:39         ` Niklas Cassel
2021-03-25  8:39           ` Niklas Cassel
2021-03-25  9:34           ` Minwoo Im
2021-03-25  9:34             ` Minwoo Im
2021-03-25 12:25   ` Niklas Cassel
2021-03-25 12:25     ` Niklas Cassel
2021-03-25 12:33     ` Minwoo Im
2021-03-25 12:33       ` Minwoo Im
2021-03-25 15:14     ` Keith Busch
2021-03-25 15:14       ` Keith Busch
2021-03-01 19:24 ` [PATCH V6 2/2] nvme: allow open for nvme-generic char device javier
2021-03-01 19:24   ` javier

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.