[PATCH 3/5] nvme: Change 'bool shutdown' into an enum shutdown_type.
Sagi Grimberg
sagi at grimberg.me
Thu Jan 4 05:26:11 PST 2024
On 1/3/24 23:04, Jeremy Allison wrote:
> Convert nvme_disable_ctrl() and nvme_dev_disable()
> inside drivers/nvme/host/pci.c to use this:
>
> bool shutdown = false == NVME_DISABLE_RESET
> bool shutdown = true == NVME_DISABLE_SHUTDOWN_SYNC.
>
> This will make it easier to add a third request later:
> NVME_DISABLE_SHUTDOWN_ASNYC
>
> As nvme_disable_ctrl() is used outside of drivers/nvme/host/pci.c,
> convert the callers of nvme_disable_ctrl() to this convention too.
>
> Signed-off-by: Jeremy Allison <jallison at ciq.com>
> ---
> drivers/nvme/host/apple.c | 4 ++--
> drivers/nvme/host/core.c | 6 +++---
> drivers/nvme/host/nvme.h | 7 +++++-
> drivers/nvme/host/pci.c | 44 +++++++++++++++++++-------------------
> drivers/nvme/host/rdma.c | 3 ++-
> drivers/nvme/host/tcp.c | 3 ++-
> drivers/nvme/target/loop.c | 2 +-
> 7 files changed, 38 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/nvme/host/apple.c b/drivers/nvme/host/apple.c
> index 596bb11eeba5..764639ede41d 100644
> --- a/drivers/nvme/host/apple.c
> +++ b/drivers/nvme/host/apple.c
> @@ -844,8 +844,8 @@ static void apple_nvme_disable(struct apple_nvme *anv, bool shutdown)
> * NVMe disabled state, after a clean shutdown).
> */
> if (shutdown)
> - nvme_disable_ctrl(&anv->ctrl, shutdown);
> - nvme_disable_ctrl(&anv->ctrl, false);
> + nvme_disable_ctrl(&anv->ctrl, NVME_DISABLE_SHUTDOWN_SYNC);
> + nvme_disable_ctrl(&anv->ctrl, NVME_DISABLE_RESET);
> }
>
> WRITE_ONCE(anv->ioq.enabled, false);
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index 50818dbcfa1a..e1b2facb7d6a 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -2219,12 +2219,12 @@ static int nvme_wait_ready(struct nvme_ctrl *ctrl, u32 mask, u32 val,
> return ret;
> }
>
> -int nvme_disable_ctrl(struct nvme_ctrl *ctrl, bool shutdown)
> +int nvme_disable_ctrl(struct nvme_ctrl *ctrl, enum shutdown_type shutdown_type)
> {
> int ret;
>
> ctrl->ctrl_config &= ~NVME_CC_SHN_MASK;
> - if (shutdown)
> + if (shutdown_type == NVME_DISABLE_SHUTDOWN_SYNC)
> ctrl->ctrl_config |= NVME_CC_SHN_NORMAL;
> else
> ctrl->ctrl_config &= ~NVME_CC_ENABLE;
> @@ -2233,7 +2233,7 @@ int nvme_disable_ctrl(struct nvme_ctrl *ctrl, bool shutdown)
> if (ret)
> return ret;
>
> - if (shutdown) {
> + if (shutdown_type == NVME_DISABLE_SHUTDOWN_SYNC) {
> return nvme_wait_ready(ctrl, NVME_CSTS_SHST_MASK,
> NVME_CSTS_SHST_CMPLT,
> ctrl->shutdown_timeout, "shutdown");
> diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
> index 6092cc361837..1a748640f2fb 100644
> --- a/drivers/nvme/host/nvme.h
> +++ b/drivers/nvme/host/nvme.h
> @@ -187,6 +187,11 @@ enum {
> NVME_MPATH_IO_STATS = (1 << 2),
> };
>
> +enum shutdown_type {
> + NVME_DISABLE_RESET = 0,
> + NVME_DISABLE_SHUTDOWN_SYNC = 1
> +};
I also don't find this a bit awkward especially when another
enumeration value is added later on.
An alternative approach would be to stick with nvme_disable_ctrl that
accepts shutdown bool, and add nvme_disable_ctrl_nowait() wrapper for it
that call-sites can use. Or even better, because we only have a nowait
variant for shutdown, perhaps we can add nvme_shutdown_ctrl_nowait()
wrapper that accepts only the ctrl instead (this will require splitting
nvme_disable_ctrl to the disabling phase and the waiting phase, but that
is hidden in the core as static functions).
More information about the Linux-nvme
mailing list