On Wed, Jun 7, 2023 at 6:14 PM Roberts, Martin <martin.roberts@xxxxxxxxx> wrote:
>
> Re: virtio-blk: support completion batching for the IRQ path · torvalds/linux@07b679f · GitHub
>
> Signed-off-by: Suwan Kim suwan.kim027@xxxxxxxxx
>
> Signed-off-by: Michael S. Tsirkin mst@xxxxxxxxxx
>
>
>
>
>
> This change appears to have broken things…
>
> We now see applications hanging during disk accesses.
>
> e.g.
>
> multi-port virtio-blk device running in h/w (FPGA)
>
> Host running a simple ‘fio‘ test.
>
> [global]
>
> thread=1
>
> direct=1
>
> ioengine=libaio
>
> norandommap=1
>
> group_reporting=1
>
> bs=4K
>
> rw=read
>
> iodepth=128
>
> runtime=1
>
> numjobs=4
>
> time_based
>
> [job0]
>
> filename=/dev/vda
>
> [job1]
>
> filename=/dev/vdb
>
> [job2]
>
> filename=/dev/vdc
>
> ...
>
> [job15]
>
> filename=/dev/vdp
>
>
>
> i.e. 16 disks; 4 queues per disk; simple burst of 4KB reads
>
> This is repeatedly run in a loop.
>
>
>
> After a few, normally <10 seconds, fio hangs.
>
> With 64 queues (16 disks), failure occurs within a few seconds; with 8 queues (2 disks) it may take ~hour before hanging.
>
> Last message:
>
> fio-3.19
>
> Starting 8 threads
>
> Jobs: 1 (f=1): [_(7),R(1)][68.3%][eta 03h:11m:06s]
>
> I think this means at the end of the run 1 queue was left incomplete.
>
>
>
> ‘diskstats’ (run while fio is hung) shows no outstanding transactions.
>
> e.g.
>
> $ cat /proc/diskstats
>
> ...
>
> 252 0 vda 1843140071 0 14745120568 712568645 0 0 0 0 0 3117947 712568645 0 0 0 0 0 0
>
> 252 16 vdb 1816291511 0 14530332088 704905623 0 0 0 0 0 3117711 704905623 0 0 0 0 0 0
>
> ...
>
>
>
> Other stats (in the h/w, and added to the virtio-blk driver ([a]virtio_queue_rq(), [b]virtblk_handle_req(), [c]virtblk_request_done()) all agree, and show every request had a completion, and that virtblk_request_done() never gets called.
>
> e.g.
>
> PF= 0 vq=0 1 2 3
>
> [a]request_count - 839416590 813148916 105586179 84988123
>
> [b]completion1_count - 839416590 813148916 105586179 84988123
>
> [c]completion2_count - 0 0 0 0
>
>
>
> PF= 1 vq=0 1 2 3
>
> [a]request_count - 823335887 812516140 104582672 75856549
>
> [b]completion1_count - 823335887 812516140 104582672 75856549
>
> [c]completion2_count - 0 0 0 0
>
>
>
> i.e. the issue is after the virtio-blk driver.
>
>
>
>
>
> This change was introduced in kernel 6.3.0.
>
> I am seeing this using 6.3.3.
>
> If I run with an earlier kernel (5.15), it does not occur.
>
> If I make a simple patch to the 6.3.3 virtio-blk driver, to skip the blk_mq_add_to_batch()call, it does not fail.
>
> e.g.
>
> kernel 5.15 – this is OK
>
> virtio_blk.c,virtblk_done() [irq handler]
>
> if (likely(!blk_should_fake_timeout(req->q))) {
>
> blk_mq_complete_request(req);
>
> }
>
>
>
> kernel 6.3.3 – this fails
>
> virtio_blk.c,virtblk_handle_req() [irq handler]
>
> if (likely(!blk_should_fake_timeout(req->q))) {
>
> if (!blk_mq_complete_request_remote(req)) {
>
> if (!blk_mq_add_to_batch(req, iob, virtblk_vbr_status(vbr), virtblk_complete_batch)) {
>
> virtblk_request_done(req); //this never gets called... so blk_mq_add_to_batch() must always succeed
>
> }
>
> }
>
> }
>
>
>
> If I do, kernel 6.3.3 – this is OK
>
> virtio_blk.c,virtblk_handle_req() [irq handler]
>
> if (likely(!blk_should_fake_timeout(req->q))) {
>
> if (!blk_mq_complete_request_remote(req)) {
>
> virtblk_request_done(req); //force this here...
>
> if (!blk_mq_add_to_batch(req, iob, virtblk_vbr_status(vbr), virtblk_complete_batch)) {
>
> virtblk_request_done(req); //this never gets called... so blk_mq_add_to_batch() must always succeed
>
> }
>
> }
>
> }
>
>
>
>
>
> Perhaps you might like to fix/test/revert this change…
>
> Martin
>
>
Hi Martin,
There are many changes between 6.3.0 and 6.3.3.
Could you try to find a commit which triggers the io hang?
Is it ok with 6.3.0 kernel or with reverting
"virtio-blk: support completion batching for the IRQ path" commit?
We need to confirm which commit is causing the error.
Regards,
Suwan Kim
