[PATCH-RFC 2/5] nvme: tcp: split controller bringup handling

[Caleb Sander]

On Mon, Jun 3, 2024 at 4:08 PM Keith Busch <kbusch at meta.com> wrote:
>
> From: Keith Busch <kbusch at kernel.org>
>
> Drivers must call nvme_uninit_ctrl after a successful nvme_init_ctrl.
> Split the allocation side out to make the error handling boundary easier
> to navigate. The nvme tcp driver's error handling had different returns
> in the error goto labels, which harm readability.
>
> Signed-off-by: Keith Busch <kbusch at kernel.org>
> ---
>  drivers/nvme/host/tcp.c | 25 +++++++++++++++++++------
>  1 file changed, 19 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c
> index 8b5e4327fe83b..7f75d01dcbed0 100644
> --- a/drivers/nvme/host/tcp.c
> +++ b/drivers/nvme/host/tcp.c
> @@ -2686,7 +2686,7 @@ nvme_tcp_existing_controller(struct nvmf_ctrl_options *opts)
>         return found;
>  }
>
> -static struct nvme_ctrl *nvme_tcp_create_ctrl(struct device *dev,
> +static struct nvme_tcp_ctrl *nvme_tcp_alloc_ctrl(struct device *dev,
>                 struct nvmf_ctrl_options *opts)
>  {
>         struct nvme_tcp_ctrl *ctrl;
> @@ -2761,6 +2761,24 @@ static struct nvme_ctrl *nvme_tcp_create_ctrl(struct device *dev,
>         if (ret)
>                 goto out_kfree_queues;
>
> +       return ctrl;
> +out_kfree_queues:
> +       kfree(ctrl->queues);
> +out_free_ctrl:
> +       kfree(ctrl);
> +       return ERR_PTR(ret);
> +}
> +
> +static struct nvme_ctrl *nvme_tcp_create_ctrl(struct device *dev,
> +               struct nvmf_ctrl_options *opts)
> +{
> +       struct nvme_tcp_ctrl *ctrl;
> +       int ret;
> +
> +       ctrl = nvme_tcp_alloc_ctrl(dev, opts);
> +       if (IS_ERR(ctrl))
> +               return (struct nvme_ctrl *)ctrl;

This cast doesn't look right, as struct nvme_tcp_ctrl doesn't start
with a struct nvme_ctrl field. Looks like this should be &ctrl->ctrl
to match the current implementation of nvme_tcp_create_ctrl()?

> +
>         if (!nvme_change_ctrl_state(&ctrl->ctrl, NVME_CTRL_CONNECTING)) {
>                 WARN_ON_ONCE(1);
>                 ret = -EINTR;
> @@ -2786,11 +2804,6 @@ static struct nvme_ctrl *nvme_tcp_create_ctrl(struct device *dev,
>         if (ret > 0)
>                 ret = -EIO;
>         return ERR_PTR(ret);
> -out_kfree_queues:
> -       kfree(ctrl->queues);
> -out_free_ctrl:
> -       kfree(ctrl);
> -       return ERR_PTR(ret);
>  }
>
>  static struct nvmf_transport_ops nvme_tcp_transport = {
> --
> 2.43.0
>
>