Linux-NVME Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] nvme: log additional message for controller status
@ 2020-02-24 16:32 Rupesh Girase
  2020-02-24 17:06 ` Christoph Hellwig
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Rupesh Girase @ 2020-02-24 16:32 UTC (permalink / raw)
  To: kbusch; +Cc: axboe, Rupesh Girase, hch, linux-nvme

	Logging the controller fatal and ready status would help to
	identfy if issue lies within kernel nvme subsytem or
	controller is unhealthy.

Signed-off-by: Rupesh Girase <rgirase@redhat.com>
---
 drivers/nvme/host/core.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index ada59df..2dfca9d 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -2079,8 +2079,9 @@ static int nvme_wait_ready(struct nvme_ctrl *ctrl, u64 cap, bool enabled)
 			return -EINTR;
 		if (time_after(jiffies, timeout)) {
 			dev_err(ctrl->device,
-				"Device not ready; aborting %s\n", enabled ?
-						"initialisation" : "reset");
+				"Device not ready; aborting %s, RDY=0x%x, CFS=0x%x\n",
+				enabled ? "initialisation" : "reset",
+				csts & NVME_CSTS_RDY, csts & NVME_CSTS_CFS);
 			return -ENODEV;
 		}
 	}
-- 
1.8.3.1


_______________________________________________
linux-nvme mailing list
linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] nvme: log additional message for controller status
  2020-02-24 16:32 [PATCH] nvme: log additional message for controller status Rupesh Girase
@ 2020-02-24 17:06 ` Christoph Hellwig
  2020-02-24 18:54 ` Chaitanya Kulkarni
  2020-02-25 15:55 ` Keith Busch
  2 siblings, 0 replies; 5+ messages in thread
From: Christoph Hellwig @ 2020-02-24 17:06 UTC (permalink / raw)
  To: Rupesh Girase; +Cc: kbusch, axboe, hch, linux-nvme

On Mon, Feb 24, 2020 at 10:02:04PM +0530, Rupesh Girase wrote:
> 	Logging the controller fatal and ready status would help to
> 	identfy if issue lies within kernel nvme subsytem or
> 	controller is unhealthy.
> 
> Signed-off-by: Rupesh Girase <rgirase@redhat.com>

There should be no tabs before the commit message.

Otherwise this looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

_______________________________________________
linux-nvme mailing list
linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] nvme: log additional message for controller status
  2020-02-24 16:32 [PATCH] nvme: log additional message for controller status Rupesh Girase
  2020-02-24 17:06 ` Christoph Hellwig
@ 2020-02-24 18:54 ` Chaitanya Kulkarni
  2020-02-25 15:55 ` Keith Busch
  2 siblings, 0 replies; 5+ messages in thread
From: Chaitanya Kulkarni @ 2020-02-24 18:54 UTC (permalink / raw)
  To: Rupesh Girase, kbusch; +Cc: axboe, hch, linux-nvme

Looks good, with tabs in commit log fixed at the time of applying
patch.

Reviewed-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>

On 2/24/20 8:34 AM, Rupesh Girase wrote:
> 	Logging the controller fatal and ready status would help to
> 	identfy if issue lies within kernel nvme subsytem or
> 	controller is unhealthy.
>
> Signed-off-by: Rupesh Girase <rgirase@redhat.com>



_______________________________________________
linux-nvme mailing list
linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] nvme: log additional message for controller status
  2020-02-24 16:32 [PATCH] nvme: log additional message for controller status Rupesh Girase
  2020-02-24 17:06 ` Christoph Hellwig
  2020-02-24 18:54 ` Chaitanya Kulkarni
@ 2020-02-25 15:55 ` Keith Busch
  2020-02-25 20:24   ` Sagi Grimberg
  2 siblings, 1 reply; 5+ messages in thread
From: Keith Busch @ 2020-02-25 15:55 UTC (permalink / raw)
  To: Rupesh Girase; +Cc: axboe, hch, linux-nvme

On Mon, Feb 24, 2020 at 10:02:04PM +0530, Rupesh Girase wrote:
>  			dev_err(ctrl->device,
> -				"Device not ready; aborting %s\n", enabled ?
> -						"initialisation" : "reset");
> +				"Device not ready; aborting %s, RDY=0x%x, CFS=0x%x\n",
> +				enabled ? "initialisation" : "reset",
> +				csts & NVME_CSTS_RDY, csts & NVME_CSTS_CFS);

The RDY bit is a bit redundant. If "initialisation", we know it's not 1,
and if "reset", we know it's not 0. We also know it's not all 1's if we
reach here.

CFS may be useful, but the print will look a strange. If CFS is set,
you'll see "CFS=0x2", but the CFS field can only be either 0 or 1. I say
just print out the entire csts field in case SHST or NSSRO may provide
insights to the vendor.

_______________________________________________
linux-nvme mailing list
linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] nvme: log additional message for controller status
  2020-02-25 15:55 ` Keith Busch
@ 2020-02-25 20:24   ` Sagi Grimberg
  0 siblings, 0 replies; 5+ messages in thread
From: Sagi Grimberg @ 2020-02-25 20:24 UTC (permalink / raw)
  To: Keith Busch, Rupesh Girase; +Cc: axboe, hch, linux-nvme


> On Mon, Feb 24, 2020 at 10:02:04PM +0530, Rupesh Girase wrote:
>>   			dev_err(ctrl->device,
>> -				"Device not ready; aborting %s\n", enabled ?
>> -						"initialisation" : "reset");
>> +				"Device not ready; aborting %s, RDY=0x%x, CFS=0x%x\n",
>> +				enabled ? "initialisation" : "reset",
>> +				csts & NVME_CSTS_RDY, csts & NVME_CSTS_CFS);
> 
> The RDY bit is a bit redundant. If "initialisation", we know it's not 1,
> and if "reset", we know it's not 0. We also know it's not all 1's if we
> reach here.
> 
> CFS may be useful, but the print will look a strange. If CFS is set,
> you'll see "CFS=0x2", but the CFS field can only be either 0 or 1. I say
> just print out the entire csts field in case SHST or NSSRO may provide
> insights to the vendor.

Suggestions sound fine, looks reasonable either way.

Reviewed-by: Sagi Grimberg <sagi@grimberg.me>

_______________________________________________
linux-nvme mailing list
linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, back to index

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-24 16:32 [PATCH] nvme: log additional message for controller status Rupesh Girase
2020-02-24 17:06 ` Christoph Hellwig
2020-02-24 18:54 ` Chaitanya Kulkarni
2020-02-25 15:55 ` Keith Busch
2020-02-25 20:24   ` Sagi Grimberg

Linux-NVME Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-nvme/0 linux-nvme/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-nvme linux-nvme/ https://lore.kernel.org/linux-nvme \
		linux-nvme@lists.infradead.org
	public-inbox-index linux-nvme

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.infradead.lists.linux-nvme


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git