Messages in this thread | ![/](/images/icornerl.gif) | | From | Chaitanya Kulkarni <> | Subject | Re: [PATCH] nvme-fabrics: use reserved tag for reg read/write command | Date | Tue, 30 Apr 2024 06:59:51 +0000 |
| |
On 4/29/2024 10:10 PM, 许春光 wrote: > 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. >>
cool, can you please submit a blktest [1] for this ? I'm not sure if we have any coverage for this scenario ...
-ck
[1] https://github.com/osandov/blktests
| ![\](/images/icornerr.gif) |