All of lore.kernel.org
 help / color / mirror / Atom feed
From: willy@linux.intel.com (Matthew Wilcox)
Subject: [PATCH] NVMe: Add a character device for each nvme device
Date: Fri, 27 Jul 2012 14:12:12 -0400	[thread overview]
Message-ID: <20120727181212.GM22985@linux.intel.com> (raw)
In-Reply-To: <1343407458-29909-1-git-send-email-keith.busch@intel.com>

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

WARNING: multiple messages have this Message-ID (diff)
From: Matthew Wilcox <willy@linux.intel.com>
To: Keith Busch <keith.busch@intel.com>
Cc: linux-nvme@lists.infradead.org, linux-kernel@vger.kernel.org,
	Alan Cox <alan@lxorguk.ukuu.org.uk>, Greg KH <greg@kroah.com>
Subject: Re: [PATCH] NVMe: Add a character device for each nvme device
Date: Fri, 27 Jul 2012 14:12:12 -0400	[thread overview]
Message-ID: <20120727181212.GM22985@linux.intel.com> (raw)
In-Reply-To: <1343407458-29909-1-git-send-email-keith.busch@intel.com>

On Fri, Jul 27, 2012 at 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@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@lists.infradead.org
> http://merlin.infradead.org/mailman/listinfo/linux-nvme

  reply	other threads:[~2012-07-27 18:12 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-07-27 16:44 [PATCH] NVMe: Add a character device for each nvme device Keith Busch
2012-07-27 18:12 ` Matthew Wilcox [this message]
2012-07-27 18:12   ` Matthew Wilcox
2012-07-27 18:25   ` Greg KH
2012-07-27 18:25     ` Greg KH
2012-07-27 19:08     ` Matthew Wilcox
2012-07-27 19:08       ` Matthew Wilcox
2012-07-27 19:21       ` Greg KH
2012-07-27 19:21         ` Greg KH
2012-07-27 20:30         ` Matthew Wilcox
2012-07-27 20:30           ` Matthew Wilcox
2012-07-27 19:28   ` Jeff Garzik
2012-07-27 19:28     ` Jeff Garzik
2012-07-27 20:26     ` Matthew Wilcox
2012-07-27 20:26       ` Matthew Wilcox
2012-07-27 20:42       ` Jeff Garzik
2012-07-27 20:42         ` Jeff Garzik
2012-07-27 20:44 ` Matthew Wilcox
  -- strict thread matches above, loose matches on Subject: below --
2012-08-02 19:10 Keith Busch

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20120727181212.GM22985@linux.intel.com \
    --to=willy@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.