[PATCH] NVMe: Add reset controller sysfs entry

[PATCH] NVMe: Add reset controller sysfs entry

[Keith Busch]

We need the ability to perform an nvme controller reset as discussed on
the mailing list thread:

  http://lists.infradead.org/pipermail/linux-nvme/2015-March/001585.html

This adds a sysfs entry that when written to will reset perform an NVMe
controller reset if the controller was successfully initialized in the
first place.

This also adds locking around resetting the device in the async probe
method so the driver can't schedule two resets.

Signed-off-by: Keith Busch <keith.busch at intel.com>
Cc: Brandon Schultz <brandon.schulz at hgst.com>
Cc: David Sariel <david.sariel at pmcs.com>
---
The other proposals were to use an IOCTL, and I'm still open to Ack if
someone wants to submit that patch. This patch just makes it possible
to do with sysfs since I'm in a minority that prefers to script these
things rather than write a program to issue ioctls.

 drivers/block/nvme-core.c |   31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/drivers/block/nvme-core.c b/drivers/block/nvme-core.c
index 0d72ff2..07b92f4 100644
--- a/drivers/block/nvme-core.c
+++ b/drivers/block/nvme-core.c
@@ -2908,6 +2908,26 @@ static void nvme_reset_workfn(struct work_struct *work)
 	dev->reset_workfn(work);
 }
 
+static ssize_t nvme_reset(struct device *dev, struct device_attribute *attr,
+                                               const char *buf, size_t count)
+{
+	struct nvme_dev *ndev = dev_get_drvdata(dev);
+
+	if (!ndev->admin_q || blk_queue_dying(ndev->admin_q))
+		return -ENODEV;
+
+	spin_lock(&dev_list_lock);
+	if (!work_pending(&ndev->reset_work)) {
+		ndev->reset_workfn = nvme_reset_failed_dev;
+		queue_work(nvme_workq, &ndev->reset_work);
+	}
+	spin_unlock(&dev_list_lock);
+
+	flush_work(&ndev->reset_work);
+	return count;
+}
+static DEVICE_ATTR(reset_controller, S_IWUSR, NULL, nvme_reset);
+
 static void nvme_async_probe(struct work_struct *work);
 static int nvme_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 {
@@ -2952,12 +2972,20 @@ static int nvme_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 		goto release_pools;
 	}
 	get_device(dev->device);
+	dev_set_drvdata(dev->device, dev);
+
+	result = device_create_file(dev->device, &dev_attr_reset_controller);
+	if (result)
+		goto put_dev;
 
 	INIT_LIST_HEAD(&dev->node);
 	INIT_WORK(&dev->probe_work, nvme_async_probe);
 	schedule_work(&dev->probe_work);
 	return 0;
 
+ put_dev:
+	device_destroy(nvme_class, MKDEV(nvme_char_major, dev->instance));
+	put_device(dev->device);
  release_pools:
 	nvme_release_prp_pools(dev);
  release:
@@ -2986,10 +3014,12 @@ static void nvme_async_probe(struct work_struct *work)
 		goto reset;
 	return;
  reset:
+	spin_lock(&dev_list_lock);
 	if (!work_busy(&dev->reset_work)) {
 		dev->reset_workfn = nvme_reset_failed_dev;
 		queue_work(nvme_workq, &dev->reset_work);
 	}
+	spin_unlock(&dev_list_lock);
 }
 
 static void nvme_reset_notify(struct pci_dev *pdev, bool prepare)
@@ -3019,6 +3049,7 @@ static void nvme_remove(struct pci_dev *pdev)
 	pci_set_drvdata(pdev, NULL);
 	flush_work(&dev->probe_work);
 	flush_work(&dev->reset_work);
+	device_remove_file(dev->device, &dev_attr_reset_controller);
 	nvme_dev_shutdown(dev);
 	nvme_dev_remove(dev);
 	nvme_dev_remove_admin(dev);
-- 
1.7.10.4

[PATCH] NVMe: Add reset controller sysfs entry

[Brandon Schulz]

Keith -

Thanks for submitting this patch.  I think this sysfs mechanism is a good add.  I talked to Dave Darrington about getting it applied to our internal tree and let you know if he has any feedback as well.

I'm also interested in eventually providing the IOCTL you mentioned, because I think they have different use-cases.

David Sariel - Are you planning to revise the patch posted earlier for this, or should I talk to our team about that as well?

Brandon

-----Original Message-----
From: Keith Busch [mailto:keith.busch@intel.com] 
Sent: Friday, April 3, 2015 4:20 PM
To: linux-nvme at lists.infradead.org
Cc: Keith Busch; Brandon Schulz; David Sariel
Subject: [PATCH] NVMe: Add reset controller sysfs entry

We need the ability to perform an nvme controller reset as discussed on the mailing list thread:

  http://lists.infradead.org/pipermail/linux-nvme/2015-March/001585.html

This adds a sysfs entry that when written to will reset perform an NVMe controller reset if the controller was successfully initialized in the first place.

This also adds locking around resetting the device in the async probe method so the driver can't schedule two resets.

Signed-off-by: Keith Busch <keith.busch at intel.com>
Cc: Brandon Schultz <brandon.schulz at hgst.com>
Cc: David Sariel <david.sariel at pmcs.com>
---
The other proposals were to use an IOCTL, and I'm still open to Ack if someone wants to submit that patch. This patch just makes it possible to do with sysfs since I'm in a minority that prefers to script these things rather than write a program to issue ioctls.

 drivers/block/nvme-core.c |   31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/drivers/block/nvme-core.c b/drivers/block/nvme-core.c index 0d72ff2..07b92f4 100644
--- a/drivers/block/nvme-core.c
+++ b/drivers/block/nvme-core.c
@@ -2908,6 +2908,26 @@ static void nvme_reset_workfn(struct work_struct *work)
 	dev->reset_workfn(work);
 }
 
+static ssize_t nvme_reset(struct device *dev, struct device_attribute *attr,
+                                               const char *buf, size_t 
+count) {
+	struct nvme_dev *ndev = dev_get_drvdata(dev);
+
+	if (!ndev->admin_q || blk_queue_dying(ndev->admin_q))
+		return -ENODEV;
+
+	spin_lock(&dev_list_lock);
+	if (!work_pending(&ndev->reset_work)) {
+		ndev->reset_workfn = nvme_reset_failed_dev;
+		queue_work(nvme_workq, &ndev->reset_work);
+	}
+	spin_unlock(&dev_list_lock);
+
+	flush_work(&ndev->reset_work);
+	return count;
+}
+static DEVICE_ATTR(reset_controller, S_IWUSR, NULL, nvme_reset);
+
 static void nvme_async_probe(struct work_struct *work);  static int nvme_probe(struct pci_dev *pdev, const struct pci_device_id *id)  { @@ -2952,12 +2972,20 @@ static int nvme_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 		goto release_pools;
 	}
 	get_device(dev->device);
+	dev_set_drvdata(dev->device, dev);
+
+	result = device_create_file(dev->device, &dev_attr_reset_controller);
+	if (result)
+		goto put_dev;
 
 	INIT_LIST_HEAD(&dev->node);
 	INIT_WORK(&dev->probe_work, nvme_async_probe);
 	schedule_work(&dev->probe_work);
 	return 0;
 
+ put_dev:
+	device_destroy(nvme_class, MKDEV(nvme_char_major, dev->instance));
+	put_device(dev->device);
  release_pools:
 	nvme_release_prp_pools(dev);
  release:
@@ -2986,10 +3014,12 @@ static void nvme_async_probe(struct work_struct *work)
 		goto reset;
 	return;
  reset:
+	spin_lock(&dev_list_lock);
 	if (!work_busy(&dev->reset_work)) {
 		dev->reset_workfn = nvme_reset_failed_dev;
 		queue_work(nvme_workq, &dev->reset_work);
 	}
+	spin_unlock(&dev_list_lock);
 }
 
 static void nvme_reset_notify(struct pci_dev *pdev, bool prepare) @@ -3019,6 +3049,7 @@ static void nvme_remove(struct pci_dev *pdev)
 	pci_set_drvdata(pdev, NULL);
 	flush_work(&dev->probe_work);
 	flush_work(&dev->reset_work);
+	device_remove_file(dev->device, &dev_attr_reset_controller);
 	nvme_dev_shutdown(dev);
 	nvme_dev_remove(dev);
 	nvme_dev_remove_admin(dev);
--
1.7.10.4

[PATCH] NVMe: Add reset controller sysfs entry

[David Sariel]

Hi Keith, Brandon,
It took me a while to get hands on it. Attaching the patch according to Keith's remarks:

diff -uprN a/drivers/block/nvme-core.c b/drivers/block/nvme-core.c
--- a/drivers/block/nvme-core.c	2014-09-06 02:37:11.000000000 +0300
+++ b/drivers/block/nvme-core.c	2015-05-03 21:05:12.000000000 +0300
@@ -2627,6 +2627,18 @@ static long nvme_dev_ioctl(struct file *
 	switch (cmd) {
 	case NVME_IOCTL_ADMIN_CMD:
 		return nvme_user_admin_cmd(dev, (void __user *)arg);
+	case NVME_IOCTL_RESET:
+        if(work_busy(&dev->reset_work) )
+        	return -EBUSY;
+
+        dev_warn(&dev->pci_dev->dev, "resetting controller\n");
+        dev->reset_workfn = nvme_reset_failed_dev;
+        queue_work(nvme_workq, &dev->reset_work);
+
+        /* Wait for a work to finish executing the last queueing instance */
+        flush_work(&dev->reset_work);
+
+		return 0;
 	default:
 		return -ENOTTY;
 	}
diff -uprN a/include/uapi/linux/nvme.h b/include/uapi/linux/nvme.h
--- a/include/uapi/linux/nvme.h	2014-09-06 02:37:11.000000000 +0300
+++ b/include/uapi/linux/nvme.h	2015-05-03 21:05:30.000000000 +0300
@@ -513,5 +513,7 @@ struct nvme_admin_cmd {
 #define NVME_IOCTL_ID		_IO('N', 0x40)
 #define NVME_IOCTL_ADMIN_CMD	_IOWR('N', 0x41, struct nvme_admin_cmd)
 #define NVME_IOCTL_SUBMIT_IO	_IOW('N', 0x42, struct nvme_user_io)
+/* 0x43 is used by nvme-cli */
+#define NVME_IOCTL_RESET  _IO('N', 0x44)
 
 #endif /* _UAPI_LINUX_NVME_H */


Although the use case is for the reset is for pre 3.15 kernels, I guess that the patch should be applied to the latest kernel. Am I right?
Thanks
David


-----Original Message-----
From: Brandon Schulz [mailto:brandon.schulz@hgst.com] 
Sent: Monday, April 06, 2015 7:56 PM
To: Keith Busch; linux-nvme at lists.infradead.org; David Sariel
Cc: David Darrington
Subject: RE: [PATCH] NVMe: Add reset controller sysfs entry

Keith -

Thanks for submitting this patch.  I think this sysfs mechanism is a good add.  I talked to Dave Darrington about getting it applied to our internal tree and let you know if he has any feedback as well.

I'm also interested in eventually providing the IOCTL you mentioned, because I think they have different use-cases.

David Sariel - Are you planning to revise the patch posted earlier for this, or should I talk to our team about that as well?

Brandon

-----Original Message-----
From: Keith Busch [mailto:keith.busch@intel.com] 
Sent: Friday, April 3, 2015 4:20 PM
To: linux-nvme at lists.infradead.org
Cc: Keith Busch; Brandon Schulz; David Sariel
Subject: [PATCH] NVMe: Add reset controller sysfs entry

We need the ability to perform an nvme controller reset as discussed on the mailing list thread:

  http://lists.infradead.org/pipermail/linux-nvme/2015-March/001585.html

This adds a sysfs entry that when written to will reset perform an NVMe controller reset if the controller was successfully initialized in the first place.

This also adds locking around resetting the device in the async probe method so the driver can't schedule two resets.

Signed-off-by: Keith Busch <keith.busch at intel.com>
Cc: Brandon Schultz <brandon.schulz at hgst.com>
Cc: David Sariel <david.sariel at pmcs.com>
---
The other proposals were to use an IOCTL, and I'm still open to Ack if someone wants to submit that patch. This patch just makes it possible to do with sysfs since I'm in a minority that prefers to script these things rather than write a program to issue ioctls.

 drivers/block/nvme-core.c |   31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/drivers/block/nvme-core.c b/drivers/block/nvme-core.c index 0d72ff2..07b92f4 100644
--- a/drivers/block/nvme-core.c
+++ b/drivers/block/nvme-core.c
@@ -2908,6 +2908,26 @@ static void nvme_reset_workfn(struct work_struct *work)
 	dev->reset_workfn(work);
 }
 
+static ssize_t nvme_reset(struct device *dev, struct device_attribute *attr,
+                                               const char *buf, size_t 
+count) {
+	struct nvme_dev *ndev = dev_get_drvdata(dev);
+
+	if (!ndev->admin_q || blk_queue_dying(ndev->admin_q))
+		return -ENODEV;
+
+	spin_lock(&dev_list_lock);
+	if (!work_pending(&ndev->reset_work)) {
+		ndev->reset_workfn = nvme_reset_failed_dev;
+		queue_work(nvme_workq, &ndev->reset_work);
+	}
+	spin_unlock(&dev_list_lock);
+
+	flush_work(&ndev->reset_work);
+	return count;
+}
+static DEVICE_ATTR(reset_controller, S_IWUSR, NULL, nvme_reset);
+
 static void nvme_async_probe(struct work_struct *work);  static int nvme_probe(struct pci_dev *pdev, const struct pci_device_id *id)  { @@ -2952,12 +2972,20 @@ static int nvme_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 		goto release_pools;
 	}
 	get_device(dev->device);
+	dev_set_drvdata(dev->device, dev);
+
+	result = device_create_file(dev->device, &dev_attr_reset_controller);
+	if (result)
+		goto put_dev;
 
 	INIT_LIST_HEAD(&dev->node);
 	INIT_WORK(&dev->probe_work, nvme_async_probe);
 	schedule_work(&dev->probe_work);
 	return 0;
 
+ put_dev:
+	device_destroy(nvme_class, MKDEV(nvme_char_major, dev->instance));
+	put_device(dev->device);
  release_pools:
 	nvme_release_prp_pools(dev);
  release:
@@ -2986,10 +3014,12 @@ static void nvme_async_probe(struct work_struct *work)
 		goto reset;
 	return;
  reset:
+	spin_lock(&dev_list_lock);
 	if (!work_busy(&dev->reset_work)) {
 		dev->reset_workfn = nvme_reset_failed_dev;
 		queue_work(nvme_workq, &dev->reset_work);
 	}
+	spin_unlock(&dev_list_lock);
 }
 
 static void nvme_reset_notify(struct pci_dev *pdev, bool prepare) @@ -3019,6 +3049,7 @@ static void nvme_remove(struct pci_dev *pdev)
 	pci_set_drvdata(pdev, NULL);
 	flush_work(&dev->probe_work);
 	flush_work(&dev->reset_work);
+	device_remove_file(dev->device, &dev_attr_reset_controller);
 	nvme_dev_shutdown(dev);
 	nvme_dev_remove(dev);
 	nvme_dev_remove_admin(dev);
--
1.7.10.4

[PATCH] NVMe: Add reset controller sysfs entry

[Keith Busch]

On Sun, 3 May 2015, David Sariel wrote:
> Hi Keith, Brandon,
> It took me a while to get hands on it. Attaching the patch according to Keith's remarks:

Could you send this in a format that can be applied? We need a proper
patch with commit log, and I think your email client is mucking up
the formatting.

> diff -uprN a/drivers/block/nvme-core.c b/drivers/block/nvme-core.c
> --- a/drivers/block/nvme-core.c	2014-09-06 02:37:11.000000000 +0300
> +++ b/drivers/block/nvme-core.c	2015-05-03 21:05:12.000000000 +0300
> @@ -2627,6 +2627,18 @@ static long nvme_dev_ioctl(struct file *
> 	switch (cmd) {
> 	case NVME_IOCTL_ADMIN_CMD:
> 		return nvme_user_admin_cmd(dev, (void __user *)arg);
> +	case NVME_IOCTL_RESET:
> +        if(work_busy(&dev->reset_work) )
> +        	return -EBUSY;
> +
> +        dev_warn(&dev->pci_dev->dev, "resetting controller\n");
> +        dev->reset_workfn = nvme_reset_failed_dev;
> +        queue_work(nvme_workq, &dev->reset_work);
> +
> +        /* Wait for a work to finish executing the last queueing instance */
> +        flush_work(&dev->reset_work);
> +
> +		return 0;
> 	default:
> 		return -ENOTTY;
> 	}
> diff -uprN a/include/uapi/linux/nvme.h b/include/uapi/linux/nvme.h
> --- a/include/uapi/linux/nvme.h	2014-09-06 02:37:11.000000000 +0300
> +++ b/include/uapi/linux/nvme.h	2015-05-03 21:05:30.000000000 +0300
> @@ -513,5 +513,7 @@ struct nvme_admin_cmd {
> #define NVME_IOCTL_ID		_IO('N', 0x40)
> #define NVME_IOCTL_ADMIN_CMD	_IOWR('N', 0x41, struct nvme_admin_cmd)
> #define NVME_IOCTL_SUBMIT_IO	_IOW('N', 0x42, struct nvme_user_io)
> +/* 0x43 is used by nvme-cli */
> +#define NVME_IOCTL_RESET  _IO('N', 0x44)
>
> #endif /* _UAPI_LINUX_NVME_H */
>
>
> Although the use case is for the reset is for pre 3.15 kernels, I guess that the patch should be applied to the latest kernel. Am I right?

This is actually useful upstream as well if we don't want to rely on
the pci reset to hook into NVMe controller resets. Sometimes we don't
even have pci reset available.

[PATCH] NVMe: Add reset controller sysfs entry

[David Sariel]

Hi Keith
Attaching the patch as a file. I have patched against https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git.
Hope it's ok now. LMK if anything else is needed.
Thanks!
D



-----Original Message-----
From: Keith Busch [mailto:keith.busch@intel.com] 
Sent: Tuesday, May 12, 2015 6:11 PM
To: David Sariel
Cc: Brandon Schulz; Keith Busch; linux-nvme at lists.infradead.org; David Darrington
Subject: RE: [PATCH] NVMe: Add reset controller sysfs entry

On Sun, 3 May 2015, David Sariel wrote:
> Hi Keith, Brandon,
> It took me a while to get hands on it. Attaching the patch according to Keith's remarks:

Could you send this in a format that can be applied? We need a proper patch with commit log, and I think your email client is mucking up the formatting.

> diff -uprN a/drivers/block/nvme-core.c b/drivers/block/nvme-core.c
> --- a/drivers/block/nvme-core.c	2014-09-06 02:37:11.000000000 +0300
> +++ b/drivers/block/nvme-core.c	2015-05-03 21:05:12.000000000 +0300
> @@ -2627,6 +2627,18 @@ static long nvme_dev_ioctl(struct file *
> 	switch (cmd) {
> 	case NVME_IOCTL_ADMIN_CMD:
> 		return nvme_user_admin_cmd(dev, (void __user *)arg);
> +	case NVME_IOCTL_RESET:
> +        if(work_busy(&dev->reset_work) )
> +        	return -EBUSY;
> +
> +        dev_warn(&dev->pci_dev->dev, "resetting controller\n");
> +        dev->reset_workfn = nvme_reset_failed_dev;
> +        queue_work(nvme_workq, &dev->reset_work);
> +
> +        /* Wait for a work to finish executing the last queueing instance */
> +        flush_work(&dev->reset_work);
> +
> +		return 0;
> 	default:
> 		return -ENOTTY;
> 	}
> diff -uprN a/include/uapi/linux/nvme.h b/include/uapi/linux/nvme.h
> --- a/include/uapi/linux/nvme.h	2014-09-06 02:37:11.000000000 +0300
> +++ b/include/uapi/linux/nvme.h	2015-05-03 21:05:30.000000000 +0300
> @@ -513,5 +513,7 @@ struct nvme_admin_cmd {
> #define NVME_IOCTL_ID		_IO('N', 0x40)
> #define NVME_IOCTL_ADMIN_CMD	_IOWR('N', 0x41, struct nvme_admin_cmd)
> #define NVME_IOCTL_SUBMIT_IO	_IOW('N', 0x42, struct nvme_user_io)
> +/* 0x43 is used by nvme-cli */
> +#define NVME_IOCTL_RESET  _IO('N', 0x44)
>
> #endif /* _UAPI_LINUX_NVME_H */
>
>
> Although the use case is for the reset is for pre 3.15 kernels, I guess that the patch should be applied to the latest kernel. Am I right?

This is actually useful upstream as well if we don't want to rely on the pci reset to hook into NVMe controller resets. Sometimes we don't even have pci reset available.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: reset_ioctl.patch
Type: application/octet-stream
Size: 1331 bytes
Desc: reset_ioctl.patch
URL: <http://lists.infradead.org/pipermail/linux-nvme/attachments/20150518/68ef3a8c/attachment.obj>

FW: [PATCH] NVMe: Add reset controller sysfs entry

[David Sariel]

Hi Keith, Jens,
Is everything ok with the patch? Do you need anything else from me?
Thanks
D

-----Original Message-----
From: Linux-nvme [mailto:linux-nvme-bounces@lists.infradead.org] On Behalf Of David Sariel
Sent: Monday, May 18, 2015 7:58 PM
To: Keith Busch
Cc: David Darrington; Brandon Schulz; linux-nvme at lists.infradead.org
Subject: RE: [PATCH] NVMe: Add reset controller sysfs entry

Hi Keith
Attaching the patch as a file. I have patched against https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git.
Hope it's ok now. LMK if anything else is needed.
Thanks!
D



-----Original Message-----
From: Keith Busch [mailto:keith.busch@intel.com] 
Sent: Tuesday, May 12, 2015 6:11 PM
To: David Sariel
Cc: Brandon Schulz; Keith Busch; linux-nvme at lists.infradead.org; David Darrington
Subject: RE: [PATCH] NVMe: Add reset controller sysfs entry

On Sun, 3 May 2015, David Sariel wrote:
> Hi Keith, Brandon,
> It took me a while to get hands on it. Attaching the patch according to Keith's remarks:

Could you send this in a format that can be applied? We need a proper patch with commit log, and I think your email client is mucking up the formatting.

> diff -uprN a/drivers/block/nvme-core.c b/drivers/block/nvme-core.c
> --- a/drivers/block/nvme-core.c	2014-09-06 02:37:11.000000000 +0300
> +++ b/drivers/block/nvme-core.c	2015-05-03 21:05:12.000000000 +0300
> @@ -2627,6 +2627,18 @@ static long nvme_dev_ioctl(struct file *
> 	switch (cmd) {
> 	case NVME_IOCTL_ADMIN_CMD:
> 		return nvme_user_admin_cmd(dev, (void __user *)arg);
> +	case NVME_IOCTL_RESET:
> +        if(work_busy(&dev->reset_work) )
> +        	return -EBUSY;
> +
> +        dev_warn(&dev->pci_dev->dev, "resetting controller\n");
> +        dev->reset_workfn = nvme_reset_failed_dev;
> +        queue_work(nvme_workq, &dev->reset_work);
> +
> +        /* Wait for a work to finish executing the last queueing instance */
> +        flush_work(&dev->reset_work);
> +
> +		return 0;
> 	default:
> 		return -ENOTTY;
> 	}
> diff -uprN a/include/uapi/linux/nvme.h b/include/uapi/linux/nvme.h
> --- a/include/uapi/linux/nvme.h	2014-09-06 02:37:11.000000000 +0300
> +++ b/include/uapi/linux/nvme.h	2015-05-03 21:05:30.000000000 +0300
> @@ -513,5 +513,7 @@ struct nvme_admin_cmd {
> #define NVME_IOCTL_ID		_IO('N', 0x40)
> #define NVME_IOCTL_ADMIN_CMD	_IOWR('N', 0x41, struct nvme_admin_cmd)
> #define NVME_IOCTL_SUBMIT_IO	_IOW('N', 0x42, struct nvme_user_io)
> +/* 0x43 is used by nvme-cli */
> +#define NVME_IOCTL_RESET  _IO('N', 0x44)
>
> #endif /* _UAPI_LINUX_NVME_H */
>
>
> Although the use case is for the reset is for pre 3.15 kernels, I guess that the patch should be applied to the latest kernel. Am I right?

This is actually useful upstream as well if we don't want to rely on the pci reset to hook into NVMe controller resets. Sometimes we don't even have pci reset available.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: reset_ioctl.patch
Type: application/octet-stream
Size: 1331 bytes
Desc: reset_ioctl.patch
URL: <http://lists.infradead.org/pipermail/linux-nvme/attachments/20150531/eb9e762e/attachment.obj>

FW: [PATCH] NVMe: Add reset controller sysfs entry

[Keith Busch]

On Sun, 31 May 2015, David Sariel wrote:
> Hi Keith, Jens,
> Is everything ok with the patch? Do you need anything else from me?
> Thanks
> D

Looks goot to me. This is as far as I can take it though.

[PATCH] NVMe: Add reset controller sysfs entry

[Keith Busch]

On Fri, 5 Jun 2015, Jens Axboe wrote:
> On 06/04/2015 04:08 PM, Keith Busch wrote:
>> Thanks for fixing the merge. This tests successfully on my machine. I
>> didn't have a strong opinion either way on what a user sees when
>> requesting resets while a reset is in progress, and what you've merged
>> looks good to me.
>
> Great, thanks Keith. Can I add your signed-off-by? It'd be most fair to 
> attribute the patch to you, since the ioctl part is now just calling your 
> reset function and all I did was shuffle some stuff around.

Sounds good to me. Thanks!

[PATCH] NVMe: Add reset controller sysfs entry

[Jens Axboe]

On 06/04/2015 04:08 PM, Keith Busch wrote:
>
>
> On Thu, 4 Jun 2015, Jens Axboe wrote:
>
>> On 06/04/2015 02:00 PM, Jens Axboe wrote:
>>> On 06/04/2015 09:39 AM, Christoph Hellwig wrote:
>>>> On Thu, Jun 04, 2015@03:26:03PM +0000, Keith Busch wrote:
>>>>> On Thu, 4 Jun 2015, Brandon Schulz wrote:
>>>>>> Do you plan to merge this into your legacy tree somewhere that we can
>>>>>> reference in Bugzillas/etc. with the Linux distro vendors?
>>>>>
>>>>> There is a strong preference with vendors to see fixes upstream prior
>>>>> to backporting but this is a long-standing percieved weakness in Linux
>>>>> vs. other operating systems; no one wants to reboot their machine for
>>>>> a f/w upgrade ... so, okay, I'll apply it to legacy and see if I can
>>>>> lobby OSVs to take out-of-stream fixes.
>>>>
>>>> Eww.  Jens, can you please just apply this patch so that we're not
>>>> having such a mess?
>>>
>>> Yeah, I'll apply it now for 4.2.
>>
>> Well, that was a bit of a mess. Attached is my quick attempt at
>> combining the two, and reusing the reset code from the ioctl path as
>> well. One functional change was NOT flushing the work if we don't
>> queue it in nvme_reset(), the previous code from Keith did that.
>> Otherwise it should be straight forward.
>>
>> Please review, I'll hold off on committing this.
>
> Hi Jens,
>
> Thanks for fixing the merge. This tests successfully on my machine. I
> didn't have a strong opinion either way on what a user sees when
> requesting resets while a reset is in progress, and what you've merged
> looks good to me.

Great, thanks Keith. Can I add your signed-off-by? It'd be most fair to 
attribute the patch to you, since the ioctl part is now just calling 
your reset function and all I did was shuffle some stuff around.

-- 
Jens Axboe

[PATCH] NVMe: Add reset controller sysfs entry

[Keith Busch]

On Thu, 4 Jun 2015, Jens Axboe wrote:

> On 06/04/2015 02:00 PM, Jens Axboe wrote:
>> On 06/04/2015 09:39 AM, Christoph Hellwig wrote:
>>> On Thu, Jun 04, 2015@03:26:03PM +0000, Keith Busch wrote:
>>>> On Thu, 4 Jun 2015, Brandon Schulz wrote:
>>>>> Do you plan to merge this into your legacy tree somewhere that we can
>>>>> reference in Bugzillas/etc. with the Linux distro vendors?
>>>> 
>>>> There is a strong preference with vendors to see fixes upstream prior
>>>> to backporting but this is a long-standing percieved weakness in Linux
>>>> vs. other operating systems; no one wants to reboot their machine for
>>>> a f/w upgrade ... so, okay, I'll apply it to legacy and see if I can
>>>> lobby OSVs to take out-of-stream fixes.
>>> 
>>> Eww.  Jens, can you please just apply this patch so that we're not
>>> having such a mess?
>> 
>> Yeah, I'll apply it now for 4.2.
>
> Well, that was a bit of a mess. Attached is my quick attempt at combining the 
> two, and reusing the reset code from the ioctl path as well. One functional 
> change was NOT flushing the work if we don't queue it in nvme_reset(), the 
> previous code from Keith did that. Otherwise it should be straight forward.
>
> Please review, I'll hold off on committing this.

Hi Jens,

Thanks for fixing the merge. This tests successfully on my machine. I
didn't have a strong opinion either way on what a user sees when
requesting resets while a reset is in progress, and what you've merged
looks good to me.

[PATCH] NVMe: Add reset controller sysfs entry

[Jens Axboe]

On 06/04/2015 02:00 PM, Jens Axboe wrote:
> On 06/04/2015 09:39 AM, Christoph Hellwig wrote:
>> On Thu, Jun 04, 2015@03:26:03PM +0000, Keith Busch wrote:
>>> On Thu, 4 Jun 2015, Brandon Schulz wrote:
>>>> Do you plan to merge this into your legacy tree somewhere that we can
>>>> reference in Bugzillas/etc. with the Linux distro vendors?
>>>
>>> There is a strong preference with vendors to see fixes upstream prior
>>> to backporting but this is a long-standing percieved weakness in Linux
>>> vs. other operating systems; no one wants to reboot their machine for
>>> a f/w upgrade ... so, okay, I'll apply it to legacy and see if I can
>>> lobby OSVs to take out-of-stream fixes.
>>
>> Eww.  Jens, can you please just apply this patch so that we're not
>> having such a mess?
>
> Yeah, I'll apply it now for 4.2.

Well, that was a bit of a mess. Attached is my quick attempt at 
combining the two, and reusing the reset code from the ioctl path as 
well. One functional change was NOT flushing the work if we don't queue 
it in nvme_reset(), the previous code from Keith did that. Otherwise it 
should be straight forward.

Please review, I'll hold off on committing this.

-- 
Jens Axboe

-------------- next part --------------
A non-text attachment was scrubbed...
Name: nvme-reset.patch
Type: text/x-patch
Size: 3651 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-nvme/attachments/20150604/ff0c718e/attachment.bin>

[PATCH] NVMe: Add reset controller sysfs entry

[Jens Axboe]

On 06/04/2015 09:39 AM, Christoph Hellwig wrote:
> On Thu, Jun 04, 2015@03:26:03PM +0000, Keith Busch wrote:
>> On Thu, 4 Jun 2015, Brandon Schulz wrote:
>>> Do you plan to merge this into your legacy tree somewhere that we can
>>> reference in Bugzillas/etc. with the Linux distro vendors?
>>
>> There is a strong preference with vendors to see fixes upstream prior
>> to backporting but this is a long-standing percieved weakness in Linux
>> vs. other operating systems; no one wants to reboot their machine for
>> a f/w upgrade ... so, okay, I'll apply it to legacy and see if I can
>> lobby OSVs to take out-of-stream fixes.
>
> Eww.  Jens, can you please just apply this patch so that we're not
> having such a mess?

Yeah, I'll apply it now for 4.2.

-- 
Jens Axboe

[PATCH] NVMe: Add reset controller sysfs entry

[Christoph Hellwig]

On Thu, Jun 04, 2015@03:26:03PM +0000, Keith Busch wrote:
> On Thu, 4 Jun 2015, Brandon Schulz wrote:
> >Do you plan to merge this into your legacy tree somewhere that we can
> >reference in Bugzillas/etc. with the Linux distro vendors?
> 
> There is a strong preference with vendors to see fixes upstream prior
> to backporting but this is a long-standing percieved weakness in Linux
> vs. other operating systems; no one wants to reboot their machine for
> a f/w upgrade ... so, okay, I'll apply it to legacy and see if I can
> lobby OSVs to take out-of-stream fixes.

Eww.  Jens, can you please just apply this patch so that we're not
having such a mess?

[PATCH] NVMe: Add reset controller sysfs entry

[Keith Busch]

On Thu, 4 Jun 2015, Brandon Schulz wrote:
> Do you plan to merge this into your legacy tree somewhere that we can
> reference in Bugzillas/etc. with the Linux distro vendors?

There is a strong preference with vendors to see fixes upstream prior
to backporting but this is a long-standing percieved weakness in Linux
vs. other operating systems; no one wants to reboot their machine for
a f/w upgrade ... so, okay, I'll apply it to legacy and see if I can
lobby OSVs to take out-of-stream fixes.

> On 6/1/15, 9:13 AM, "Keith Busch" <keith.busch@intel.com> wrote:
>
>> On Sun, 31 May 2015, David Sariel wrote:
>>> Hi Keith, Jens,
>>> Is everything ok with the patch? Do you need anything else from me?
>>> Thanks
>>> D
>>
>> Looks goot to me. This is as far as I can take it though.