[PATCH 1/1] nvme: Modify the debug level for setting shutdown timeout

[PATCH 1/1] nvme: Modify the debug level for setting shutdown timeout

[Max Gurtovoy]

When an NVMe controller reports RTD3 Entry Latency larger than
the value of shutdown_timeout module parameter, we update the
shutdown_timeout accordingly to honor RTD3 Entry Latency. Use an
Informational debug level instead of a Warning level for it.

Signed-off-by: Max Gurtovoy <maxg at mellanox.com>
---
 drivers/nvme/host/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 1e46e60..4643876 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -2253,7 +2253,7 @@ int nvme_init_identify(struct nvme_ctrl *ctrl)
 						 shutdown_timeout, 60);
 
 		if (ctrl->shutdown_timeout != shutdown_timeout)
-			dev_warn(ctrl->device,
+			dev_info(ctrl->device,
 				 "Shutdown timeout set to %u seconds\n",
 				 ctrl->shutdown_timeout);
 	} else
-- 
1.8.3.1

[PATCH 1/1] nvme: Modify the debug level for setting shutdown timeout

[Christoph Hellwig]

On Sun, Dec 31, 2017@03:33:27PM +0200, Max Gurtovoy wrote:
> When an NVMe controller reports RTD3 Entry Latency larger than
> the value of shutdown_timeout module parameter, we update the
> shutdown_timeout accordingly to honor RTD3 Entry Latency. Use an
> Informational debug level instead of a Warning level for it.
> 
> Signed-off-by: Max Gurtovoy <maxg at mellanox.com>

I'm ok with just info, but in the end it seems like a pretty
buggy device..

[PATCH 1/1] nvme: Modify the debug level for setting shutdown timeout

[Max Gurtovoy]

On 1/4/2018 12:04 PM, Christoph Hellwig wrote:
> On Sun, Dec 31, 2017@03:33:27PM +0200, Max Gurtovoy wrote:
>> When an NVMe controller reports RTD3 Entry Latency larger than
>> the value of shutdown_timeout module parameter, we update the
>> shutdown_timeout accordingly to honor RTD3 Entry Latency. Use an
>> Informational debug level instead of a Warning level for it.
>>
>> Signed-off-by: Max Gurtovoy <maxg at mellanox.com>
> 
> I'm ok with just info, but in the end it seems like a pretty
> buggy device..
> 

but shutdown_timeout is a module param that one can set to be 10 secs 
(for example) and then it might be == transition_time and we won't see 
the print. Then is it not a buggy device anymore ? :)

[PATCH 1/1] nvme: Modify the debug level for setting shutdown timeout

[Martin K. Petersen]

Max,

> but shutdown_timeout is a module param that one can set to be 10 secs
> (for example) and then it might be == transition_time and we won't see
> the print. Then is it not a buggy device anymore ? :)

The intent was to only print when the resulting shutdown_timeout
deviated from the default value (whether the driver default or the
admin-provided module parameter).

My only concern with info over warning is that info is often dropped.
And therefore you have a harder time finding out which value was in
effect when you subsequently peak at the logs.

-- 
Martin K. Petersen	Oracle Linux Engineering

[PATCH 1/1] nvme: Modify the debug level for setting shutdown timeout

[Max Gurtovoy]

Hi Martin,


On 1/4/2018 7:24 PM, Martin K. Petersen wrote:
> 
> Max,
> 
>> but shutdown_timeout is a module param that one can set to be 10 secs
>> (for example) and then it might be == transition_time and we won't see
>> the print. Then is it not a buggy device anymore ? :)
> 
> The intent was to only print when the resulting shutdown_timeout
> deviated from the default value (whether the driver default or the
> admin-provided module parameter).
> 
> My only concern with info over warning is that info is often dropped.
> And therefore you have a harder time finding out which value was in
> effect when you subsequently peak at the logs.
> 

Ok, it just bothered me to see this print every time I probed the device.
But I can live with that. You can ignore this patch.

-Max.