Re: [PATCH] nvme-fabrics: use reserved tag for reg read/write command

[许春光 <brookxu.cn@gmail.com>]

Chaitanya Kulkarni <chaitanyak@nvidia.com> 于2024年4月30日周二 11:47写道：
>
> On 4/29/2024 7:17 PM, brookxu.cn wrote:
> > From: Chunguang Xu <chunguang.xu@shopee.com>
> >
> > In some scenarios, if too many commands are issued by nvme command in
> > the same time by user tasks, this may exhaust all tags of admin_q. If
> > a reset (nvme reset or IO timeout) occurs before these commands finish,
> > reconnect routine may fail to update nvme regs due to insufficient tags,
> > which will cause kernel hang forever. In order to workaround this issue,
> > maybe we can let reg_read32()/reg_read64()/reg_write32() use reserved
> > tags. This maybe safe for nvmf:
> >
> > 1. For the disable ctrl path,  we will not issue connect command
> > 2. For the enable ctrl / fw activate path, since connect and reg_xx()
> >     are called serially.
> >
>
> Given the complexity of the scenario described above, is it possible to
> write a script for this scenario that will trigger this and submit to
> blktest ? not that this is a blocker to get this patch reviewed, but
> believe it is needed in long run, WDYT ?

Thanks for you reply, I can easily reproduce it in my VMs by following steps:
STEP 1. In order to reduce the complexity of reproduction, I reduce
NVME_AQ_MQ_TAG_DEPTH from 31 to 8

STEP 2. Create a nvme device by NVMe over tcp, such as following command:
nvme connect -t tcp -a 192.168.122.20 -s 4420 -n
nqn.2014-08.org.nvmexpress.mytest

STEP 3. Buind and run the c++ program issues nvme commands as followed:
#include <sys/types.h>
#include <signal.h>
#include <unistd.h>
#include <vector>
#include <set>
#include <stdlib.h>
#include <sys/types.h>
#include <sys/wait.h>

const int concurrency = 64;
std::set<pid_t> chlds;

int __exit = 0;
void  sigint_proc(int signo)
{
        __exit = 1;
}

int main(int argc, char **argv)
{
        signal(SIGINT, sigint_proc);

        for (auto i  = 0; i < concurrency; i++) {
                auto pid = fork();
                if (!pid) {
                        while (true) {
                                system("nvme list -o json 2>&1 > /dev/null");
                        }
                }

                chlds.insert(pid);
        }

        while (!__exit) {
                if (chlds.empty())
                        break;

                for (auto pid : chlds) {
                        int wstatus, ret;
                        ret = waitpid(pid, &wstatus, WNOWAIT);
                        if (ret > 0) {
                                chlds.erase(pid);
                                break;
                        }
                }
                usleep(1000);
        }

        // exit
        for (auto pid : chlds)
                kill(pid, SIGKILL);

        return 0;
}

STEP 4. Open a new console, running the followed command:
while [ true ]; do nvme reset /dev/nvme0; sleep `echo "$RANDOM%1" | bc`; done

We will reproduce this issue soon.
>
> > So the reserved tags may still be enough while reg_xx() use reserved tags.
> >
> > Signed-off-by: Chunguang Xu <chunguang.xu@shopee.com>
> > ---
>
> -ck
>
>