On 5/13/20 11:33 PM, Xiaoguang Wang wrote: > hi, > >> On 5/13/20 12:45 PM, Jens Axboe wrote: >>> Hi Xiaoguang, >>> >>> Was doing some other testing today, and noticed a hang with fixed files. >>> I did a bit of poor mans bisecting, and came up with this one: >>> >>> commit 0558955373023b08f638c9ede36741b0e4200f58 >>> Author: Xiaoguang Wang <xiaoguang.wang@xxxxxxxxxxxxxxxxx> >>> Date: Tue Mar 31 14:05:18 2020 +0800 >>> >>> io_uring: refactor file register/unregister/update handling >>> >>> If I revert this one, the test completes fine. >>> >>> The case case is pretty simple, just run t/io_uring from the fio >>> repo, default settings: >>> >>> [ fio] # t/io_uring /dev/nvme0n1p2 >>> Added file /dev/nvme0n1p2 >>> sq_ring ptr = 0x0x7fe1cb81f000 >>> sqes ptr = 0x0x7fe1cb81d000 >>> cq_ring ptr = 0x0x7fe1cb81b000 >>> polled=1, fixedbufs=1, buffered=0 QD=128, sq_ring=128, cq_ring=256 >>> submitter=345 >>> IOPS=240096, IOS/call=32/31, inflight=91 (91) >>> IOPS=249696, IOS/call=32/31, inflight=99 (99) >>> ^CExiting on signal 2 >>> >>> and ctrl-c it after a second or so. You'll then notice a kworker that >>> is stuck in io_sqe_files_unregister(), here: >>> >>> /* wait for all refs nodes to complete */ >>> wait_for_completion(&data->done); >>> >>> I'll try and debug this a bit, and for some reason it doens't trigger >>> with the liburing fixed file setup. Just wanted to throw this out there, >>> so if you have cycles, please do take a look at it. >> >> https://lore.kernel.org/io-uring/015659db-626c-5a78-6746-081a45175f45@xxxxxxxxx/T/#u > Thanks for this fix, and sorry, it's my bad, I didn't cover this case > when sending patches. Can you share your test cases or test method > when developing io_uring? Usually I just run test cases under > liburing/test, seems it's not enough. It really should be enough, the case that triggered this issue is the combination of fixed files and polled IO. I'll need to add that to eg test/read-write.c, which does a lot of combinations already. The only issue is that polled IO only works on some files / devices. I've been meaning to add a config file to the liburing regression tests, so you can configure a device to use for testing. With an NVMe device in there, we should be able to have full coverage. -- Jens Axboe
