[RFC PATCH] nvme-fc: move tagset removal to nvme_fc_delete_ctrl()

[Ewan Milne]

So testing looks like it is showing that the WARN_ON() I referenced
earlier goes away
after reverting all the recent keepalive changes, all the way back to
and including:

a54a93d0e359 nvme: move stopping keep-alive into nvme_uninit_ctrl()

But I think this is just because this makes the keeaplive stop early enough.

I think the problem is that the call do nvme_stop_keep_alive() does
not wait unitil
the keepalive command, if in progress, completes.  It just makes sure that
nvme_keep_alive_work() is not queued or running.  But the keepalive command
is asynchronous, invoked via blk_execute_rq_nowait().  (I had thought that the
ref taken on the nvme_fc_ctrl while the command was in progress would have
covered this though.)

-Ewan

On Wed, Jan 15, 2025 at 8:57 PM Ming Lei <ming.lei at redhat.com> wrote:
>
> Hi Ewan,
>
> On Wed, Jan 15, 2025 at 12:33:50PM -0500, Ewan Milne wrote:
> > Moving the tagset removal to the end of nvme_fc_delete_ctrl() ran OK and we
> > no longer see the WARN_ON() seen earlier, however I think that there is a
> > potential problem with doing this because the keepalive is not stopped until
> > nvme_uninit_ctrl() is called.  On our systems w/FC kato is normally disabled
> > so we wouldn't see any problem but this isn't true in the general case.
>
> Please see e9869c85c811 ("nvme-fabrics: fix kernel crash while shutting down controller")
> which stops the keepalive request at the entry of
> nvme_remove_admin_tag_set().
>
> So your above concern shouldn't exist.
>
> Thanks,
> Ming
>