Skip to contentSkip to conversation

nvme: split pci module out of core module

PATCH Superseded Ming Lin 6 participants 24 messages Feb 8, 2016 – Feb 10, 2016 NVMe

v1 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

Ming Lin

author
#0 lore raw

cover nvme: split pci module out of core module

We'll have nvme over fabric dirver soon.
This series split nvme pci module out of nvme core module.

nvme.ko is split into 2 modules:
nvme-core.ko: the core part
nvme.ko: the PCI driver

Ming Lin (4):
nvme: move timeout variables to core.c and export it
nvme: export more symbols
nvme: split dev_list_lock
nvme: split pci module out of core module

 drivers/nvme/host/Kconfig  | 10 +++++++---
 drivers/nvme/host/Makefile | 10 ++++++----
 drivers/nvme/host/core.c   | 40 +++++++++++++++++++++++++++++++++++++---
 drivers/nvme/host/nvme.h   |  2 --
 drivers/nvme/host/pci.c    | 26 ++------------------------
 5 files changed, 52 insertions(+), 36 deletions(-)
Signature · 1 line
1.9.1
author
#1 lore raw

PATCH 1/4 nvme: move timeout variables to core.c and export it

Signed-off-by Ming Lin <ming.l at ssi.samsung.com>
---
 drivers/nvme/host/core.c | 14 ++++++++++++++
 drivers/nvme/host/pci.c  | 12 ------------
 2 files changed, 14 insertions(+), 12 deletions(-)
drivers/nvme/host/core.c +14 -0
index c5bf001..3faed20 100644
@@ -33,6 +33,20 @@
 
 #define NVME_MINORS		(1U << MINORBITS)
 
+unsigned char admin_timeout = 60;
+module_param(admin_timeout, byte, 0644);
+MODULE_PARM_DESC(admin_timeout, "timeout in seconds for admin commands");
+EXPORT_SYMBOL_GPL(admin_timeout);
+
+unsigned char nvme_io_timeout = 30;
+module_param_named(io_timeout, nvme_io_timeout, byte, 0644);
+MODULE_PARM_DESC(io_timeout, "timeout in seconds for I/O");
+EXPORT_SYMBOL_GPL(nvme_io_timeout);
+
+unsigned char shutdown_timeout = 5;
+module_param(shutdown_timeout, byte, 0644);
+MODULE_PARM_DESC(shutdown_timeout, "timeout in seconds for controller shutdown");
+
 static int nvme_major;
 module_param(nvme_major, int, 0);
 
drivers/nvme/host/pci.c +0 -12
index 72ef832..deba7ac 100644
@@ -57,18 +57,6 @@
 #define NVME_NR_AEN_COMMANDS	1
 #define NVME_AQ_BLKMQ_DEPTH	(NVME_AQ_DEPTH - NVME_NR_AEN_COMMANDS)
 
-unsigned char admin_timeout = 60;
-module_param(admin_timeout, byte, 0644);
-MODULE_PARM_DESC(admin_timeout, "timeout in seconds for admin commands");
-
-unsigned char nvme_io_timeout = 30;
-module_param_named(io_timeout, nvme_io_timeout, byte, 0644);
-MODULE_PARM_DESC(io_timeout, "timeout in seconds for I/O");
-
-unsigned char shutdown_timeout = 5;
-module_param(shutdown_timeout, byte, 0644);
-MODULE_PARM_DESC(shutdown_timeout, "timeout in seconds for controller shutdown");
-
 static int use_threaded_interrupts;
 module_param(use_threaded_interrupts, int, 0);
 
Signature · 1 line
1.9.1
#2 lore raw

PATCH 1/4 nvme: move timeout variables to core.c and export it

Looks good,

Reviewed-by Christoph Hellwig <hch at lst.de>
#3 lore raw

PATCH 1/4 nvme: move timeout variables to core.c and export it

In reply to Ming Lin
Quoted 2 lines · Ming Lin
Signed-off-by Ming Lin <ming.l at ssi.samsung.com>

---

I'd really like to have one or two sentences more in the log but anyways

Reviewed-by Johannes Thumshirn <jthumshirn at suse.de>
Quoted 60 lines
 drivers/nvme/host/core.c | 14 ++++++++++++++
 drivers/nvme/host/pci.c  | 12 ------------
 2 files changed, 14 insertions(+), 12 deletions(-)
drivers/nvme/host/core.c +14 -0
index c5bf001..3faed20 100644
@@ -33,6 +33,20 @@
 
 #define NVME_MINORS		(1U << MINORBITS)
 
+unsigned char admin_timeout = 60;
+module_param(admin_timeout, byte, 0644);
+MODULE_PARM_DESC(admin_timeout, "timeout in seconds for admin commands");
+EXPORT_SYMBOL_GPL(admin_timeout);
+
+unsigned char nvme_io_timeout = 30;
+module_param_named(io_timeout, nvme_io_timeout, byte, 0644);
+MODULE_PARM_DESC(io_timeout, "timeout in seconds for I/O");
+EXPORT_SYMBOL_GPL(nvme_io_timeout);
+
+unsigned char shutdown_timeout = 5;
+module_param(shutdown_timeout, byte, 0644);
+MODULE_PARM_DESC(shutdown_timeout, "timeout in seconds for controller shutdown");
+
 static int nvme_major;
 module_param(nvme_major, int, 0);
 
drivers/nvme/host/pci.c +0 -12
index 72ef832..deba7ac 100644
@@ -57,18 +57,6 @@
 #define NVME_NR_AEN_COMMANDS	1
 #define NVME_AQ_BLKMQ_DEPTH	(NVME_AQ_DEPTH - NVME_NR_AEN_COMMANDS)
 
-unsigned char admin_timeout = 60;
-module_param(admin_timeout, byte, 0644);
-MODULE_PARM_DESC(admin_timeout, "timeout in seconds for admin commands");
-
-unsigned char nvme_io_timeout = 30;
-module_param_named(io_timeout, nvme_io_timeout, byte, 0644);
-MODULE_PARM_DESC(io_timeout, "timeout in seconds for I/O");
-
-unsigned char shutdown_timeout = 5;
-module_param(shutdown_timeout, byte, 0644);
-MODULE_PARM_DESC(shutdown_timeout, "timeout in seconds for controller shutdown");
-
 static int use_threaded_interrupts;
 module_param(use_threaded_interrupts, int, 0);
 
Signature · 7 lines
1.9.1


_______________________________________________
Linux-nvme mailing list
Linux-nvme at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
Signature · 6 lines
Johannes Thumshirn                                          Storage
jthumshirn at suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 N?rnberg
GF: Felix Imend?rffer, Jane Smithard, Graham Norton
HRB 21284 (AG N?rnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
#4 lore raw

PATCH 1/4 nvme: move timeout variables to core.c and export it

In reply to Ming Lin
Quoted 13 lines · Ming Lin
Signed-off-by Ming Lin <ming.l at ssi.samsung.com>
---
 drivers/nvme/host/core.c | 14 ++++++++++++++
 drivers/nvme/host/pci.c  | 12 ------------
 2 files changed, 14 insertions(+), 12 deletions(-)
drivers/nvme/host/core.c +0 -0
index c5bf001..3faed20 100644
@@ -33,6 +33,20 @@
 
 #define NVME_MINORS		(1U << MINORBITS)

drivers/nvme/host/core.c line 44

Quoted 10 lines
drivers/nvme/host/core.c +9 -0
 
+unsigned char admin_timeout = 60;
+module_param(admin_timeout, byte, 0644);
+MODULE_PARM_DESC(admin_timeout, "timeout in seconds for admin commands");
+EXPORT_SYMBOL_GPL(admin_timeout);
+
+unsigned char nvme_io_timeout = 30;
+module_param_named(io_timeout, nvme_io_timeout, byte, 0644);
+MODULE_PARM_DESC(io_timeout, "timeout in seconds for I/O");
+EXPORT_SYMBOL_GPL(nvme_io_timeout);

Can these be exported without _GPL? There might be a couple of drivers out of tree which would like to access these variables.

author
#5 lore raw

PATCH 2/4 nvme: export more symbols

In reply to Ming Lin
Signed-off-by Ming Lin <ming.l at ssi.samsung.com>
---
 drivers/nvme/host/core.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)
drivers/nvme/host/core.c +16 -1
index 3faed20..8e4b8ac 100644
@@ -102,6 +102,7 @@ void nvme_requeue_req(struct request *req)
 		blk_mq_kick_requeue_list(req->q);
 	spin_unlock_irqrestore(req->q->queue_lock, flags);
 }
+EXPORT_SYMBOL_GPL(nvme_requeue_req);
 
 struct request *nvme_alloc_request(struct request_queue *q,
 		struct nvme_command *cmd, unsigned int flags)
@@ -125,6 +126,7 @@ struct request *nvme_alloc_request(struct request_queue *q,
 
 	return req;
 }
+EXPORT_SYMBOL_GPL(nvme_alloc_request);
 
 /*
  * Returns 0 on success.  If the result is negative, it's a Linux error code;
@@ -162,6 +164,7 @@ int nvme_submit_sync_cmd(struct request_queue *q, struct nvme_command *cmd,
 {
 	return __nvme_submit_sync_cmd(q, cmd, buffer, bufflen, NULL, 0);
 }
+EXPORT_SYMBOL_GPL(nvme_submit_sync_cmd);
 
 int __nvme_submit_user_cmd(struct request_queue *q, struct nvme_command *cmd,
 		void __user *ubuffer, unsigned bufflen,
@@ -377,6 +380,7 @@ int nvme_set_queue_count(struct nvme_ctrl *ctrl, int *count)
 	*count = min(*count, nr_io_queues);
 	return 0;
 }
+EXPORT_SYMBOL_GPL(nvme_set_queue_count);
 
 static int nvme_submit_io(struct nvme_ns *ns, struct nvme_user_io __user *uio)
 {
@@ -783,6 +787,7 @@ int nvme_disable_ctrl(struct nvme_ctrl *ctrl, u64 cap)
 		return ret;
 	return nvme_wait_ready(ctrl, cap, false);
 }
+EXPORT_SYMBOL_GPL(nvme_disable_ctrl);
 
 int nvme_enable_ctrl(struct nvme_ctrl *ctrl, u64 cap)
 {
@@ -814,6 +819,7 @@ int nvme_enable_ctrl(struct nvme_ctrl *ctrl, u64 cap)
 		return ret;
 	return nvme_wait_ready(ctrl, cap, true);
 }
+EXPORT_SYMBOL_GPL(nvme_enable_ctrl);
 
 int nvme_shutdown_ctrl(struct nvme_ctrl *ctrl)
 {
@@ -844,6 +850,7 @@ int nvme_shutdown_ctrl(struct nvme_ctrl *ctrl)
 
 	return ret;
 }
+EXPORT_SYMBOL_GPL(nvme_shutdown_ctrl);
 
 /*
  * Initialize the cached copies of the Identify data and various controller
@@ -905,6 +912,7 @@ int nvme_init_identify(struct nvme_ctrl *ctrl)
 	kfree(id);
 	return 0;
 }
+EXPORT_SYMBOL_GPL(nvme_init_identify);
 
 static int nvme_dev_open(struct inode *inode, struct file *file)
 {
@@ -1310,6 +1318,7 @@ void nvme_scan_namespaces(struct nvme_ctrl *ctrl)
 	mutex_unlock(&ctrl->namespaces_mutex);
 	kfree(id);
 }
+EXPORT_SYMBOL_GPL(nvme_scan_namespaces);
 
 void nvme_remove_namespaces(struct nvme_ctrl *ctrl)
 {
@@ -1320,6 +1329,7 @@ void nvme_remove_namespaces(struct nvme_ctrl *ctrl)
 		nvme_ns_remove(ns);
 	mutex_unlock(&ctrl->namespaces_mutex);
 }
+EXPORT_SYMBOL_GPL(nvme_remove_namespaces);
 
 static DEFINE_IDA(nvme_instance_ida);
 
@@ -1351,13 +1361,14 @@ static void nvme_release_instance(struct nvme_ctrl *ctrl)
 }
 
 void nvme_uninit_ctrl(struct nvme_ctrl *ctrl)
- {
+{
 	device_destroy(nvme_class, MKDEV(nvme_char_major, ctrl->instance));
 
 	spin_lock(&dev_list_lock);
 	list_del(&ctrl->node);
 	spin_unlock(&dev_list_lock);
 }
+EXPORT_SYMBOL_GPL(nvme_uninit_ctrl);
 
 static void nvme_free_ctrl(struct kref *kref)
 {
@@ -1373,6 +1384,7 @@ void nvme_put_ctrl(struct nvme_ctrl *ctrl)
 {
 	kref_put(&ctrl->kref, nvme_free_ctrl);
 }
+EXPORT_SYMBOL_GPL(nvme_put_ctrl);
 
 /*
  * Initialize a NVMe controller structures.  This needs to be called during
@@ -1416,6 +1428,7 @@ out_release_instance:
 out:
 	return ret;
 }
+EXPORT_SYMBOL_GPL(nvme_init_ctrl);
 
 void nvme_stop_queues(struct nvme_ctrl *ctrl)
 {
@@ -1432,6 +1445,7 @@ void nvme_stop_queues(struct nvme_ctrl *ctrl)
 	}
 	mutex_unlock(&ctrl->namespaces_mutex);
 }
+EXPORT_SYMBOL_GPL(nvme_stop_queues);
 
 void nvme_start_queues(struct nvme_ctrl *ctrl)
 {
@@ -1445,6 +1459,7 @@ void nvme_start_queues(struct nvme_ctrl *ctrl)
 	}
 	mutex_unlock(&ctrl->namespaces_mutex);
 }
+EXPORT_SYMBOL_GPL(nvme_start_queues);
 
 int __init nvme_core_init(void)
 {
Signature · 1 line
1.9.1
#6 lore raw

PATCH 2/4 nvme: export more symbols

These are the first symbols exports from the nvme module :)

Also a little explanation on why might be useful, a single sentence should be enough.

Otherwise looks good:

Reviewed-by Christoph Hellwig <hch at lst.de>
#7 lore raw

PATCH 2/4 nvme: export more symbols

In reply to Ming Lin
Quoted 2 lines · Ming Lin
Signed-off-by Ming Lin <ming.l at ssi.samsung.com>

---

Like Christoph already said, please be a bit more verbose with your commit messages.

Reviewed-by Johannes Thumshirn <jthumshirn at suse.de>
Signature · 6 lines
Johannes Thumshirn                                          Storage
jthumshirn at suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 N?rnberg
GF: Felix Imend?rffer, Jane Smithard, Graham Norton
HRB 21284 (AG N?rnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
#8 lore raw

PATCH 2/4 nvme: export more symbols

In reply to Ming Lin
Quoted 131 lines · Ming Lin
Signed-off-by Ming Lin <ming.l at ssi.samsung.com>
---
 drivers/nvme/host/core.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)
drivers/nvme/host/core.c +15 -1
index 3faed20..8e4b8ac 100644
@@ -102,6 +102,7 @@ void nvme_requeue_req(struct request *req)
 		blk_mq_kick_requeue_list(req->q);
 	spin_unlock_irqrestore(req->q->queue_lock, flags);
 }
+EXPORT_SYMBOL_GPL(nvme_requeue_req);
 
 struct request *nvme_alloc_request(struct request_queue *q,
 		struct nvme_command *cmd, unsigned int flags)
@@ -125,6 +126,7 @@ struct request *nvme_alloc_request(struct request_queue *q,
 
 	return req;
 }
+EXPORT_SYMBOL_GPL(nvme_alloc_request);
 
 /*
  * Returns 0 on success.  If the result is negative, it's a Linux error code;
@@ -162,6 +164,7 @@ int nvme_submit_sync_cmd(struct request_queue *q, struct nvme_command *cmd,
 {
 	return __nvme_submit_sync_cmd(q, cmd, buffer, bufflen, NULL, 0);
 }
+EXPORT_SYMBOL_GPL(nvme_submit_sync_cmd);
 
 int __nvme_submit_user_cmd(struct request_queue *q, struct nvme_command *cmd,
 		void __user *ubuffer, unsigned bufflen,
@@ -377,6 +380,7 @@ int nvme_set_queue_count(struct nvme_ctrl *ctrl, int *count)
 	*count = min(*count, nr_io_queues);
 	return 0;
 }
+EXPORT_SYMBOL_GPL(nvme_set_queue_count);
 
 static int nvme_submit_io(struct nvme_ns *ns, struct nvme_user_io __user *uio)
 {
@@ -783,6 +787,7 @@ int nvme_disable_ctrl(struct nvme_ctrl *ctrl, u64 cap)
 		return ret;
 	return nvme_wait_ready(ctrl, cap, false);
 }
+EXPORT_SYMBOL_GPL(nvme_disable_ctrl);
 
 int nvme_enable_ctrl(struct nvme_ctrl *ctrl, u64 cap)
 {
@@ -814,6 +819,7 @@ int nvme_enable_ctrl(struct nvme_ctrl *ctrl, u64 cap)
 		return ret;
 	return nvme_wait_ready(ctrl, cap, true);
 }
+EXPORT_SYMBOL_GPL(nvme_enable_ctrl);
 
 int nvme_shutdown_ctrl(struct nvme_ctrl *ctrl)
 {
@@ -844,6 +850,7 @@ int nvme_shutdown_ctrl(struct nvme_ctrl *ctrl)
 
 	return ret;
 }
+EXPORT_SYMBOL_GPL(nvme_shutdown_ctrl);
 
 /*
  * Initialize the cached copies of the Identify data and various controller
@@ -905,6 +912,7 @@ int nvme_init_identify(struct nvme_ctrl *ctrl)
 	kfree(id);
 	return 0;
 }
+EXPORT_SYMBOL_GPL(nvme_init_identify);
 
 static int nvme_dev_open(struct inode *inode, struct file *file)
 {
@@ -1310,6 +1318,7 @@ void nvme_scan_namespaces(struct nvme_ctrl *ctrl)
 	mutex_unlock(&ctrl->namespaces_mutex);
 	kfree(id);
 }
+EXPORT_SYMBOL_GPL(nvme_scan_namespaces);
 
 void nvme_remove_namespaces(struct nvme_ctrl *ctrl)
 {
@@ -1320,6 +1329,7 @@ void nvme_remove_namespaces(struct nvme_ctrl *ctrl)
 		nvme_ns_remove(ns);
 	mutex_unlock(&ctrl->namespaces_mutex);
 }
+EXPORT_SYMBOL_GPL(nvme_remove_namespaces);
 
 static DEFINE_IDA(nvme_instance_ida);
 
@@ -1351,13 +1361,14 @@ static void nvme_release_instance(struct nvme_ctrl *ctrl)
 }
 
 void nvme_uninit_ctrl(struct nvme_ctrl *ctrl)
- {
+{
 	device_destroy(nvme_class, MKDEV(nvme_char_major, ctrl->instance));
 
 	spin_lock(&dev_list_lock);
 	list_del(&ctrl->node);
 	spin_unlock(&dev_list_lock);
 }
+EXPORT_SYMBOL_GPL(nvme_uninit_ctrl);
 
 static void nvme_free_ctrl(struct kref *kref)
 {
@@ -1373,6 +1384,7 @@ void nvme_put_ctrl(struct nvme_ctrl *ctrl)
 {
 	kref_put(&ctrl->kref, nvme_free_ctrl);
 }
+EXPORT_SYMBOL_GPL(nvme_put_ctrl);
 
 /*
  * Initialize a NVMe controller structures.  This needs to be called during
@@ -1416,6 +1428,7 @@ out_release_instance:
 out:
 	return ret;
 }
+EXPORT_SYMBOL_GPL(nvme_init_ctrl);
 
 void nvme_stop_queues(struct nvme_ctrl *ctrl)
 {
@@ -1432,6 +1445,7 @@ void nvme_stop_queues(struct nvme_ctrl *ctrl)
 	}
 	mutex_unlock(&ctrl->namespaces_mutex);
 }
+EXPORT_SYMBOL_GPL(nvme_stop_queues);
 
 void nvme_start_queues(struct nvme_ctrl *ctrl)
 {

drivers/nvme/host/core.c void nvme_start_queues(struct nvme_ctrl *ctrl) · line 1465

Quoted 7 lines
drivers/nvme/host/core.c +1 -0
@@ -1445,6 +1459,7 @@ void nvme_start_queues(struct nvme_ctrl *ctrl)
 	}
 	mutex_unlock(&ctrl->namespaces_mutex);
 }
+EXPORT_SYMBOL_GPL(nvme_start_queues);
 
 int __init nvme_core_init(void)
 {

Hi Ming,

Can these be exported without _GPL?

#9 lore raw

PATCH 2/4 nvme: export more symbols

Quoted 1 line

Can these be exported without _GPL?

No way. I consider every Linux module a derivative work, but anything poking into the NVMe internals surely is much as it can. We better keep this clear warning sign.

author
#10 lore raw

PATCH 3/4 nvme: split dev_list_lock

In reply to Ming Lin

Split dev_list_lock into one in the core and one in the PCI driver.

Signed-off-by Ming Lin <ming.l at ssi.samsung.com>
---
 drivers/nvme/host/core.c | 2 +-
 drivers/nvme/host/nvme.h | 2 --
 drivers/nvme/host/pci.c  | 1 +
 3 files changed, 2 insertions(+), 3 deletions(-)
drivers/nvme/host/core.c +1 -1
index 8e4b8ac..3928366 100644
@@ -54,7 +54,7 @@ static int nvme_char_major;
 module_param(nvme_char_major, int, 0);
 
 static LIST_HEAD(nvme_ctrl_list);
-DEFINE_SPINLOCK(dev_list_lock);
+static DEFINE_SPINLOCK(dev_list_lock);
 
 static struct class *nvme_class;
 
drivers/nvme/host/nvme.h +0 -2
index 4fb5bb7..3cea0ed 100644
@@ -265,8 +265,6 @@ int nvme_set_features(struct nvme_ctrl *dev, unsigned fid, unsigned dword11,
 			dma_addr_t dma_addr, u32 *result);
 int nvme_set_queue_count(struct nvme_ctrl *ctrl, int *count);
 
-extern spinlock_t dev_list_lock;
-
 struct sg_io_hdr;
 
 int nvme_sg_io(struct nvme_ns *ns, struct sg_io_hdr __user *u_hdr);
drivers/nvme/host/pci.c +1 -0
index deba7ac..9797595 100644
@@ -65,6 +65,7 @@ module_param(use_cmb_sqes, bool, 0644);
 MODULE_PARM_DESC(use_cmb_sqes, "use controller's memory buffer for I/O SQes");
 
 static LIST_HEAD(dev_list);
+static DEFINE_SPINLOCK(dev_list_lock);
 static struct task_struct *nvme_thread;
 static struct workqueue_struct *nvme_workq;
 static wait_queue_head_t nvme_kthread_wait;
Signature · 1 line
1.9.1
#11 lore raw

PATCH 3/4 nvme: split dev_list_lock

Looks fine,

Reviewed-by Christoph Hellwig <hch at lst.de>
#12 lore raw

PATCH 3/4 nvme: split dev_list_lock

In reply to Ming Lin
Quoted 1 line · Ming Lin

Split dev_list_lock into one in the core and one in the PCI driver.

The dev_list_lock in core.c doesn't really protect the dev_list, like it does in pci.c. Wouldn't it be better to rename it to something more appropriate? I just can't come up with a name, as it seems to be some kind of catch all lock.

Otherwise

Reviewed-by Johannes Thumshirn <jthumshirn at suse.de>
Quoted 54 lines
Signed-off-by Ming Lin <ming.l at ssi.samsung.com>
---
 drivers/nvme/host/core.c | 2 +-
 drivers/nvme/host/nvme.h | 2 --
 drivers/nvme/host/pci.c  | 1 +
 3 files changed, 2 insertions(+), 3 deletions(-)
drivers/nvme/host/core.c +1 -1
index 8e4b8ac..3928366 100644
@@ -54,7 +54,7 @@ static int nvme_char_major;
 module_param(nvme_char_major, int, 0);
 
 static LIST_HEAD(nvme_ctrl_list);
-DEFINE_SPINLOCK(dev_list_lock);
+static DEFINE_SPINLOCK(dev_list_lock);
 
 static struct class *nvme_class;
 
drivers/nvme/host/nvme.h +0 -2
index 4fb5bb7..3cea0ed 100644
@@ -265,8 +265,6 @@ int nvme_set_features(struct nvme_ctrl *dev, unsigned fid, unsigned dword11,
 			dma_addr_t dma_addr, u32 *result);
 int nvme_set_queue_count(struct nvme_ctrl *ctrl, int *count);
 
-extern spinlock_t dev_list_lock;
-
 struct sg_io_hdr;
 
 int nvme_sg_io(struct nvme_ns *ns, struct sg_io_hdr __user *u_hdr);
drivers/nvme/host/pci.c +1 -0
index deba7ac..9797595 100644
@@ -65,6 +65,7 @@ module_param(use_cmb_sqes, bool, 0644);
 MODULE_PARM_DESC(use_cmb_sqes, "use controller's memory buffer for I/O SQes");
 
 static LIST_HEAD(dev_list);
+static DEFINE_SPINLOCK(dev_list_lock);
 static struct task_struct *nvme_thread;
 static struct workqueue_struct *nvme_workq;
 static wait_queue_head_t nvme_kthread_wait;
Signature · 7 lines
1.9.1


_______________________________________________
Linux-nvme mailing list
Linux-nvme at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
Signature · 6 lines
Johannes Thumshirn                                          Storage
jthumshirn at suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 N?rnberg
GF: Felix Imend?rffer, Jane Smithard, Graham Norton
HRB 21284 (AG N?rnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
#13 lore raw

PATCH 3/4 nvme: split dev_list_lock

Quoted 6 lines · Johannes Thumshirn
Quoted 1 line · Ming Lin

Split dev_list_lock into one in the core and one in the PCI driver.

The dev_list_lock in core.c doesn't really protect the dev_list, like it does in pci.c. Wouldn't it be better to rename it to something more appropriate? I just can't come up with a name, as it seems to be some kind of catch all lock.

I plans to remove the core.c dev_list_lock by rewriting the character device interface code. Let's keep the name for now, it'll go away.

(and the pci.c might be gone even sooner, but at least it has the right name :))

#14 lore raw

PATCH 3/4 nvme: split dev_list_lock

Quoted 8 lines · Christoph Hellwig
Quoted 6 lines · Johannes Thumshirn
Quoted 1 line · Ming Lin

Split dev_list_lock into one in the core and one in the PCI driver.

The dev_list_lock in core.c doesn't really protect the dev_list, like it does in pci.c. Wouldn't it be better to rename it to something more appropriate? I just can't come up with a name, as it seems to be some kind of catch all lock.

Quoted 5 lines

I plans to remove the core.c dev_list_lock by rewriting the character device interface code. Let's keep the name for now, it'll go away.

(and the pci.c might be gone even sooner, but at least it has the right name :))

OK, then I'm fine with it.

Signature · 6 lines
Johannes Thumshirn                                          Storage
jthumshirn at suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 N?rnberg
GF: Felix Imend?rffer, Jane Smithard, Graham Norton
HRB 21284 (AG N?rnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
author
#15 lore raw

PATCH 4/4 nvme: split pci module out of core module

In reply to Ming Lin

This splits nvme.ko into 2 modules:
nvme-core.ko: the core part
nvme.ko: the PCI driver

Also changes config name:
s/CONFIG_BLK_DEV_NVME/CONFIG_NVME_PCI
s/CONFIG_BLK_DEV_NVME_SCSI/CONFIG_NVME_SCSI

Signed-off-by Ming Lin <ming.l at ssi.samsung.com>
---
 drivers/nvme/host/Kconfig  | 10 +++++++---
 drivers/nvme/host/Makefile | 10 ++++++----
 drivers/nvme/host/core.c   |  7 ++++++-
 drivers/nvme/host/pci.c    | 13 +------------
 4 files changed, 20 insertions(+), 20 deletions(-)
drivers/nvme/host/Kconfig +7 -3
index 5d62373..883dcd6 100644
@@ -1,6 +1,10 @@
-config BLK_DEV_NVME
+config NVME_CORE
+	tristate
+
+config NVME_PCI
 	tristate "NVM Express block device"
 	depends on PCI && BLOCK
+	select NVME_CORE
 	---help---
 	  The NVM Express driver is for solid state drives directly
 	  connected to the PCI or PCI Express bus.  If you know you
@@ -9,9 +13,9 @@ config BLK_DEV_NVME
 	  To compile this driver as a module, choose M here: the
 	  module will be called nvme.
 
-config BLK_DEV_NVME_SCSI
+config NVME_SCSI
 	bool "SCSI emulation for NVMe device nodes"
-	depends on BLK_DEV_NVME
+	depends on NVME_CORE
 	---help---
 	  This adds support for the SG_IO ioctl on the NVMe character
 	  and block devices nodes, as well a a translation for a small
drivers/nvme/host/Makefile +6 -4
index 51bf908..98b057e 100644
@@ -1,6 +1,8 @@
+obj-$(CONFIG_NVME_CORE)		+= nvme-core.o
+obj-$(CONFIG_NVME_PCI)		+= nvme.o
 
-obj-$(CONFIG_BLK_DEV_NVME)     += nvme.o
+nvme-core-y				:= core.o
+nvme-core-$(CONFIG_NVME_SCSI)		+= scsi.o
+nvme-core-$(CONFIG_NVM)			+= lightnvm.o
 
-lightnvm-$(CONFIG_NVM)			:= lightnvm.o
-nvme-y					+= core.o pci.o $(lightnvm-y)
-nvme-$(CONFIG_BLK_DEV_NVME_SCSI)        += scsi.o
+nvme-y					+= pci.o
drivers/nvme/host/core.c +6 -1
index 3928366..eaf9d34 100644
@@ -485,7 +485,7 @@ static int nvme_ioctl(struct block_device *bdev, fmode_t mode,
 		return nvme_user_cmd(ns->ctrl, ns, (void __user *)arg);
 	case NVME_IOCTL_SUBMIT_IO:
 		return nvme_submit_io(ns, (void __user *)arg);
-#ifdef CONFIG_BLK_DEV_NVME_SCSI
+#ifdef CONFIG_NVME_SCSI
 	case SG_GET_VERSION_NUM:
 		return nvme_sg_get_version_num((void __user *)arg);
 	case SG_IO:
@@ -1499,3 +1499,8 @@ void nvme_core_exit(void)
 	class_destroy(nvme_class);
 	__unregister_chrdev(nvme_char_major, 0, NVME_MINORS, "nvme");
 }
+
+MODULE_LICENSE("GPL");
+MODULE_VERSION("1.0");
+module_init(nvme_core_init);
+module_exit(nvme_core_exit);
drivers/nvme/host/pci.c +1 -12
index 9797595..4cc32fa 100644
@@ -2220,26 +2220,15 @@ static int __init nvme_init(void)
 	if (!nvme_workq)
 		return -ENOMEM;
 
-	result = nvme_core_init();
-	if (result < 0)
-		goto kill_workq;
-
 	result = pci_register_driver(&nvme_driver);
 	if (result)
-		goto core_exit;
-	return 0;
-
- core_exit:
-	nvme_core_exit();
- kill_workq:
-	destroy_workqueue(nvme_workq);
+		destroy_workqueue(nvme_workq);
 	return result;
 }
 
 static void __exit nvme_exit(void)
 {
 	pci_unregister_driver(&nvme_driver);
-	nvme_core_exit();
 	destroy_workqueue(nvme_workq);
 	BUG_ON(nvme_thread && !IS_ERR(nvme_thread));
 	_nvme_check_size();
Signature · 1 line
1.9.1
#16 lore raw

PATCH 4/4 nvme: split pci module out of core module

Quoted 7 lines · Ming Lin

This splits nvme.ko into 2 modules:
nvme-core.ko: the core part
nvme.ko: the PCI driver

Also changes config name:
s/CONFIG_BLK_DEV_NVME/CONFIG_NVME_PCI
s/CONFIG_BLK_DEV_NVME_SCSI/CONFIG_NVME_SCSI

Is there any auto-migration for those who already have CONFIG_BLK_DEV_NVME set in their kernel configs? If not, this could lead to a bunch of non-booting systems.

Signature · 6 lines
Johannes Thumshirn                                          Storage
jthumshirn at suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 N?rnberg
GF: Felix Imend?rffer, Jane Smithard, Graham Norton
HRB 21284 (AG N?rnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
author
#17 lore raw

PATCH 4/4 nvme: split pci module out of core module

Quoted 5 lines · Johannes Thumshirn
Quoted 4 lines · Ming Lin

This splits nvme.ko into 2 modules:
nvme-core.ko: the core part
nvme.ko: the PCI driver

Quoted 7 lines
Quoted 3 lines

Also changes config name:
s/CONFIG_BLK_DEV_NVME/CONFIG_NVME_PCI
s/CONFIG_BLK_DEV_NVME_SCSI/CONFIG_NVME_SCSI

Is there any auto-migration for those who already have CONFIG_BLK_DEV_NVME set in their kernel configs? If not, this could lead to a bunch of non-booting systems.

Hi Michal(CCed),

As you saw above, I'm changing:
CONFIG_BLK_DEV_NVME to CONFIG_NVME_PCI
and
CONFIG_BLK_DEV_NVME_SCSI to CONFIG_NVME_SCSI

Is there Kconfig magic to keep existing config working? ie, if CONFIG_BLK_DEV_NVME was set in .config, then CONFIG_NVME_PCI should be set automatically?

Thanks,
Ming

#18 lore raw

PATCH 4/4 nvme: split pci module out of core module

Quoted 14 lines · Ming Lin
Quoted 5 lines · Johannes Thumshirn
Quoted 4 lines · Ming Lin

This splits nvme.ko into 2 modules:
nvme-core.ko: the core part
nvme.ko: the PCI driver

Quoted 7 lines
Quoted 3 lines

Also changes config name:
s/CONFIG_BLK_DEV_NVME/CONFIG_NVME_PCI
s/CONFIG_BLK_DEV_NVME_SCSI/CONFIG_NVME_SCSI

Is there any auto-migration for those who already have CONFIG_BLK_DEV_NVME set in their kernel configs? If not, this could lead to a bunch of non-booting systems.

Quoted 8 lines

Hi Michal(CCed),

As you saw above, I'm changing:
CONFIG_BLK_DEV_NVME to CONFIG_NVME_PCI
and
CONFIG_BLK_DEV_NVME_SCSI to CONFIG_NVME_SCSI

Is there Kconfig magic to keep existing config working?

Not if you remove the old option from the Kconfig file. You could keep the old option and make it select the new one. But then the benefit of the whole operation becomes questionable.

Michal

#19 lore raw

PATCH 4/4 nvme: split pci module out of core module

In reply to Ming Lin

Looks fine,

but as Johannes poined out we'll probably need some Kconfig magic to keep existing configfs working.

author
#20 lore raw

PATCH 4/4 nvme: split pci module out of core module

Quoted 4 lines · Christoph Hellwig

Looks fine,

but as Johannes poined out we'll probably need some Kconfig magic to keep existing configfs working.

I didn't find the magic.
So just keep the old CONFIG name in v2 patches.

#21 lore raw

PATCH 4/4 nvme: split pci module out of core module

In reply to Ming Lin
Quoted 3 lines · Ming Lin

This splits nvme.ko into 2 modules:
nvme-core.ko: the core part
nvme.ko: the PCI driver

Do we want to let PCI take the generic "nvme.ko" name? What will the other nvme transport modules be called?

Otherwise, looks good to me.

author
#22 lore raw

PATCH 4/4 nvme: split pci module out of core module

Quoted 7 lines · Keith Busch
Quoted 3 lines · Ming Lin

This splits nvme.ko into 2 modules:
nvme-core.ko: the core part
nvme.ko: the PCI driver

Do we want to let PCI take the generic "nvme.ko" name? What will the other nvme transport modules be called?

Better name would be: nvme_pci.ko, but that may break, for example, userspace scripts.

nvme_rdma.ko: IB/RoCE/iWARP
nvme_fc.ko: Fibre Channel
nvme_vhost.ko
....

Quoted 2 lines

Otherwise, looks good to me.

#23 lore raw

cover nvme: split pci module out of core module

In reply to Ming Lin
Quoted 6 lines · Ming Lin

We'll have nvme over fabric dirver soon.
This series split nvme pci module out of nvme core module.

nvme.ko is split into 2 modules:
nvme-core.ko: the core part
nvme.ko: the PCI driver

Might be worth explaining why we are doing this: for the NVMe over Fabrics drivers that are going to reuse the core, as well as things like the vhost NVMe driver if we're going to merge it.

We'll also need something to take proper references to the transport driver on open in this series. E.g. this patch from Sagi in the Fabrics tree:

---

Quoted 1 line

From 1fafa7768fe93bd6a7ec335738bbae9e00f4a634 Mon Sep 17 00:00:00 2001

From: Sagi Grimberg <[email protected]>
Date: Tue, 2 Feb 2016 21:52:51 +0100
Subject: nvme/host: reference the fabric module for each bdev open callout

We don't want to be able to unload the fabric driver when we have openened referenced to our namespaces. Thus, for each nvme_open we take a reference on the fabric driver and put it in nvme_release. This behavior is consistent with the scsi model.

This resolves the panic when unloading a fabric module with mpath holders.

Signed-off-by Sagi Grimberg <sagig at mellanox.com>
Reviewed-by Christoph Hellwig <hch at lst.de>
Reviewed-by Ian Bakshan <ianb at mellanox.com>
---
 drivers/nvme/host/core.c | 19 ++++++++++++++++---
 drivers/nvme/host/nvme.h |  1 +
 drivers/nvme/host/pci.c  |  1 +
 3 files changed, 18 insertions(+), 3 deletions(-)
drivers/nvme/host/core.c +16 -3
index a152b8c..9fe7471 100644
@@ -71,11 +71,21 @@ static struct nvme_ns *nvme_get_ns_from_disk(struct gendisk *disk)
 
 	spin_lock(&dev_list_lock);
 	ns = disk->private_data;
-	if (ns && !kref_get_unless_zero(&ns->kref))
-		ns = NULL;
+	if (ns) {
+		if (!kref_get_unless_zero(&ns->kref))
+			goto fail;
+		if (!try_module_get(ns->ctrl->ops->module))
+			goto fail_put_ns;
+	}
 	spin_unlock(&dev_list_lock);
 
 	return ns;
+
+fail_put_ns:
+	kref_put(&ns->kref, nvme_free_ns);
+fail:
+	spin_unlock(&dev_list_lock);
+	return NULL;
 }
 
 void nvme_requeue_req(struct request *req)
@@ -517,7 +527,10 @@ static int nvme_open(struct block_device *bdev, fmode_t mode)
 
 static void nvme_release(struct gendisk *disk, fmode_t mode)
 {
-	nvme_put_ns(disk->private_data);
+	struct nvme_ns *ns = disk->private_data;
+
+	module_put(ns->ctrl->ops->module);
+	nvme_put_ns(ns);
 }
 
 static int nvme_getgeo(struct block_device *bdev, struct hd_geometry *geo)
drivers/nvme/host/nvme.h +1 -0
index 62209aa..cce40c9 100644
@@ -119,6 +119,7 @@ struct nvme_ns {
 };
 
 struct nvme_ctrl_ops {
+	struct module *module;
 	int (*reg_read32)(struct nvme_ctrl *ctrl, u32 off, u32 *val);
 	int (*reg_write32)(struct nvme_ctrl *ctrl, u32 off, u32 val);
 	int (*reg_read64)(struct nvme_ctrl *ctrl, u32 off, u64 *val);
drivers/nvme/host/pci.c +1 -0
index ad24442d..526a30b 100644
@@ -2016,6 +2016,7 @@ static int nvme_pci_reset_ctrl(struct nvme_ctrl *ctrl)
 }
 
 static const struct nvme_ctrl_ops nvme_pci_ctrl_ops = {
+	.module			= THIS_MODULE,
 	.reg_read32		= nvme_pci_reg_read32,
 	.reg_write32		= nvme_pci_reg_write32,
 	.reg_read64		= nvme_pci_reg_read64,
Signature · 1 line
2.1.4