Re: [PATCH 0/4] nvme: start keep-alive after admin queue setup

[Hannes Reinecke <hare@suse.de>]

On 10/24/23 01:16, Keith Busch wrote:
> On Mon, Oct 23, 2023 at 11:54:37AM +0200, Hannes Reinecke wrote:
>> But we start keep alive unconditionally nowadays; each and every
>> host driver (including PCI) is calling into nvme_start_ctrl(),
>> which calls nvme_start_keep_alive() without any checks.
> 
> I see nvme_start_keep_alive() is always called, but that checks
> 'ctrl->kato' which will always be 0 for PCI.
>   
But it also means we don't have to check for fabrics here...

>> Be it as it may, tying it into the state transition won't work
>> as the 'CONNECTING' state is entered _before_ any queues are setup,
>> so starting keep-alive here would be too early.
> 
> That's just IO eueues, right? Keep-alive is an admin command, and the
> admin queue should be up in the CONNECTING state.

Nope. 'loop' creates the admin queue after setting the status to 
'CONNECTING'. So there is no clear rule when 'CONNECTING' is entered.

There is a valid point to be made to enter 'CONNECTING' only when the
admin queue is setup, but currently that's not the case.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                Kernel Storage Architect
hare@suse.de                              +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Ivo Totev, Andrew
Myers, Andrew McDonald, Martje Boudien Moerman