• Michael S. Tsirkin's avatar
    Revert "virtio-blk: support completion batching for the IRQ path" · afd384f0
    Michael S. Tsirkin authored
    This reverts commit 07b679f7
    
    .
    
    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
    Reported-by: default avatarkernel test robot <lkp@intel.com>
    Closes: https://lore.kernel.org/oe-kbuild-all/202306090826.C1fZmdMe-lkp@intel.com/
    
    
    Cc: Suwan Kim <suwan.kim027@gmail.com>
    Tested-by: edliaw@google.com
    Reported-by: default avatar"Roberts, Martin" <martin.roberts@intel.com>
    Message-Id: <336455b4f630f329380a8f53ee8cad3868764d5c.1686295549.git.mst@redhat.com>
    Signed-off-by: default avatarMichael S. Tsirkin <mst@redhat.com>
    afd384f0
virtio_blk.c 43.6 KB