Re: [PATCH 2/2] nvme: make keep-alive synchronous operation

[Christoph Hellwig <hch@lst.de>]

On Fri, Oct 04, 2024 at 05:16:57PM +0530, Nilay Shroff wrote:
> The nvme keep-alive operation, which executes at a periodic interval,
> could potentially sneak in while shutting down a fabric controller.
> This may lead to a race between the fabric controller admin queue
> destroy code path (while shutting down controller) and the blk-mq
> hw/hctx queuing from the keep-alive thread.
> 
> This fix helps avoid race by implementing keep-alive as a synchronous
> operation so that admin queue-usage ref counter is decremented only
> after keep-alive command finish execution and returns its status.

With that you mean ->q_usage_counter?

Moving to synchronous submission and wasting a workqueue context for
that is a bit sad.  I think just removing the blk_mq_free_request call
from nvme_keep_alive_finish and returning RQ_END_IO_FREE instead
should have the same effect, or am I missing something?

> Also, while we are at it, instead of first acquiring ctrl lock and then
> accessing NVMe controller state, lets use the helper function
> nvme_ctrl_state() in nvme_keep_alive_end_io() and get rid of the
> lock.

Please split that into a separate patch.