Re: [PATCH 2/3] nvme: sanitize KATO setting

[Christoph Hellwig <hch@lst.de>]

On Wed, Mar 03, 2021 at 04:53:58PM +0800, Chao Leng wrote:
>
>
> On 2021/3/2 17:26, Hannes Reinecke wrote:
>> According to the NVMe base spec the KATO commands should be sent
>> at half of the KATO interval, to properly account for round-trip
>> times.
>> As we now will only ever send one KATO command per connection we
>> can easily use the recommended values.
>> This also fixes a potential issue where the request timeout for
>> the KATO command does not match the value in the connect command,
>> which might be causing spurious connection drops from the target.
>>
>> Signed-off-by: Hannes Reinecke <hare@suse.de>
>> ---
>>   drivers/nvme/host/core.c    | 9 ++++++---
>>   drivers/nvme/host/fabrics.c | 2 +-
>>   drivers/nvme/host/nvme.h    | 9 ++++++++-
>>   3 files changed, 15 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
>> index f890b310499e..6d096d41a82f 100644
>> --- a/drivers/nvme/host/core.c
>> +++ b/drivers/nvme/host/core.c
>> @@ -1223,7 +1223,8 @@ static void nvme_keep_alive_end_io(struct request *rq, blk_status_t status)
>>   		startka = true;
>>   	spin_unlock_irqrestore(&ctrl->lock, flags);
>>   	if (startka)
>> -		queue_delayed_work(nvme_wq, &ctrl->ka_work, ctrl->kato * HZ);
>> +		queue_delayed_work(nvme_wq, &ctrl->ka_work,
>> +				   NVME_KATO_DELAY(ctrl->kato) * HZ);
> * HZ and then >> 1 is a better choice. do like this: NVME_KATO_DELAY(ctrl->kato * HZ).

Also there is no reason to not include the "* HZ" in NVME_KATO_DELAY,
as all callers do it anyway.  Also please turn NVME_KATO_DELAY into a
normal inline function using normal lower case naming.  Alternatively
we could also just do a helper for the whole queue_delayed_work, which
might actually end up even cleaner.

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