[PATCH] nvme-tcp: add additional info for nvme_tcp_timeout log

[Engel, Amit]

Yes, io timeout is obtainable via sysfs.
But this io_timeout param is writable and the user can modify this value at any time.
So I still find it usufl to have this value as part of this log.

Regarding the cntlid, I agree - we can remove it.

Thanks
Amit

-----Original Message-----
From: Sagi Grimberg <sagi at grimberg.me> 
Sent: Monday, 5 December 2022 11:59
To: Engel, Amit <Amit.Engel at Dell.com>; linux-nvme at lists.infradead.org
Cc: Grupi, Elad <Elad.Grupi at dell.com>
Subject: Re: [PATCH] nvme-tcp: add additional info for nvme_tcp_timeout log


[EXTERNAL EMAIL] 


> Hi Sagi, thank you for your inputs
> 
> It seems to me that logging nvme command id is sufficient and there is 
> no need to log also tag and genctr separately This is due to the fact that command id is constructed as such:
> 
> | xxxx | xxxxxxxxxxxx |
>    gen    request tag
> 
> What do you think?

Yes, that is fine I guess.

> Regarding the io timeout value, I got your point..
> logging the io timeout threshold (which is the delta from allocation 
> to submission) is sufficient
> 
> regarding nvme-tcp trace points -  I don’t believe that would help.
> This timeout log is on the host side, and we can't ensure that a 
> customer that is running nvme host will enable/use trace points In addition, the whole point of this specific log is for debug, I guess - right?

To some extent.

> 
> How does this log look?
> kernel: nvme nvme0: queue 4: cntlid 1 type 4 cid 4163 opcode 0x2 io 
> timeout threshold 30000ms (Including the cntlid, cid (which includes 
> genctr+tag), opc, and timeout threshold value in ms)

Both the cntlid and the io timeout are obtainable via sysfs, why add these to the log? The cntlid is often just an ephemeral value assigned by the controller.

opcode is fine to add.