[PATCH 3/5] nvme: Change 'bool shutdown' into an enum shutdown_type.

[Jeremy Allison]

On Mon, Jan 08, 2024 at 07:42:09PM +0200, Sagi Grimberg wrote:
>On 1/4/24 20:44, Jeremy Allison wrote:
>>
>>There needs to be some way to pass a use
>>"two-pass" flag down from nvme_shutdown()
>>into nvme_dev_disable()
>Ideally nvme_dev_disable can be broken in a way that
>nvme_shutdown can have 2-3 helpers and the rest of the
>code can remain intact calling nvme_dev_disable (internally
>calling these helpers).

I think the safest way to do this is to leave
nvme_dev_disable() alone as much as possible,
and just have it call a nvme_shutdown_ctrl_nowait()
function when the bool/enum tells it it's in a
two-pass situation.

I have code that does this, I'm just waiting
for the 'bool or enum' decision.

>>I guess I could add a second bool flag passed
>>down from nvme_shutdown() all the way to
>>nvme_dev_disable(), which then calls the
>>nvme_shutdown_ctrl_nowait() varient instead
>>of nvme_disable_ctrl() if both "bool shutdown"
>>and "bool twopass" are set but that seems kind
>>of ugly and confusing to me.
>
>Probably.. Maybe call it dontwait (bool).
>And yes, it is ugly, but better than moving the
>ugliness to the core.

No arguments from me :-).

>>How about keeping the enum internal to
>>drivers/nvme/host/pci.c
>>and making it:
>>
>>enum shutdown_type {
>>     NVME_DISABLE_RESET,
>>     NVME_DISABLE_SHUTDOWN,
>>     NVME_DISABLE_SHUTDOWN_TWOPASS
>>};
>
>NVME_PCI_

Got it !

>>and changing 'bool shutdown' to enum shutdown_type
>>inside the static functions nvme_disable_prepare_reset()
>>and nvme_dev_disable() only ?
>>
>>That way we don't have to have the confusing
>>double bool:
>
>Neither are great. I'll defer to Keith and Christoph on this one.

OK, I'll use whatever Keith and Christoph agree on.