Messages in this thread | | | From | 许春光 <> | Date | Tue, 30 Apr 2024 16:59:00 +0800 | Subject | Re: [PATCH] nvme-fabrics: use reserved tag for reg read/write command |
| |
Chaitanya Kulkarni <chaitanyak@nvidia.com> 于2024年4月30日周二 14:59写道: > > 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 ... >
My pleasure, after reviewed, if no more doubt, I will try to do it, thanks
> -ck > > [1] https://github.com/osandov/blktests >
| |