[PATCH] nvmet: fix false keep-alive timeout when a controller is torn down

[Yi Zhang]

Verified the issue on the latest linux-block/for-next

Tested-by: Yi Zhang <yi.zhang at redhat.com>

On Wed, May 26, 2021 at 12:24 AM Sagi Grimberg <sagi at grimberg.me> wrote:
>
> Controller teardown flow may take some time in case it has many I/O
> queues, and the host may not send us keep-alive during this period.
> Hence reset the traffic based keep-alive timer so we don't trigger
> a controller teardown as a result of a keep-alive expiration.
>
> Reported-by: Yi Zhang <yi.zhang at redhat.com>
> Signed-off-by: Sagi Grimberg <sagi at grimberg.me>
> ---
>  drivers/nvme/target/core.c  | 16 ++++++++++++----
>  drivers/nvme/target/nvmet.h |  2 +-
>  2 files changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/nvme/target/core.c b/drivers/nvme/target/core.c
> index 1853db38b682..e991b4671aeb 100644
> --- a/drivers/nvme/target/core.c
> +++ b/drivers/nvme/target/core.c
> @@ -388,10 +388,10 @@ static void nvmet_keep_alive_timer(struct work_struct *work)
>  {
>         struct nvmet_ctrl *ctrl = container_of(to_delayed_work(work),
>                         struct nvmet_ctrl, ka_work);
> -       bool cmd_seen = ctrl->cmd_seen;
> +       bool reset_tbkas = ctrl->reset_tbkas;
>
> -       ctrl->cmd_seen = false;
> -       if (cmd_seen) {
> +       ctrl->reset_tbkas = false;
> +       if (reset_tbkas) {
>                 pr_debug("ctrl %d reschedule traffic based keep-alive timer\n",
>                         ctrl->cntlid);
>                 schedule_delayed_work(&ctrl->ka_work, ctrl->kato * HZ);
> @@ -804,6 +804,14 @@ void nvmet_sq_destroy(struct nvmet_sq *sq)
>         percpu_ref_exit(&sq->ref);
>
>         if (ctrl) {
> +               /*
> +                * teardown flow may take some time, and the host
> +                * may not send us keep-alive during this period,
> +                * hence reset the traffic based keep-alive timer
> +                * so we don't trigger a controller teardown as
> +                * a result of a keep-alive expiration.
> +                */
> +               ctrl->reset_tbkas = true;
>                 nvmet_ctrl_put(ctrl);
>                 sq->ctrl = NULL; /* allows reusing the queue later */
>         }
> @@ -952,7 +960,7 @@ bool nvmet_req_init(struct nvmet_req *req, struct nvmet_cq *cq,
>         }
>
>         if (sq->ctrl)
> -               sq->ctrl->cmd_seen = true;
> +               sq->ctrl->reset_tbkas = true;
>
>         return true;
>
> diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h
> index d69a409515d6..53aea9a8056e 100644
> --- a/drivers/nvme/target/nvmet.h
> +++ b/drivers/nvme/target/nvmet.h
> @@ -167,7 +167,7 @@ struct nvmet_ctrl {
>         struct nvmet_subsys     *subsys;
>         struct nvmet_sq         **sqs;
>
> -       bool                    cmd_seen;
> +       bool                    reset_tbkas;
>
>         struct mutex            lock;
>         u64                     cap;
> --
> 2.27.0
>
>
> _______________________________________________
> Linux-nvme mailing list
> Linux-nvme at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-nvme
>


-- 
Best Regards,
  Yi Zhang