[PATCH] NVMe: Add a character device for each nvme device

[PATCH] NVMe: Add a character device for each nvme device

[Keith Busch]

Registers a character device for the nvme module and creates character
files as /dev/nvmeN for each nvme device probed, where N is the device
instance. The character devices support nvme admin ioctl commands so
that nvme devices without namespaces can be managed.

Signed-off-by: Keith Busch <keith.busch at intel.com>
---
 drivers/block/nvme.c |   55 +++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 54 insertions(+), 1 deletions(-)

diff --git a/drivers/block/nvme.c b/drivers/block/nvme.c
index 7bcd882..8a16ac8 100644
--- a/drivers/block/nvme.c
+++ b/drivers/block/nvme.c
@@ -20,6 +20,7 @@
 #include <linux/bio.h>
 #include <linux/bitops.h>
 #include <linux/blkdev.h>
+#include <linux/cdev.h>
 #include <linux/delay.h>
 #include <linux/errno.h>
 #include <linux/fs.h>
@@ -45,6 +46,7 @@
 #define SQ_SIZE(depth)		(depth * sizeof(struct nvme_command))
 #define CQ_SIZE(depth)		(depth * sizeof(struct nvme_completion))
 #define NVME_MINORS 64
+#define NVME_MAX_DEVS 1024
 #define NVME_IO_TIMEOUT	(5 * HZ)
 #define ADMIN_TIMEOUT	(60 * HZ)
 
@@ -54,9 +56,13 @@ module_param(nvme_major, int, 0);
 static int use_threaded_interrupts;
 module_param(use_threaded_interrupts, int, 0);
 
+static int nvme_char_major;
+module_param(nvme_char_major, int, 0);
+
 static DEFINE_SPINLOCK(dev_list_lock);
 static LIST_HEAD(dev_list);
 static struct task_struct *nvme_thread;
+static struct class *nvme_char_cl;
 
 /*
  * Represents an NVM Express device.  Each nvme_dev is a PCI function.
@@ -1222,6 +1228,35 @@ static const struct block_device_operations nvme_fops = {
 	.compat_ioctl	= nvme_ioctl,
 };
 
+static long nvme_char_ioctl(struct file *f, unsigned int cmd, unsigned long arg)
+{
+	struct nvme_dev *dev;
+	int instance = iminor(f->f_dentry->d_inode);
+
+	spin_lock(&dev_list_lock);
+	list_for_each_entry(dev, &dev_list, node) {
+		if (dev->instance == instance)
+			break;
+	}
+	spin_unlock(&dev_list_lock);
+
+	if (&dev->node == &dev_list)
+		return -ENOTTY;
+	
+	switch (cmd) {
+	case NVME_IOCTL_ADMIN_CMD:
+		return nvme_user_admin_cmd(dev, (void __user *)arg);
+	default:
+		return -ENOTTY;
+	}
+}
+
+static const struct file_operations nvme_char_fops = {
+	.owner		= THIS_MODULE,
+	.unlocked_ioctl	= nvme_char_ioctl,
+	.compat_ioctl	= nvme_char_ioctl,
+};
+
 static void nvme_timeout_ios(struct nvme_queue *nvmeq)
 {
 	int depth = nvmeq->q_depth - 1;
@@ -1632,6 +1667,8 @@ static int __devinit nvme_probe(struct pci_dev *pdev,
 	if (result)
 		goto delete;
 
+	device_create(nvme_char_cl, NULL, MKDEV(nvme_char_major, dev->instance),
+						NULL, "nvme%d", dev->instance);
 	return 0;
 
  delete:
@@ -1660,6 +1697,7 @@ static void __devexit nvme_remove(struct pci_dev *pdev)
 {
 	struct nvme_dev *dev = pci_get_drvdata(pdev);
 	nvme_dev_remove(dev);
+	device_destroy(nvme_char_cl, MKDEV(nvme_char_major, dev->instance));
 	pci_disable_msix(pdev);
 	iounmap(dev->bar);
 	nvme_release_instance(dev);
@@ -1721,11 +1759,24 @@ static int __init nvme_init(void)
 	else if (result > 0)
 	    nvme_major = result;
 
+	result = __register_chrdev(nvme_char_major, 0, NVME_MAX_DEVS, "nvme",
+							&nvme_char_fops);
+	if (result < 0)
+		goto unregister_blkdev;
+	else if (result > 0)
+		nvme_char_major = result;
+	nvme_char_cl = class_create(THIS_MODULE, "nvme");
+	if (!nvme_char_cl)
+		goto unregister_chrdev;
 	result = pci_register_driver(&nvme_driver);
 	if (result)
-		goto unregister_blkdev;
+		goto destroy_class;
 	return 0;
 
+ destroy_class:
+	class_destroy(nvme_char_cl);
+ unregister_chrdev:
+	__unregister_chrdev(nvme_char_major, 0, NVME_MAX_DEVS, "nvme");
  unregister_blkdev:
 	unregister_blkdev(nvme_major, "nvme");
  kill_kthread:
@@ -1736,6 +1787,8 @@ static int __init nvme_init(void)
 static void __exit nvme_exit(void)
 {
 	pci_unregister_driver(&nvme_driver);
+	class_destroy(nvme_char_cl);
+	__unregister_chrdev(nvme_char_major, 0, NVME_MAX_DEVS, "nvme");
 	unregister_blkdev(nvme_major, "nvme");
 	kthread_stop(nvme_thread);
 }
-- 
1.7.0.4

[PATCH] NVMe: Add a character device for each nvme device

[Matthew Wilcox]

On Fri, Jul 27, 2012@10:44:18AM -0600, Keith Busch wrote:
> Registers a character device for the nvme module and creates character
> files as /dev/nvmeN for each nvme device probed, where N is the device
> instance. The character devices support nvme admin ioctl commands so
> that nvme devices without namespaces can be managed.

I don't see a problem here, but I'm no expert at sysfs / character devices.
Alan, Greg, anyone else see any problems with how this character device is
created / destroyed?

> 
> Signed-off-by: Keith Busch <keith.busch at intel.com>
> ---
>  drivers/block/nvme.c |   55 +++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 files changed, 54 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/block/nvme.c b/drivers/block/nvme.c
> index 7bcd882..8a16ac8 100644
> --- a/drivers/block/nvme.c
> +++ b/drivers/block/nvme.c
> @@ -20,6 +20,7 @@
>  #include <linux/bio.h>
>  #include <linux/bitops.h>
>  #include <linux/blkdev.h>
> +#include <linux/cdev.h>
>  #include <linux/delay.h>
>  #include <linux/errno.h>
>  #include <linux/fs.h>
> @@ -45,6 +46,7 @@
>  #define SQ_SIZE(depth)		(depth * sizeof(struct nvme_command))
>  #define CQ_SIZE(depth)		(depth * sizeof(struct nvme_completion))
>  #define NVME_MINORS 64
> +#define NVME_MAX_DEVS 1024
>  #define NVME_IO_TIMEOUT	(5 * HZ)
>  #define ADMIN_TIMEOUT	(60 * HZ)
>  
> @@ -54,9 +56,13 @@ module_param(nvme_major, int, 0);
>  static int use_threaded_interrupts;
>  module_param(use_threaded_interrupts, int, 0);
>  
> +static int nvme_char_major;
> +module_param(nvme_char_major, int, 0);
> +
>  static DEFINE_SPINLOCK(dev_list_lock);
>  static LIST_HEAD(dev_list);
>  static struct task_struct *nvme_thread;
> +static struct class *nvme_char_cl;
>  
>  /*
>   * Represents an NVM Express device.  Each nvme_dev is a PCI function.
> @@ -1222,6 +1228,35 @@ static const struct block_device_operations nvme_fops = {
>  	.compat_ioctl	= nvme_ioctl,
>  };
>  
> +static long nvme_char_ioctl(struct file *f, unsigned int cmd, unsigned long arg)
> +{
> +	struct nvme_dev *dev;
> +	int instance = iminor(f->f_dentry->d_inode);
> +
> +	spin_lock(&dev_list_lock);
> +	list_for_each_entry(dev, &dev_list, node) {
> +		if (dev->instance == instance)
> +			break;
> +	}
> +	spin_unlock(&dev_list_lock);
> +
> +	if (&dev->node == &dev_list)
> +		return -ENOTTY;
> +	
> +	switch (cmd) {
> +	case NVME_IOCTL_ADMIN_CMD:
> +		return nvme_user_admin_cmd(dev, (void __user *)arg);
> +	default:
> +		return -ENOTTY;
> +	}
> +}
> +
> +static const struct file_operations nvme_char_fops = {
> +	.owner		= THIS_MODULE,
> +	.unlocked_ioctl	= nvme_char_ioctl,
> +	.compat_ioctl	= nvme_char_ioctl,
> +};
> +
>  static void nvme_timeout_ios(struct nvme_queue *nvmeq)
>  {
>  	int depth = nvmeq->q_depth - 1;
> @@ -1632,6 +1667,8 @@ static int __devinit nvme_probe(struct pci_dev *pdev,
>  	if (result)
>  		goto delete;
>  
> +	device_create(nvme_char_cl, NULL, MKDEV(nvme_char_major, dev->instance),
> +						NULL, "nvme%d", dev->instance);
>  	return 0;
>  
>   delete:
> @@ -1660,6 +1697,7 @@ static void __devexit nvme_remove(struct pci_dev *pdev)
>  {
>  	struct nvme_dev *dev = pci_get_drvdata(pdev);
>  	nvme_dev_remove(dev);
> +	device_destroy(nvme_char_cl, MKDEV(nvme_char_major, dev->instance));
>  	pci_disable_msix(pdev);
>  	iounmap(dev->bar);
>  	nvme_release_instance(dev);
> @@ -1721,11 +1759,24 @@ static int __init nvme_init(void)
>  	else if (result > 0)
>  	    nvme_major = result;
>  
> +	result = __register_chrdev(nvme_char_major, 0, NVME_MAX_DEVS, "nvme",
> +							&nvme_char_fops);
> +	if (result < 0)
> +		goto unregister_blkdev;
> +	else if (result > 0)
> +		nvme_char_major = result;
> +	nvme_char_cl = class_create(THIS_MODULE, "nvme");
> +	if (!nvme_char_cl)
> +		goto unregister_chrdev;
>  	result = pci_register_driver(&nvme_driver);
>  	if (result)
> -		goto unregister_blkdev;
> +		goto destroy_class;
>  	return 0;
>  
> + destroy_class:
> +	class_destroy(nvme_char_cl);
> + unregister_chrdev:
> +	__unregister_chrdev(nvme_char_major, 0, NVME_MAX_DEVS, "nvme");
>   unregister_blkdev:
>  	unregister_blkdev(nvme_major, "nvme");
>   kill_kthread:
> @@ -1736,6 +1787,8 @@ static int __init nvme_init(void)
>  static void __exit nvme_exit(void)
>  {
>  	pci_unregister_driver(&nvme_driver);
> +	class_destroy(nvme_char_cl);
> +	__unregister_chrdev(nvme_char_major, 0, NVME_MAX_DEVS, "nvme");
>  	unregister_blkdev(nvme_major, "nvme");
>  	kthread_stop(nvme_thread);
>  }
> -- 
> 1.7.0.4
> 
> 
> _______________________________________________
> Linux-nvme mailing list
> Linux-nvme at lists.infradead.org
> http://merlin.infradead.org/mailman/listinfo/linux-nvme

[PATCH] NVMe: Add a character device for each nvme device

[Greg KH]

On Fri, Jul 27, 2012@02:12:12PM -0400, Matthew Wilcox wrote:
> On Fri, Jul 27, 2012@10:44:18AM -0600, Keith Busch wrote:
> > Registers a character device for the nvme module and creates character
> > files as /dev/nvmeN for each nvme device probed, where N is the device
> > instance. The character devices support nvme admin ioctl commands so
> > that nvme devices without namespaces can be managed.
> 
> I don't see a problem here, but I'm no expert at sysfs / character devices.
> Alan, Greg, anyone else see any problems with how this character device is
> created / destroyed?

Yes, see below:

> > +	device_create(nvme_char_cl, NULL, MKDEV(nvme_char_major, dev->instance),
> > +						NULL, "nvme%d", dev->instance);

You just created a device at the "root" of sysfs, which is wrong,
especially when you do have a parent device here.  Please use it.

Also, why are you creating your own class?  Can't this just be a misc
device?  And if you want to create your own class, please don't, use a
bus, as that is what is really happening here, right?  We are trying to
move away from using 'struct class' wherever possible (one of these days
we'll just remove it...)

thanks,

greg k-h

[PATCH] NVMe: Add a character device for each nvme device

[Matthew Wilcox]

On Fri, Jul 27, 2012@11:25:46AM -0700, Greg KH wrote:
> On Fri, Jul 27, 2012@02:12:12PM -0400, Matthew Wilcox wrote:
> > I don't see a problem here, but I'm no expert at sysfs / character devices.
> > Alan, Greg, anyone else see any problems with how this character device is
> > created / destroyed?
> 
> Yes, see below:

Thanks!

> > > +	device_create(nvme_char_cl, NULL, MKDEV(nvme_char_major, dev->instance),
> > > +						NULL, "nvme%d", dev->instance);
> 
> You just created a device at the "root" of sysfs, which is wrong,
> especially when you do have a parent device here.  Please use it.

OK, that makes sense; this device should be the child of the pci_dev that
it belongs to.

> Also, why are you creating your own class?  Can't this just be a misc
> device?  And if you want to create your own class, please don't, use a
> bus, as that is what is really happening here, right?  We are trying to
> move away from using 'struct class' wherever possible (one of these days
> we'll just remove it...)

What we're trying to achieve here is to create one character device
per NVMe controller that gets plugged in.  Each NVMe controller is-a
PCI function.  The reason we're trying to do this is so that we can send
commands to the NVMe controller, even when there is no storage present
(eg a drive is shipped from the factory with no configured storage).

So we have no particular desire to create a new struct class, or struct
bus.  If we can create a misc device per PCIe function that's bound to our
driver, that's great!  Can you recommend a driver that does this already?

[PATCH] NVMe: Add a character device for each nvme device

[Greg KH]

On Fri, Jul 27, 2012@03:08:21PM -0400, Matthew Wilcox wrote:
> On Fri, Jul 27, 2012@11:25:46AM -0700, Greg KH wrote:
> > On Fri, Jul 27, 2012@02:12:12PM -0400, Matthew Wilcox wrote:
> > > I don't see a problem here, but I'm no expert at sysfs / character devices.
> > > Alan, Greg, anyone else see any problems with how this character device is
> > > created / destroyed?
> > 
> > Yes, see below:
> 
> Thanks!
> 
> > > > +	device_create(nvme_char_cl, NULL, MKDEV(nvme_char_major, dev->instance),
> > > > +						NULL, "nvme%d", dev->instance);
> > 
> > You just created a device at the "root" of sysfs, which is wrong,
> > especially when you do have a parent device here.  Please use it.
> 
> OK, that makes sense; this device should be the child of the pci_dev that
> it belongs to.
> 
> > Also, why are you creating your own class?  Can't this just be a misc
> > device?  And if you want to create your own class, please don't, use a
> > bus, as that is what is really happening here, right?  We are trying to
> > move away from using 'struct class' wherever possible (one of these days
> > we'll just remove it...)
> 
> What we're trying to achieve here is to create one character device
> per NVMe controller that gets plugged in.  Each NVMe controller is-a
> PCI function.  The reason we're trying to do this is so that we can send
> commands to the NVMe controller, even when there is no storage present
> (eg a drive is shipped from the factory with no configured storage).
> 
> So we have no particular desire to create a new struct class, or struct
> bus.  If we can create a misc device per PCIe function that's bound to our
> driver, that's great!  Can you recommend a driver that does this already?

I don't think there is one, but it shouldn't be that hard to just create
a 'struct misdevice' for each one of the devices you want to create,
would it?

But, as you really are a "specific type", a bus_type might be overkill,
so the original use of device_create() should be fine.  Just be sure to
fix the parent pointer issue, and you should be fine, right?

greg k-h

[PATCH] NVMe: Add a character device for each nvme device

[Jeff Garzik]

On 07/27/2012 02:12 PM, Matthew Wilcox wrote:
> On Fri, Jul 27, 2012@10:44:18AM -0600, Keith Busch wrote:
>> Registers a character device for the nvme module and creates character
>> files as /dev/nvmeN for each nvme device probed, where N is the device
>> instance. The character devices support nvme admin ioctl commands so
>> that nvme devices without namespaces can be managed.
>
> I don't see a problem here, but I'm no expert at sysfs / character devices.
> Alan, Greg, anyone else see any problems with how this character device is
> created / destroyed?

This seems like something normally done via a control device that is 
addressible via bsg.

This is -not- a NAK, but maybe the storage folks have a different 
preference for an admin-command path.

	Jeff

[PATCH] NVMe: Add a character device for each nvme device

[Matthew Wilcox]

On Fri, Jul 27, 2012@03:28:25PM -0400, Jeff Garzik wrote:
> On 07/27/2012 02:12 PM, Matthew Wilcox wrote:
> >On Fri, Jul 27, 2012@10:44:18AM -0600, Keith Busch wrote:
> >>Registers a character device for the nvme module and creates character
> >>files as /dev/nvmeN for each nvme device probed, where N is the device
> >>instance. The character devices support nvme admin ioctl commands so
> >>that nvme devices without namespaces can be managed.
> >
> >I don't see a problem here, but I'm no expert at sysfs / character devices.
> >Alan, Greg, anyone else see any problems with how this character device is
> >created / destroyed?
> 
> This seems like something normally done via a control device that is
> addressible via bsg.

I'm not convinced about that.  bsg requires a request_queue, and we
don't have one in the absence of any storage.  There doesn't even seem
to be a standard way of sending commands to SCSI hosts, let alone block
device controllers.

Maybe we should design such a mechanism, but maybe we shouldn't ... as we
find common things to do, we tend to move those to sysfs, not ioctls,
and the kinds of commands that are being sent here are essentially
vendor-specific NVMe commands; it's not clear they'd fit neatly into a
generic mechanism.

> This is -not- a NAK, but maybe the storage folks have a different
> preference for an admin-command path.
> 
> 	Jeff
> 

[PATCH] NVMe: Add a character device for each nvme device

[Matthew Wilcox]

On Fri, Jul 27, 2012@12:21:00PM -0700, Greg KH wrote:
> > > Also, why are you creating your own class?  Can't this just be a misc
> > > device?  And if you want to create your own class, please don't, use a
> > > bus, as that is what is really happening here, right?  We are trying to
> > > move away from using 'struct class' wherever possible (one of these days
> > > we'll just remove it...)
> > 
> > What we're trying to achieve here is to create one character device
> > per NVMe controller that gets plugged in.  Each NVMe controller is-a
> > PCI function.  The reason we're trying to do this is so that we can send
> > commands to the NVMe controller, even when there is no storage present
> > (eg a drive is shipped from the factory with no configured storage).
> > 
> > So we have no particular desire to create a new struct class, or struct
> > bus.  If we can create a misc device per PCIe function that's bound to our
> > driver, that's great!  Can you recommend a driver that does this already?
> 
> I don't think there is one, but it shouldn't be that hard to just create
> a 'struct misdevice' for each one of the devices you want to create,
> would it?
> 
> But, as you really are a "specific type", a bus_type might be overkill,
> so the original use of device_create() should be fine.  Just be sure to
> fix the parent pointer issue, and you should be fine, right?

Works for me.  I don't think we're going to have any software that
depends on it being a class or a bus, so it's easy to change later.
All we really care about is /dev/nvmeN being created.

[PATCH] NVMe: Add a character device for each nvme device

[Jeff Garzik]

On 07/27/2012 04:26 PM, Matthew Wilcox wrote:
> Maybe we should design such a mechanism, but maybe we shouldn't ... as we
> find common things to do, we tend to move those to sysfs, not ioctls,
> and the kinds of commands that are being sent here are essentially
> vendor-specific NVMe commands; it's not clear they'd fit neatly into a
> generic mechanism.

You're delivering arbitrary packets to the device from userspace, and it 
is returning arbitrary packets to userspace.

This is a familiar pattern...  It is quite analagous to "send 
vendor-specific commands from userspace to a drive"

	Jeff

[PATCH] NVMe: Add a character device for each nvme device

[Matthew Wilcox]

On Fri, Jul 27, 2012@10:44:18AM -0600, Keith Busch wrote:
> @@ -1222,6 +1228,35 @@ static const struct block_device_operations nvme_fops = {
>  	.compat_ioctl	= nvme_ioctl,
>  };
>  
> +static long nvme_char_ioctl(struct file *f, unsigned int cmd, unsigned long arg)
> +{
> +	struct nvme_dev *dev;
> +	int instance = iminor(f->f_dentry->d_inode);
> +
> +	spin_lock(&dev_list_lock);
> +	list_for_each_entry(dev, &dev_list, node) {
> +		if (dev->instance == instance)
> +			break;
> +	}
> +	spin_unlock(&dev_list_lock);

So what happens if we get a PCI hotplug event here?  nvme_remove gets
called, we unmap the BAR and kfree the dev.  Now nvme_user_admin_cmd()
is going to dereference a pointer to freed memory, and even if that
happens to work, it's going to end up writing a doorbell that doesn't
exist any more.

I think we need refcounting on the dev to fix this ... urgh.

> +	if (&dev->node == &dev_list)
> +		return -ENOTTY;
> +	
> +	switch (cmd) {
> +	case NVME_IOCTL_ADMIN_CMD:
> +		return nvme_user_admin_cmd(dev, (void __user *)arg);
> +	default:
> +		return -ENOTTY;
> +	}
> +}

[PATCH] NVMe: Add a character device for each nvme device

[Keith Busch]

Registers a character device for the nvme module and creates character
files as /dev/nvmeN for each nvme device probed, where N is the device
instance. The character devices support nvme admin ioctl commands so
that nvme devices without namespaces can be managed.

Signed-off-by: Keith Busch <keith.busch at intel.com>
---
 drivers/block/nvme.c |   96 ++++++++++++++++++++++++++++++++++++++++++++++---
 1 files changed, 90 insertions(+), 6 deletions(-)

diff --git a/drivers/block/nvme.c b/drivers/block/nvme.c
index 3278fbd..3d13d4b 100644
--- a/drivers/block/nvme.c
+++ b/drivers/block/nvme.c
@@ -20,6 +20,7 @@
 #include <linux/bio.h>
 #include <linux/bitops.h>
 #include <linux/blkdev.h>
+#include <linux/cdev.h>
 #include <linux/delay.h>
 #include <linux/errno.h>
 #include <linux/fs.h>
@@ -45,6 +46,7 @@
 #define SQ_SIZE(depth)		(depth * sizeof(struct nvme_command))
 #define CQ_SIZE(depth)		(depth * sizeof(struct nvme_completion))
 #define NVME_MINORS 64
+#define NVME_MAX_DEVS 1024
 #define NVME_IO_TIMEOUT	(5 * HZ)
 #define ADMIN_TIMEOUT	(60 * HZ)
 
@@ -54,9 +56,15 @@ module_param(nvme_major, int, 0);
 static int use_threaded_interrupts;
 module_param(use_threaded_interrupts, int, 0);
 
+static int nvme_char_major;
+module_param(nvme_char_major, int, 0);
+
 static DEFINE_SPINLOCK(dev_list_lock);
 static LIST_HEAD(dev_list);
 static struct task_struct *nvme_thread;
+static struct class *nvme_char_cl;
+
+static void nvme_remove_dev(struct kref *kref);
 
 /*
  * Represents an NVM Express device.  Each nvme_dev is a PCI function.
@@ -75,6 +83,7 @@ struct nvme_dev {
 	struct msix_entry *entry;
 	struct nvme_bar __iomem *bar;
 	struct list_head namespaces;
+	struct kref kref;
 	char serial[20];
 	char model[40];
 	char firmware_rev[8];
@@ -1226,6 +1235,54 @@ static const struct block_device_operations nvme_fops = {
 	.compat_ioctl	= nvme_ioctl,
 };
 
+static int nvme_char_open(struct inode *inode, struct file *f)
+{
+	struct nvme_dev *dev;
+	int instance = iminor(f->f_dentry->d_inode);
+
+	spin_lock(&dev_list_lock);
+	list_for_each_entry(dev, &dev_list, node) {
+		if (dev->instance == instance) {
+			kref_get(&dev->kref);
+			f->private_data = dev;
+			break;
+		}
+	}
+	spin_unlock(&dev_list_lock);
+
+	if (!f->private_data)
+		return -ENXIO;
+
+	return 0;
+}
+
+static int nvme_char_release(struct inode *inode, struct file *f)
+{
+	struct nvme_dev *dev = f->private_data;
+	kref_put(&dev->kref, nvme_remove_dev);
+	return 0;
+}
+
+static long nvme_char_ioctl(struct file *f, unsigned int cmd, unsigned long arg)
+{
+	struct nvme_dev *dev = f->private_data;
+
+	switch (cmd) {
+	case NVME_IOCTL_ADMIN_CMD:
+		return nvme_user_admin_cmd(dev, (void __user *)arg);
+	default:
+		return -ENOTTY;
+	}
+}
+
+static const struct file_operations nvme_char_fops = {
+	.owner		= THIS_MODULE,
+	.open		= nvme_char_open,
+	.release	= nvme_char_release,
+	.unlocked_ioctl	= nvme_char_ioctl,
+	.compat_ioctl	= nvme_char_ioctl,
+};
+
 static void nvme_timeout_ios(struct nvme_queue *nvmeq)
 {
 	int depth = nvmeq->q_depth - 1;
@@ -1664,6 +1721,11 @@ static int __devinit nvme_probe(struct pci_dev *pdev,
 	if (result)
 		goto delete;
 
+	kref_init(&dev->kref);
+
+	device_create(nvme_char_cl, &pdev->dev,
+					MKDEV(nvme_char_major, dev->instance),
+					NULL, "nvme%d", dev->instance);
 	return 0;
 
  delete:
@@ -1688,21 +1750,28 @@ static int __devinit nvme_probe(struct pci_dev *pdev,
 	return result;
 }
 
-static void __devexit nvme_remove(struct pci_dev *pdev)
+static void nvme_remove_dev(struct kref *kref)
 {
-	struct nvme_dev *dev = pci_get_drvdata(pdev);
+	struct nvme_dev *dev = container_of(kref, struct nvme_dev, kref);
 	nvme_dev_remove(dev);
-	pci_disable_msix(pdev);
+	device_destroy(nvme_char_cl, MKDEV(nvme_char_major, dev->instance));
+	pci_disable_msix(dev->pci_dev);
 	iounmap(dev->bar);
 	nvme_release_instance(dev);
 	nvme_release_prp_pools(dev);
-	pci_disable_device(pdev);
-	pci_release_regions(pdev);
+	pci_disable_device(dev->pci_dev);
+	pci_release_regions(dev->pci_dev);
 	kfree(dev->queues);
 	kfree(dev->entry);
 	kfree(dev);
 }
 
+static void __devexit nvme_remove(struct pci_dev *pdev)
+{
+	struct nvme_dev *dev = pci_get_drvdata(pdev);
+	kref_put(&dev->kref, nvme_remove_dev);
+}
+
 /* These functions are yet to be implemented */
 #define nvme_error_detected NULL
 #define nvme_dump_registers NULL
@@ -1753,11 +1822,24 @@ static int __init nvme_init(void)
 	else if (result > 0)
 		nvme_major = result;
 
+	result = __register_chrdev(nvme_char_major, 0, NVME_MAX_DEVS, "nvme",
+							&nvme_char_fops);
+	if (result < 0)
+		goto unregister_blkdev;
+	else if (result > 0)
+		nvme_char_major = result;
+	nvme_char_cl = class_create(THIS_MODULE, "nvme");
+	if (!nvme_char_cl)
+		goto unregister_chrdev;
 	result = pci_register_driver(&nvme_driver);
 	if (result)
-		goto unregister_blkdev;
+		goto destroy_class;
 	return 0;
 
+ destroy_class:
+	class_destroy(nvme_char_cl);
+ unregister_chrdev:
+	__unregister_chrdev(nvme_char_major, 0, NVME_MAX_DEVS, "nvme");
  unregister_blkdev:
 	unregister_blkdev(nvme_major, "nvme");
  kill_kthread:
@@ -1768,6 +1850,8 @@ static int __init nvme_init(void)
 static void __exit nvme_exit(void)
 {
 	pci_unregister_driver(&nvme_driver);
+	class_destroy(nvme_char_cl);
+	__unregister_chrdev(nvme_char_major, 0, NVME_MAX_DEVS, "nvme");
 	unregister_blkdev(nvme_major, "nvme");
 	kthread_stop(nvme_thread);
 }
-- 
1.7.0.4