* [PATCH 0/1] nvme: introduce generic per-namespace chardev
@ 2021-03-25 12:30 Minwoo Im
2021-03-25 12:30 ` [PATCH 1/1] " Minwoo Im
0 siblings, 1 reply; 4+ messages in thread
From: Minwoo Im @ 2021-03-25 12:30 UTC (permalink / raw)
To: linux-nvme
Cc: Keith Busch, Jens Axboe, Christoph Hellwig, Sagi Grimberg,
Javier González, Minwoo Im
Hello,
This patch introduces per-namespace character device to I/O in case that
blkdev is not initialized properly. Userspace applications are able to
I/O to the generic namespace chardev even there's no blkdev properly
initialized.
This patch is derived from Javier's patch series [1]. Changes from the
previous version are:
- Update naming convention for the chardev exactly the same with the
blkdev:
/dev/nvme-generic-XcYnZ to /dev/nvme-generic-XnY
- Re-consider the multipath support. Rather than exposing all the
path-specific namespace path with controller(e.g., nvmeXcYnZ), only
expose head namespace instance in case of multipath. Otherwise, it
will expose all the blkdevs created.
[1] https://lore.kernel.org/linux-nvme/20210301192452.16770-1-javier.gonz@samsung.com/
Minwoo Im (1):
nvme: introduce generic per-namespace chardev
drivers/nvme/host/core.c | 170 +++++++++++++++++++++++++++++++++-
drivers/nvme/host/multipath.c | 17 ++++
drivers/nvme/host/nvme.h | 25 +++++
3 files changed, 209 insertions(+), 3 deletions(-)
--
2.27.0
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH 1/1] nvme: introduce generic per-namespace chardev
2021-03-25 12:30 [PATCH 0/1] nvme: introduce generic per-namespace chardev Minwoo Im
@ 2021-03-25 12:30 ` Minwoo Im
2021-03-29 8:31 ` Kanchan Joshi
0 siblings, 1 reply; 4+ messages in thread
From: Minwoo Im @ 2021-03-25 12:30 UTC (permalink / raw)
To: linux-nvme
Cc: Keith Busch, Jens Axboe, Christoph Hellwig, Sagi Grimberg,
Javier González, Minwoo Im
Userspace has not been allowed to I/O to device that's failed to
be initialized. This patch introduces generic per-namespace character
device to allow userspace to I/O regardless the block device is there or
not.
The chardev naming convention will exactly be the same with the existing
blkdev's one. nvme_set_generic_ns_name() introduced in this patch may
look like a duplication of nvme_set_disk_name(), but it's not allowing
controller path specific. It just for per-namespace path specific. So,
the naming will be:
- /dev/nvme-generic-XnY
It also supports multipath which means it will not expose chardev for
the hidden namespace blkdevs (e.g., nvmeXcYnZ). If
/dev/nvme-generic-XnY is created for the ns_head, then I/O request will
be routed to the controller-specified path by the iopolicy in the
subsystem.
This can be controlled by the module parameter `generic_ns` which is
turned on by default.
Signed-off-by: Minwoo Im <minwoo.im.dev@gmail.com>
Signed-off-by: Javier González <javier.gonz@samsung.com>
---
drivers/nvme/host/core.c | 170 +++++++++++++++++++++++++++++++++-
drivers/nvme/host/multipath.c | 17 ++++
drivers/nvme/host/nvme.h | 25 +++++
3 files changed, 209 insertions(+), 3 deletions(-)
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index c371db47de3c..7dc0e216524b 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -61,6 +61,10 @@ static bool streams;
module_param(streams, bool, 0644);
MODULE_PARM_DESC(streams, "turn on support for Streams write directives");
+static bool generic_ns = true;
+module_param(generic_ns, bool, 0644);
+MODULE_PARM_DESC(generic_ns, "support generic namespace character device");
+
/*
* nvme_wq - hosts nvme related works that are not reset or delete
* nvme_reset_wq - hosts nvme reset works
@@ -85,8 +89,11 @@ static LIST_HEAD(nvme_subsystems);
static DEFINE_MUTEX(nvme_subsystems_lock);
static DEFINE_IDA(nvme_instance_ida);
+static DEFINE_IDA(nvme_generic_ns_minor_ida);
static dev_t nvme_ctrl_base_chr_devt;
+static dev_t nvme_generic_ns_devt;
static struct class *nvme_class;
+static struct class *nvme_generic_ns_class;
static struct class *nvme_subsys_class;
static void nvme_put_subsystem(struct nvme_subsystem *subsys);
@@ -542,6 +549,7 @@ static void nvme_free_ns_head(struct kref *ref)
struct nvme_ns_head *head =
container_of(ref, struct nvme_ns_head, ref);
+ cdev_device_del(&head->generic_ns->cdev, &head->generic_ns->device);
nvme_mpath_remove_disk(head);
ida_simple_remove(&head->subsys->ns_ida, head->instance);
cleanup_srcu_struct(&head->srcu);
@@ -3784,6 +3792,126 @@ static int __nvme_check_ids(struct nvme_subsystem *subsys,
return 0;
}
+#ifdef CONFIG_NVME_MULTIPATH
+static int nvme_generic_ns_open(struct inode *inode, struct file *file)
+{
+ struct nvme_generic_ns *generic_ns = container_of(inode->i_cdev,
+ struct nvme_generic_ns, cdev);
+
+ if (!generic_ns->head->disk)
+ return -ENODEV;
+
+ file->private_data = generic_ns;
+ return nvme_ns_head_open(generic_ns->head->disk->part0, file->f_mode);
+}
+
+static int nvme_generic_ns_release(struct inode *inode, struct file *file)
+{
+ struct nvme_generic_ns *generic_ns = container_of(inode->i_cdev,
+ struct nvme_generic_ns, cdev);
+
+ nvme_ns_head_release(generic_ns->head->disk, file->f_mode);
+
+ return 0;
+}
+
+static long nvme_generic_ns_ioctl(struct file *file, unsigned int cmd,
+ unsigned long arg)
+{
+ struct nvme_generic_ns *generic_ns = file->private_data;
+
+ return nvme_ioctl(generic_ns->head->disk->part0, file->f_mode, cmd, arg);
+}
+#else
+static int nvme_generic_ns_open(struct inode *inode, struct file *file)
+{
+ struct nvme_generic_ns *generic_ns = container_of(inode->i_cdev,
+ struct nvme_generic_ns, cdev);
+
+ if (!generic_ns->ns)
+ return -ENODEV;
+
+ file->private_data = generic_ns;
+ return nvme_open(generic_ns->ns->disk->part0, file->f_mode);
+}
+
+static int nvme_generic_ns_release(struct inode *inode, struct file *file)
+{
+ struct nvme_generic_ns *generic_ns = container_of(inode->i_cdev,
+ struct nvme_generic_ns, cdev);
+
+ nvme_release(generic_ns->ns->disk, file->f_mode);
+
+ return 0;
+}
+
+static long nvme_generic_ns_ioctl(struct file *file, unsigned int cmd,
+ unsigned long arg)
+{
+ struct nvme_generic_ns *generic_ns = file->private_data;
+
+ return nvme_ioctl(generic_ns->ns->disk->part0, file->f_mode, cmd, arg);
+}
+#endif
+
+static const struct file_operations nvme_generic_ns_fops = {
+ .owner = THIS_MODULE,
+ .open = nvme_generic_ns_open,
+ .release = nvme_generic_ns_release,
+ .unlocked_ioctl = nvme_generic_ns_ioctl,
+ .compat_ioctl = compat_ptr_ioctl,
+};
+
+static int nvme_alloc_generic_ns(struct nvme_ctrl *ctrl,
+ struct nvme_ns_head *head, struct nvme_ns *ns)
+{
+ struct nvme_generic_ns *generic_ns;
+ char name[DISK_NAME_LEN];
+ int minor;
+ int ret;
+
+ generic_ns = kzalloc_node(sizeof(*generic_ns), GFP_KERNEL,
+ ctrl->numa_node);
+ if (!generic_ns)
+ return -ENOMEM;
+
+ ret = ida_simple_get(&nvme_generic_ns_minor_ida, 0, 0, GFP_KERNEL);
+ if (ret < 0)
+ goto free_ns;
+ minor = ret;
+
+ device_initialize(&generic_ns->device);
+ generic_ns->device.devt = MKDEV(MAJOR(nvme_generic_ns_devt), minor);
+ generic_ns->device.class = nvme_generic_ns_class;
+ generic_ns->device.parent = ctrl->device;
+ dev_set_drvdata(&generic_ns->device, generic_ns);
+
+ nvme_set_generic_ns_name(name, head, ctrl);
+ ret = dev_set_name(&generic_ns->device, "%s", name);
+ if (ret)
+ goto put_ida;
+
+ cdev_init(&generic_ns->cdev, &nvme_generic_ns_fops);
+ generic_ns->cdev.owner = ctrl->ops->module;
+
+ ret = cdev_device_add(&generic_ns->cdev, &generic_ns->device);
+ if (ret)
+ goto free_kobj;
+
+ head->generic_ns = generic_ns;
+ generic_ns->head = head;
+ generic_ns->ns = ns;
+ return 0;
+
+free_kobj:
+ kfree_const(generic_ns->device.kobj.name);
+put_ida:
+ ida_simple_remove(&nvme_generic_ns_minor_ida, minor);
+free_ns:
+ kfree(generic_ns);
+ return ret;
+}
+
static struct nvme_ns_head *nvme_alloc_ns_head(struct nvme_ctrl *ctrl,
unsigned nsid, struct nvme_ns_ids *ids)
{
@@ -3862,6 +3990,13 @@ static int nvme_init_ns_head(struct nvme_ns *ns, unsigned nsid,
goto out_unlock;
}
head->shared = is_shared;
+
+ if (generic_ns && nvme_alloc_generic_ns(ctrl, head, ns)) {
+ dev_err(ctrl->device,
+ "Failed to initialize generic namespace %d\n",
+ nsid);
+ goto out_put_ns_head;
+ }
} else {
ret = -EINVAL;
if (!is_shared || !head->shared) {
@@ -3963,8 +4098,17 @@ 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 generic namespae device to be
+ * craeted.
+ */
+ if (nvme_update_ns_info(ns, id)) {
+ if (generic_ns)
+ 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)) {
@@ -3983,6 +4127,7 @@ static void nvme_alloc_ns(struct nvme_ctrl *ctrl, unsigned nsid,
nvme_mpath_add_disk(ns, id);
nvme_fault_inject_init(&ns->fault_inject, ns->disk->disk_name);
+
kfree(id);
return;
@@ -4802,13 +4947,31 @@ static int __init nvme_core_init(void)
}
nvme_class->dev_uevent = nvme_class_uevent;
+ if (!generic_ns) return 0;
+
+ result = alloc_chrdev_region(&nvme_generic_ns_devt, 0,
+ NVME_MINORS, "nvme-generic-ns");
+ if (result < 0)
+ goto destroy_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 unregister_generic_ns;
}
+
+ nvme_generic_ns_class = class_create(THIS_MODULE, "nvme-generic-ns");
+ if (IS_ERR(nvme_generic_ns_class)) {
+ result = PTR_ERR(nvme_subsys_class);
+ goto destroy_subsys_class;
+ }
+
return 0;
+destroy_subsys_class:
+ class_destroy(nvme_subsys_class);
+unregister_generic_ns:
+ unregister_chrdev_region(nvme_generic_ns_devt, NVME_MINORS);
destroy_class:
class_destroy(nvme_class);
unregister_chrdev:
@@ -4827,6 +4990,7 @@ static void __exit nvme_core_exit(void)
{
class_destroy(nvme_subsys_class);
class_destroy(nvme_class);
+ unregister_chrdev_region(nvme_generic_ns_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/multipath.c b/drivers/nvme/host/multipath.c
index a1d476e1ac02..4ec17d99de8e 100644
--- a/drivers/nvme/host/multipath.c
+++ b/drivers/nvme/host/multipath.c
@@ -65,6 +65,23 @@ void nvme_set_disk_name(char *disk_name, struct nvme_ns *ns,
}
}
+/*
+ * It sets generic namespace chardev name based on the given namespace head.
+ * The naming convention follows the existing namespace disk name from
+ * nvme_set_disk_name(), but not allowing controller path specific.
+ */
+void nvme_set_generic_ns_name(char *dev_name, struct nvme_ns_head *head,
+ struct nvme_ctrl *ctrl)
+{
+ if (!multipath) {
+ sprintf(dev_name, "nvme-generic-%dn%d",
+ ctrl->instance, head->instance);
+ } else {
+ sprintf(dev_name, "nvme-generic-%dn%d",
+ ctrl->subsys->instance, head->instance);
+ }
+}
+
void nvme_failover_req(struct request *req)
{
struct nvme_ns *ns = req->q->queuedata;
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index 76de7ed55d90..97a4a48f78ed 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -410,6 +410,8 @@ struct nvme_ns_head {
bool shared;
int instance;
struct nvme_effects_log *effects;
+ struct nvme_generic_ns *generic_ns;
+
#ifdef CONFIG_NVME_MULTIPATH
struct gendisk *disk;
struct bio_list requeue_list;
@@ -461,6 +463,20 @@ struct nvme_ns {
};
+/*
+ * Generic character device for a namespace. This device is to support I/O to
+ * namespace even it's failed to initialize due to some reasons (e.g.,
+ * unsupported protection information, etc).
+ */
+struct nvme_generic_ns {
+ struct device device;
+ struct cdev cdev;
+ struct nvme_ns_head *head;
+
+ /* targted namespace instance. only valid in non-multipath */
+ struct nvme_ns *ns;
+};
+
/* NVMe ns supports metadata actions by the controller (generate/strip) */
static inline bool nvme_ns_has_pi(struct nvme_ns *ns)
{
@@ -663,6 +679,8 @@ void nvme_mpath_wait_freeze(struct nvme_subsystem *subsys);
void nvme_mpath_start_freeze(struct nvme_subsystem *subsys);
void nvme_set_disk_name(char *disk_name, struct nvme_ns *ns,
struct nvme_ctrl *ctrl, int *flags);
+void nvme_set_generic_ns_name(char *dev_name, struct nvme_ns_head *head,
+ struct nvme_ctrl *ctrl);
void nvme_failover_req(struct request *req);
void nvme_kick_requeue_lists(struct nvme_ctrl *ctrl);
int nvme_mpath_alloc_disk(struct nvme_ctrl *ctrl,struct nvme_ns_head *head);
@@ -711,6 +729,13 @@ static inline void nvme_set_disk_name(char *disk_name, struct nvme_ns *ns,
sprintf(disk_name, "nvme%dn%d", ctrl->instance, ns->head->instance);
}
+static inline void nvme_set_generic_ns_name(char *dev_name,
+ struct nvme_ns_head *head, struct nvme_ctrl *ctrl)
+{
+ sprintf(dev_name, "nvme-generic-%dn%d", ctrl->instance,
+ head->instance);
+}
+
static inline void nvme_failover_req(struct request *req)
{
}
--
2.27.0
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 1/1] nvme: introduce generic per-namespace chardev
2021-03-25 12:30 ` [PATCH 1/1] " Minwoo Im
@ 2021-03-29 8:31 ` Kanchan Joshi
2021-03-29 11:56 ` Minwoo Im
0 siblings, 1 reply; 4+ messages in thread
From: Kanchan Joshi @ 2021-03-29 8:31 UTC (permalink / raw)
To: Minwoo Im
Cc: linux-nvme, Keith Busch, Jens Axboe, Christoph Hellwig,
Sagi Grimberg, Javier González
On Thu, Mar 25, 2021 at 6:08 PM Minwoo Im <minwoo.im.dev@gmail.com> wrote:
>
> Userspace has not been allowed to I/O to device that's failed to
> be initialized. This patch introduces generic per-namespace character
> device to allow userspace to I/O regardless the block device is there or
> not.
>
> The chardev naming convention will exactly be the same with the existing
> blkdev's one. nvme_set_generic_ns_name() introduced in this patch may
> look like a duplication of nvme_set_disk_name(), but it's not allowing
> controller path specific. It just for per-namespace path specific. So,
> the naming will be:
>
> - /dev/nvme-generic-XnY
>
> It also supports multipath which means it will not expose chardev for
> the hidden namespace blkdevs (e.g., nvmeXcYnZ). If
> /dev/nvme-generic-XnY is created for the ns_head, then I/O request will
> be routed to the controller-specified path by the iopolicy in the
> subsystem.
>
> This can be controlled by the module parameter `generic_ns` which is
> turned on by default.
>
> Signed-off-by: Minwoo Im <minwoo.im.dev@gmail.com>
> Signed-off-by: Javier González <javier.gonz@samsung.com>
> ---
> drivers/nvme/host/core.c | 170 +++++++++++++++++++++++++++++++++-
> drivers/nvme/host/multipath.c | 17 ++++
> drivers/nvme/host/nvme.h | 25 +++++
> 3 files changed, 209 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index c371db47de3c..7dc0e216524b 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -61,6 +61,10 @@ static bool streams;
> module_param(streams, bool, 0644);
> MODULE_PARM_DESC(streams, "turn on support for Streams write directives");
>
> +static bool generic_ns = true;
> +module_param(generic_ns, bool, 0644);
> +MODULE_PARM_DESC(generic_ns, "support generic namespace character device");
> +
> /*
> * nvme_wq - hosts nvme related works that are not reset or delete
> * nvme_reset_wq - hosts nvme reset works
> @@ -85,8 +89,11 @@ static LIST_HEAD(nvme_subsystems);
> static DEFINE_MUTEX(nvme_subsystems_lock);
>
> static DEFINE_IDA(nvme_instance_ida);
> +static DEFINE_IDA(nvme_generic_ns_minor_ida);
> static dev_t nvme_ctrl_base_chr_devt;
> +static dev_t nvme_generic_ns_devt;
> static struct class *nvme_class;
> +static struct class *nvme_generic_ns_class;
> static struct class *nvme_subsys_class;
>
> static void nvme_put_subsystem(struct nvme_subsystem *subsys);
> @@ -542,6 +549,7 @@ static void nvme_free_ns_head(struct kref *ref)
> struct nvme_ns_head *head =
> container_of(ref, struct nvme_ns_head, ref);
>
> + cdev_device_del(&head->generic_ns->cdev, &head->generic_ns->device);
> nvme_mpath_remove_disk(head);
> ida_simple_remove(&head->subsys->ns_ida, head->instance);
> cleanup_srcu_struct(&head->srcu);
> @@ -3784,6 +3792,126 @@ static int __nvme_check_ids(struct nvme_subsystem *subsys,
> return 0;
> }
>
> +#ifdef CONFIG_NVME_MULTIPATH
> +static int nvme_generic_ns_open(struct inode *inode, struct file *file)
> +{
> + struct nvme_generic_ns *generic_ns = container_of(inode->i_cdev,
> + struct nvme_generic_ns, cdev);
> +
> + if (!generic_ns->head->disk)
> + return -ENODEV;
> +
> + file->private_data = generic_ns;
> + return nvme_ns_head_open(generic_ns->head->disk->part0, file->f_mode);
> +}
> +
> +static int nvme_generic_ns_release(struct inode *inode, struct file *file)
> +{
> + struct nvme_generic_ns *generic_ns = container_of(inode->i_cdev,
> + struct nvme_generic_ns, cdev);
> +
> + nvme_ns_head_release(generic_ns->head->disk, file->f_mode);
> +
> + return 0;
> +}
> +
> +static long nvme_generic_ns_ioctl(struct file *file, unsigned int cmd,
> + unsigned long arg)
> +{
> + struct nvme_generic_ns *generic_ns = file->private_data;
> +
> + return nvme_ioctl(generic_ns->head->disk->part0, file->f_mode, cmd, arg);
> +}
> +#else
> +static int nvme_generic_ns_open(struct inode *inode, struct file *file)
> +{
> + struct nvme_generic_ns *generic_ns = container_of(inode->i_cdev,
> + struct nvme_generic_ns, cdev);
> +
> + if (!generic_ns->ns)
> + return -ENODEV;
> +
> + file->private_data = generic_ns;
> + return nvme_open(generic_ns->ns->disk->part0, file->f_mode);
> +}
> +
> +static int nvme_generic_ns_release(struct inode *inode, struct file *file)
> +{
> + struct nvme_generic_ns *generic_ns = container_of(inode->i_cdev,
> + struct nvme_generic_ns, cdev);
> +
> + nvme_release(generic_ns->ns->disk, file->f_mode);
> +
> + return 0;
> +}
> +
> +static long nvme_generic_ns_ioctl(struct file *file, unsigned int cmd,
> + unsigned long arg)
> +{
> + struct nvme_generic_ns *generic_ns = file->private_data;
> +
> + return nvme_ioctl(generic_ns->ns->disk->part0, file->f_mode, cmd, arg);
> +}
> +#endif
> +
> +static const struct file_operations nvme_generic_ns_fops = {
> + .owner = THIS_MODULE,
> + .open = nvme_generic_ns_open,
> + .release = nvme_generic_ns_release,
> + .unlocked_ioctl = nvme_generic_ns_ioctl,
> + .compat_ioctl = compat_ptr_ioctl,
> +};
> +
> +static int nvme_alloc_generic_ns(struct nvme_ctrl *ctrl,
> + struct nvme_ns_head *head, struct nvme_ns *ns)
> +{
> + struct nvme_generic_ns *generic_ns;
> + char name[DISK_NAME_LEN];
> + int minor;
> + int ret;
> +
> + generic_ns = kzalloc_node(sizeof(*generic_ns), GFP_KERNEL,
> + ctrl->numa_node);
> + if (!generic_ns)
> + return -ENOMEM;
> +
> + ret = ida_simple_get(&nvme_generic_ns_minor_ida, 0, 0, GFP_KERNEL);
> + if (ret < 0)
> + goto free_ns;
> + minor = ret;
> +
> + device_initialize(&generic_ns->device);
> + generic_ns->device.devt = MKDEV(MAJOR(nvme_generic_ns_devt), minor);
> + generic_ns->device.class = nvme_generic_ns_class;
> + generic_ns->device.parent = ctrl->device;
> + dev_set_drvdata(&generic_ns->device, generic_ns);
> +
> + nvme_set_generic_ns_name(name, head, ctrl);
> + ret = dev_set_name(&generic_ns->device, "%s", name);
> + if (ret)
> + goto put_ida;
> +
> + cdev_init(&generic_ns->cdev, &nvme_generic_ns_fops);
> + generic_ns->cdev.owner = ctrl->ops->module;
> +
> + ret = cdev_device_add(&generic_ns->cdev, &generic_ns->device);
> + if (ret)
> + goto free_kobj;
> +
> + head->generic_ns = generic_ns;
> + generic_ns->head = head;
> + generic_ns->ns = ns;
> + return 0;
> +
> +free_kobj:
> + kfree_const(generic_ns->device.kobj.name);
> +put_ida:
> + ida_simple_remove(&nvme_generic_ns_minor_ida, minor);
> +free_ns:
> + kfree(generic_ns);
> + return ret;
> +}
> +
> static struct nvme_ns_head *nvme_alloc_ns_head(struct nvme_ctrl *ctrl,
> unsigned nsid, struct nvme_ns_ids *ids)
> {
> @@ -3862,6 +3990,13 @@ static int nvme_init_ns_head(struct nvme_ns *ns, unsigned nsid,
> goto out_unlock;
> }
> head->shared = is_shared;
> +
> + if (generic_ns && nvme_alloc_generic_ns(ctrl, head, ns)) {
> + dev_err(ctrl->device,
> + "Failed to initialize generic namespace %d\n",
> + nsid);
> + goto out_put_ns_head;
> + }
> } else {
> ret = -EINVAL;
> if (!is_shared || !head->shared) {
> @@ -3963,8 +4098,17 @@ 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 generic namespae device to be
> + * craeted.
> + */
> + if (nvme_update_ns_info(ns, id)) {
> + if (generic_ns)
> + 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)) {
> @@ -3983,6 +4127,7 @@ static void nvme_alloc_ns(struct nvme_ctrl *ctrl, unsigned nsid,
>
> nvme_mpath_add_disk(ns, id);
> nvme_fault_inject_init(&ns->fault_inject, ns->disk->disk_name);
> +
> kfree(id);
>
> return;
> @@ -4802,13 +4947,31 @@ static int __init nvme_core_init(void)
> }
> nvme_class->dev_uevent = nvme_class_uevent;
>
> + if (!generic_ns) return 0;
This will suppress creation of nvme_subsys_class when generic_ns
parameter is set as 0.
> + result = alloc_chrdev_region(&nvme_generic_ns_devt, 0,
> + NVME_MINORS, "nvme-generic-ns");
> + if (result < 0)
> + goto destroy_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 unregister_generic_ns;
> }
> +
> + nvme_generic_ns_class = class_create(THIS_MODULE, "nvme-generic-ns");
> + if (IS_ERR(nvme_generic_ns_class)) {
> + result = PTR_ERR(nvme_subsys_class);
> + goto destroy_subsys_class;
> + }
> +
> return 0;
>
> +destroy_subsys_class:
> + class_destroy(nvme_subsys_class);
> +unregister_generic_ns:
> + unregister_chrdev_region(nvme_generic_ns_devt, NVME_MINORS);
> destroy_class:
> class_destroy(nvme_class);
> unregister_chrdev:
> @@ -4827,6 +4990,7 @@ static void __exit nvme_core_exit(void)
> {
> class_destroy(nvme_subsys_class);
> class_destroy(nvme_class);
> + unregister_chrdev_region(nvme_generic_ns_devt, NVME_MINORS);
> unregister_chrdev_region(nvme_ctrl_base_chr_devt, NVME_MINORS);
> destroy_workqueue(nvme_delete_wq);
> destroy_workqueue(nvme_reset_wq);
The "nvme-generic-ns" class has not been destroyed here.
--
Kanchan
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 1/1] nvme: introduce generic per-namespace chardev
2021-03-29 8:31 ` Kanchan Joshi
@ 2021-03-29 11:56 ` Minwoo Im
0 siblings, 0 replies; 4+ messages in thread
From: Minwoo Im @ 2021-03-29 11:56 UTC (permalink / raw)
To: Kanchan Joshi
Cc: linux-nvme, Keith Busch, Jens Axboe, Christoph Hellwig,
Sagi Grimberg, Javier González
On 21-03-29 14:01:20, Kanchan Joshi wrote:
> On Thu, Mar 25, 2021 at 6:08 PM Minwoo Im <minwoo.im.dev@gmail.com> wrote:
> >
> > Userspace has not been allowed to I/O to device that's failed to
> > be initialized. This patch introduces generic per-namespace character
> > device to allow userspace to I/O regardless the block device is there or
> > not.
> >
> > The chardev naming convention will exactly be the same with the existing
> > blkdev's one. nvme_set_generic_ns_name() introduced in this patch may
> > look like a duplication of nvme_set_disk_name(), but it's not allowing
> > controller path specific. It just for per-namespace path specific. So,
> > the naming will be:
> >
> > - /dev/nvme-generic-XnY
> >
> > It also supports multipath which means it will not expose chardev for
> > the hidden namespace blkdevs (e.g., nvmeXcYnZ). If
> > /dev/nvme-generic-XnY is created for the ns_head, then I/O request will
> > be routed to the controller-specified path by the iopolicy in the
> > subsystem.
> >
> > This can be controlled by the module parameter `generic_ns` which is
> > turned on by default.
> >
> > Signed-off-by: Minwoo Im <minwoo.im.dev@gmail.com>
> > Signed-off-by: Javier González <javier.gonz@samsung.com>
> > ---
> > drivers/nvme/host/core.c | 170 +++++++++++++++++++++++++++++++++-
> > drivers/nvme/host/multipath.c | 17 ++++
> > drivers/nvme/host/nvme.h | 25 +++++
> > 3 files changed, 209 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> > index c371db47de3c..7dc0e216524b 100644
> > --- a/drivers/nvme/host/core.c
> > +++ b/drivers/nvme/host/core.c
> > @@ -61,6 +61,10 @@ static bool streams;
> > module_param(streams, bool, 0644);
> > MODULE_PARM_DESC(streams, "turn on support for Streams write directives");
> >
> > +static bool generic_ns = true;
> > +module_param(generic_ns, bool, 0644);
> > +MODULE_PARM_DESC(generic_ns, "support generic namespace character device");
> > +
> > /*
> > * nvme_wq - hosts nvme related works that are not reset or delete
> > * nvme_reset_wq - hosts nvme reset works
> > @@ -85,8 +89,11 @@ static LIST_HEAD(nvme_subsystems);
> > static DEFINE_MUTEX(nvme_subsystems_lock);
> >
> > static DEFINE_IDA(nvme_instance_ida);
> > +static DEFINE_IDA(nvme_generic_ns_minor_ida);
> > static dev_t nvme_ctrl_base_chr_devt;
> > +static dev_t nvme_generic_ns_devt;
> > static struct class *nvme_class;
> > +static struct class *nvme_generic_ns_class;
> > static struct class *nvme_subsys_class;
> >
> > static void nvme_put_subsystem(struct nvme_subsystem *subsys);
> > @@ -542,6 +549,7 @@ static void nvme_free_ns_head(struct kref *ref)
> > struct nvme_ns_head *head =
> > container_of(ref, struct nvme_ns_head, ref);
> >
> > + cdev_device_del(&head->generic_ns->cdev, &head->generic_ns->device);
> > nvme_mpath_remove_disk(head);
> > ida_simple_remove(&head->subsys->ns_ida, head->instance);
> > cleanup_srcu_struct(&head->srcu);
> > @@ -3784,6 +3792,126 @@ static int __nvme_check_ids(struct nvme_subsystem *subsys,
> > return 0;
> > }
> >
> > +#ifdef CONFIG_NVME_MULTIPATH
> > +static int nvme_generic_ns_open(struct inode *inode, struct file *file)
> > +{
> > + struct nvme_generic_ns *generic_ns = container_of(inode->i_cdev,
> > + struct nvme_generic_ns, cdev);
> > +
> > + if (!generic_ns->head->disk)
> > + return -ENODEV;
> > +
> > + file->private_data = generic_ns;
> > + return nvme_ns_head_open(generic_ns->head->disk->part0, file->f_mode);
> > +}
> > +
> > +static int nvme_generic_ns_release(struct inode *inode, struct file *file)
> > +{
> > + struct nvme_generic_ns *generic_ns = container_of(inode->i_cdev,
> > + struct nvme_generic_ns, cdev);
> > +
> > + nvme_ns_head_release(generic_ns->head->disk, file->f_mode);
> > +
> > + return 0;
> > +}
> > +
> > +static long nvme_generic_ns_ioctl(struct file *file, unsigned int cmd,
> > + unsigned long arg)
> > +{
> > + struct nvme_generic_ns *generic_ns = file->private_data;
> > +
> > + return nvme_ioctl(generic_ns->head->disk->part0, file->f_mode, cmd, arg);
> > +}
> > +#else
> > +static int nvme_generic_ns_open(struct inode *inode, struct file *file)
> > +{
> > + struct nvme_generic_ns *generic_ns = container_of(inode->i_cdev,
> > + struct nvme_generic_ns, cdev);
> > +
> > + if (!generic_ns->ns)
> > + return -ENODEV;
> > +
> > + file->private_data = generic_ns;
> > + return nvme_open(generic_ns->ns->disk->part0, file->f_mode);
> > +}
> > +
> > +static int nvme_generic_ns_release(struct inode *inode, struct file *file)
> > +{
> > + struct nvme_generic_ns *generic_ns = container_of(inode->i_cdev,
> > + struct nvme_generic_ns, cdev);
> > +
> > + nvme_release(generic_ns->ns->disk, file->f_mode);
> > +
> > + return 0;
> > +}
> > +
> > +static long nvme_generic_ns_ioctl(struct file *file, unsigned int cmd,
> > + unsigned long arg)
> > +{
> > + struct nvme_generic_ns *generic_ns = file->private_data;
> > +
> > + return nvme_ioctl(generic_ns->ns->disk->part0, file->f_mode, cmd, arg);
> > +}
> > +#endif
> > +
> > +static const struct file_operations nvme_generic_ns_fops = {
> > + .owner = THIS_MODULE,
> > + .open = nvme_generic_ns_open,
> > + .release = nvme_generic_ns_release,
> > + .unlocked_ioctl = nvme_generic_ns_ioctl,
> > + .compat_ioctl = compat_ptr_ioctl,
> > +};
> > +
> > +static int nvme_alloc_generic_ns(struct nvme_ctrl *ctrl,
> > + struct nvme_ns_head *head, struct nvme_ns *ns)
> > +{
> > + struct nvme_generic_ns *generic_ns;
> > + char name[DISK_NAME_LEN];
> > + int minor;
> > + int ret;
> > +
> > + generic_ns = kzalloc_node(sizeof(*generic_ns), GFP_KERNEL,
> > + ctrl->numa_node);
> > + if (!generic_ns)
> > + return -ENOMEM;
> > +
> > + ret = ida_simple_get(&nvme_generic_ns_minor_ida, 0, 0, GFP_KERNEL);
> > + if (ret < 0)
> > + goto free_ns;
> > + minor = ret;
> > +
> > + device_initialize(&generic_ns->device);
> > + generic_ns->device.devt = MKDEV(MAJOR(nvme_generic_ns_devt), minor);
> > + generic_ns->device.class = nvme_generic_ns_class;
> > + generic_ns->device.parent = ctrl->device;
> > + dev_set_drvdata(&generic_ns->device, generic_ns);
> > +
> > + nvme_set_generic_ns_name(name, head, ctrl);
> > + ret = dev_set_name(&generic_ns->device, "%s", name);
> > + if (ret)
> > + goto put_ida;
> > +
> > + cdev_init(&generic_ns->cdev, &nvme_generic_ns_fops);
> > + generic_ns->cdev.owner = ctrl->ops->module;
> > +
> > + ret = cdev_device_add(&generic_ns->cdev, &generic_ns->device);
> > + if (ret)
> > + goto free_kobj;
> > +
> > + head->generic_ns = generic_ns;
> > + generic_ns->head = head;
> > + generic_ns->ns = ns;
> > + return 0;
> > +
> > +free_kobj:
> > + kfree_const(generic_ns->device.kobj.name);
> > +put_ida:
> > + ida_simple_remove(&nvme_generic_ns_minor_ida, minor);
> > +free_ns:
> > + kfree(generic_ns);
> > + return ret;
> > +}
> > +
> > static struct nvme_ns_head *nvme_alloc_ns_head(struct nvme_ctrl *ctrl,
> > unsigned nsid, struct nvme_ns_ids *ids)
> > {
> > @@ -3862,6 +3990,13 @@ static int nvme_init_ns_head(struct nvme_ns *ns, unsigned nsid,
> > goto out_unlock;
> > }
> > head->shared = is_shared;
> > +
> > + if (generic_ns && nvme_alloc_generic_ns(ctrl, head, ns)) {
> > + dev_err(ctrl->device,
> > + "Failed to initialize generic namespace %d\n",
> > + nsid);
> > + goto out_put_ns_head;
> > + }
> > } else {
> > ret = -EINVAL;
> > if (!is_shared || !head->shared) {
> > @@ -3963,8 +4098,17 @@ 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 generic namespae device to be
> > + * craeted.
> > + */
> > + if (nvme_update_ns_info(ns, id)) {
> > + if (generic_ns)
> > + 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)) {
> > @@ -3983,6 +4127,7 @@ static void nvme_alloc_ns(struct nvme_ctrl *ctrl, unsigned nsid,
> >
> > nvme_mpath_add_disk(ns, id);
> > nvme_fault_inject_init(&ns->fault_inject, ns->disk->disk_name);
> > +
> > kfree(id);
> >
> > return;
> > @@ -4802,13 +4947,31 @@ static int __init nvme_core_init(void)
> > }
> > nvme_class->dev_uevent = nvme_class_uevent;
> >
> > + if (!generic_ns) return 0;
>
> This will suppress creation of nvme_subsys_class when generic_ns
> parameter is set as 0.
Ah, nice catch, Thanks!
> > + result = alloc_chrdev_region(&nvme_generic_ns_devt, 0,
> > + NVME_MINORS, "nvme-generic-ns");
> > + if (result < 0)
> > + goto destroy_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 unregister_generic_ns;
> > }
> > +
> > + nvme_generic_ns_class = class_create(THIS_MODULE, "nvme-generic-ns");
> > + if (IS_ERR(nvme_generic_ns_class)) {
> > + result = PTR_ERR(nvme_subsys_class);
> > + goto destroy_subsys_class;
> > + }
> > +
> > return 0;
> >
> > +destroy_subsys_class:
> > + class_destroy(nvme_subsys_class);
> > +unregister_generic_ns:
> > + unregister_chrdev_region(nvme_generic_ns_devt, NVME_MINORS);
> > destroy_class:
> > class_destroy(nvme_class);
> > unregister_chrdev:
> > @@ -4827,6 +4990,7 @@ static void __exit nvme_core_exit(void)
> > {
> > class_destroy(nvme_subsys_class);
> > class_destroy(nvme_class);
> > + unregister_chrdev_region(nvme_generic_ns_devt, NVME_MINORS);
> > unregister_chrdev_region(nvme_ctrl_base_chr_devt, NVME_MINORS);
> > destroy_workqueue(nvme_delete_wq);
> > destroy_workqueue(nvme_reset_wq);
>
> The "nvme-generic-ns" class has not been destroyed here.
Will take care of this in the next version. Thanks!
>
> --
> Kanchan
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-03-29 19:28 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-25 12:30 [PATCH 0/1] nvme: introduce generic per-namespace chardev Minwoo Im
2021-03-25 12:30 ` [PATCH 1/1] " Minwoo Im
2021-03-29 8:31 ` Kanchan Joshi
2021-03-29 11:56 ` Minwoo Im
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).