Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <cover.1685350577.git.chunguang.xu@shopee.com> <bd1676d2-4078-4c02-14a9-e2f5ebe2dd29@grimberg.me>
In-Reply-To: <bd1676d2-4078-4c02-14a9-e2f5ebe2dd29@grimberg.me>
From:   =?UTF-8?B?6K645pil5YWJ?= <brookxu.cn@gmail.com>
Date:   Tue, 6 Jun 2023 22:41:42 +0800
Message-ID: <CADtkEefBGmNeO_zvXcbfJTP5TpEvowtAwx4=f_5tO=W56krW-w@mail.gmail.com>
Subject: Re: [RFC PATCH 0/4] nvme-tcp: fix hung issues for deleting
To:     Sagi Grimberg <sagi@grimberg.me>
Cc:     kbusch@kernel.org, axboe@kernel.dk, hch@lst.de,
        linux-nvme@lists.infradead.org, linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Sagi Grimberg <sagi@grimberg.me> =E4=BA=8E2023=E5=B9=B46=E6=9C=886=E6=97=A5=
=E5=91=A8=E4=BA=8C 07:09=E5=86=99=E9=81=93=EF=BC=9A
>
>
> > From: Chunguang Xu <chunguang.xu@shopee.com>
> >
> > We found that nvme_remove_namespaces() may hang in flush_work(&ctrl->sc=
an_work)
> > while removing ctrl. The root cause may due to the state of ctrl change=
d to
> > NVME_CTRL_DELETING while removing ctrl , which intterupt nvme_tcp_error=
_recovery_work()/
> > nvme_reset_ctrl_work()/nvme_tcp_reconnect_or_remove().  At this time, c=
trl is
> > freezed and queue is quiescing . Since scan_work may continue to issue =
IOs to
> > load partition table, make it blocked, and lead to nvme_tcp_error_recov=
ery_work()
> > hang in flush_work(&ctrl->scan_work).
> >
> > After analyzation, we found that there are mainly two case:
> > 1. Since ctrl is freeze, scan_work hang in __bio_queue_enter() while it=
 issue
> >     new IO to load partition table.
> > 2. Since queus is quiescing, requeue timeouted IO may hang in hctx->dis=
patch
> >     queue, leading scan_work waiting for IO completion.
>
> Hey, can you please look at the discussion with Mings' proposal in
> "nvme: add nvme_delete_dead_ctrl for avoiding io deadlock" ?

Hi grimberg:

I have read Ming's patch, it seems that MIng fixed the case my patchset mis=
sed,
Ming  fixes the hang occur while reconnect fails(reach max retries), my pat=
chset
fixes the issue that while driver processing error_recover or reconnect(hav=
e not
reach max retries), user actively remove ctrl(nvme disconnect),  this
will interrupt
error_recovery or reconnect, but ctrl is  freezed and request queue is
quiescing,
so the new IO or timeouted IOs(requeue to hctx->dispatch_list) cannot conti=
nue
to process, as a result, nvme_remove_namespaces hang on flush scan_work or
blk_mq_freeze_queue_wait, new IO hang on __bio_queue_enter(), it seems that
if the first patch of my patchset add the next code, it may cover Ming's ca=
se:

static void nvme_tcp_reconnect_or_remove(struct nvme_ctrl *ctrl)
{
    /* If we are resetting/deleting then do nothing */
    if (ctrl->state !=3D NVME_CTRL_CONNECTING) {
        WARN_ON_ONCE(ctrl->state =3D=3D NVME_CTRL_NEW ||
        ctrl->state =3D=3D NVME_CTRL_LIVE);
        return;
    }

    if (nvmf_should_reconnect(ctrl)) {
        dev_info(ctrl->device, "Reconnecting in %d seconds...\n",
            ctrl->opts->reconnect_delay);
        queue_delayed_work(nvme_wq, &to_tcp_ctrl(ctrl)->connect_work,
            ctrl->opts->reconnect_delay * HZ);
    } else {
        dev_info(ctrl->device, "Removing controller...\n");
+      nvme_ctrl_reconnect_exit(ctrl);
        nvme_delete_ctrl(ctrl);
    }
}

Thanks.

> Looks the same to me.