From 4a401cce0cf30fd6cd0e71a419e8b2087619cf54 Mon Sep 17 00:00:00 2001 From: Daniel Verkamp Date: Tue, 20 Oct 2020 00:07:49 -0700 Subject: [PATCH] devices: virtio: block: disable EVENT_IDX feature This disables the VIRTIO_RING_F_EVENT_IDX optimization, which seems to be causing I/O to sometimes not complete when used with the Linux virtio block guest driver. The exact root cause is not clear (possibly a race/memory ordering issue when reading and updating the event index), but disabling the feature flag is enough to get back to a reliable state while the failure is debugged. The particular test case that triggers this typically reproduces as a hung task in btrfs waiting for an I/O completion that never shows up. With the EVENT_IDX feature disabled, I am not able to reproduce the hang after many hours of running the test case. BUG=chromium:1097385 TEST=Run reproducer test case; do not observe any hung tasks Change-Id: Iaf37722f23c05276ec65f4d4a10c4a47af764538 Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/2486671 Reviewed-by: Dylan Reid Tested-by: kokoro Commit-Queue: Daniel Verkamp --- devices/src/virtio/block.rs | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/devices/src/virtio/block.rs b/devices/src/virtio/block.rs index 85f960b018..271d5f70c5 100644 --- a/devices/src/virtio/block.rs +++ b/devices/src/virtio/block.rs @@ -20,7 +20,6 @@ use data_model::{DataInit, Le16, Le32, Le64}; use disk::DiskFile; use msg_socket::{MsgReceiver, MsgSender}; use sync::Mutex; -use virtio_sys::virtio_ring::VIRTIO_RING_F_EVENT_IDX; use vm_control::{DiskControlCommand, DiskControlResponseSocket, DiskControlResult}; use vm_memory::GuestMemory; @@ -529,7 +528,6 @@ impl Block { let mut avail_features: u64 = base_features; avail_features |= 1 << VIRTIO_BLK_F_FLUSH; - avail_features |= 1 << VIRTIO_RING_F_EVENT_IDX; if read_only { avail_features |= 1 << VIRTIO_BLK_F_RO; } else { @@ -882,8 +880,8 @@ mod tests { let b = Block::new(features, Box::new(f), false, true, 512, None).unwrap(); // writable device should set VIRTIO_BLK_F_FLUSH + VIRTIO_BLK_F_DISCARD // + VIRTIO_BLK_F_WRITE_ZEROES + VIRTIO_F_VERSION_1 + VIRTIO_BLK_F_BLK_SIZE - // + VIRTIO_BLK_F_SEG_MAX + VIRTIO_RING_F_EVENT_IDX - assert_eq!(0x120006244, b.features()); + // + VIRTIO_BLK_F_SEG_MAX + assert_eq!(0x100006244, b.features()); } // read-write block device, non-sparse @@ -893,8 +891,8 @@ mod tests { let b = Block::new(features, Box::new(f), false, false, 512, None).unwrap(); // writable device should set VIRTIO_BLK_F_FLUSH // + VIRTIO_BLK_F_WRITE_ZEROES + VIRTIO_F_VERSION_1 + VIRTIO_BLK_F_BLK_SIZE - // + VIRTIO_BLK_F_SEG_MAX + VIRTIO_RING_F_EVENT_IDX - assert_eq!(0x120004244, b.features()); + // + VIRTIO_BLK_F_SEG_MAX + assert_eq!(0x100004244, b.features()); } // read-only block device @@ -904,8 +902,7 @@ mod tests { let b = Block::new(features, Box::new(f), true, true, 512, None).unwrap(); // read-only device should set VIRTIO_BLK_F_FLUSH and VIRTIO_BLK_F_RO // + VIRTIO_F_VERSION_1 + VIRTIO_BLK_F_BLK_SIZE + VIRTIO_BLK_F_SEG_MAX - // + VIRTIO_RING_F_EVENT_IDX - assert_eq!(0x120000264, b.features()); + assert_eq!(0x100000264, b.features()); } }