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

From: 许春光
Date: Tue Apr 30 2024 - 01:10:35 EST


Chaitanya Kulkarni <chaitanyak@xxxxxxxxxx> 于2024年4月30日周二 11:47写道:
>
> On 4/29/2024 7:17 PM, brookxu.cn wrote:
> > From: Chunguang Xu <chunguang.xu@xxxxxxxxxx>
> >
> > 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@xxxxxxxxxx>
> > ---
>
> -ck
>
>