Skip to content

Commit c9b0e64

Browse files
ManciukicShadowCurse
authored andcommitted
fix(vsock): restore previous performance with PCI disabled
We noticed that, compared to main, the vsock device is up to 30% slower when PCI is disabled. This is due to the refactor in 087e185 ("fix(vsock): pass correct index when triggering interrupts"), as we're now sending interrupts as soon as we detect the event, rather than deferring them and sending only one interrupt at the end. While with MSI we need to send multiple interrupts, so there is little difference (still, tests show up to 5% improvement with this change), with legacy IRQ there's only one interrupt line so we end up sending multiple back to back interrupts rather than a single one. This patch reverts to the previous behaviour and uses the newly introduced `trigger_queues` method to deduplicate interrupts in the case of IrqTrigger. Signed-off-by: Riccardo Mancini <[email protected]>
1 parent 8d1f375 commit c9b0e64

File tree

2 files changed

+42
-20
lines changed

2 files changed

+42
-20
lines changed

src/vmm/src/devices/virtio/vsock/device.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,16 @@ where
148148
.map_err(DeviceError::FailedSignalingIrq)
149149
}
150150

151+
/// Signal the guest which queues are ready to be consumed
152+
pub fn signal_used_queues(&self, used_queues: &[u16]) -> Result<(), DeviceError> {
153+
self.device_state
154+
.active_state()
155+
.expect("Device is not initialized")
156+
.interrupt
157+
.trigger_queues(used_queues)
158+
.map_err(DeviceError::FailedSignalingIrq)
159+
}
160+
151161
/// Walk the driver-provided RX queue buffers and attempt to fill them up with any data that we
152162
/// have pending. Return `true` if descriptors have been added to the used ring, and `false`
153163
/// otherwise.

src/vmm/src/devices/virtio/vsock/event_handler.rs

Lines changed: 32 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ use super::VsockBackend;
3434
use super::device::{EVQ_INDEX, RXQ_INDEX, TXQ_INDEX, Vsock};
3535
use crate::devices::virtio::device::VirtioDevice;
3636
use crate::devices::virtio::queue::InvalidAvailIdx;
37+
use crate::devices::virtio::vsock::defs::VSOCK_NUM_QUEUES;
3738
use crate::devices::virtio::vsock::metrics::METRICS;
3839
use crate::logger::IncMetric;
3940

@@ -47,49 +48,50 @@ where
4748
const PROCESS_EVQ: u32 = 3;
4849
const PROCESS_NOTIFY_BACKEND: u32 = 4;
4950

50-
pub fn handle_rxq_event(&mut self, evset: EventSet) {
51+
pub fn handle_rxq_event(&mut self, evset: EventSet) -> Vec<u16> {
52+
let mut used_queues = Vec::new();
5153
if evset != EventSet::IN {
5254
warn!("vsock: rxq unexpected event {:?}", evset);
5355
METRICS.rx_queue_event_fails.inc();
54-
return;
56+
return used_queues;
5557
}
5658

5759
if let Err(err) = self.queue_events[RXQ_INDEX].read() {
5860
error!("Failed to get vsock rx queue event: {:?}", err);
5961
METRICS.rx_queue_event_fails.inc();
6062
} else if self.backend.has_pending_rx() {
6163
if self.process_rx().unwrap() {
62-
self.signal_used_queue(RXQ_INDEX)
63-
.expect("vsock: Could not trigger device interrupt or RX queue");
64+
used_queues.push(RXQ_INDEX.try_into().unwrap());
6465
}
6566
METRICS.rx_queue_event_count.inc();
6667
}
68+
used_queues
6769
}
6870

69-
pub fn handle_txq_event(&mut self, evset: EventSet) {
71+
pub fn handle_txq_event(&mut self, evset: EventSet) -> Vec<u16> {
72+
let mut used_queues = Vec::new();
7073
if evset != EventSet::IN {
7174
warn!("vsock: txq unexpected event {:?}", evset);
7275
METRICS.tx_queue_event_fails.inc();
73-
return;
76+
return used_queues;
7477
}
7578

7679
if let Err(err) = self.queue_events[TXQ_INDEX].read() {
7780
error!("Failed to get vsock tx queue event: {:?}", err);
7881
METRICS.tx_queue_event_fails.inc();
7982
} else {
8083
if self.process_tx().unwrap() {
81-
self.signal_used_queue(TXQ_INDEX)
82-
.expect("vsock: Could not trigger device interrupt or TX queue");
84+
used_queues.push(TXQ_INDEX.try_into().unwrap());
8385
}
8486
METRICS.tx_queue_event_count.inc();
8587
// The backend may have queued up responses to the packets we sent during
8688
// TX queue processing. If that happened, we need to fetch those responses
8789
// and place them into RX buffers.
8890
if self.backend.has_pending_rx() && self.process_rx().unwrap() {
89-
self.signal_used_queue(RXQ_INDEX)
90-
.expect("vsock: Could not trigger device interrupt or RX queue");
91+
used_queues.push(RXQ_INDEX.try_into().unwrap());
9192
}
9293
}
94+
used_queues
9395
}
9496

9597
pub fn handle_evq_event(&mut self, evset: EventSet) {
@@ -106,23 +108,22 @@ where
106108
}
107109

108110
/// Notify backend of new events.
109-
pub fn notify_backend(&mut self, evset: EventSet) -> Result<(), InvalidAvailIdx> {
111+
pub fn notify_backend(&mut self, evset: EventSet) -> Result<Vec<u16>, InvalidAvailIdx> {
112+
let mut used_queues = Vec::new();
110113
self.backend.notify(evset);
111114
// After the backend has been kicked, it might've freed up some resources, so we
112115
// can attempt to send it more data to process.
113116
// In particular, if `self.backend.send_pkt()` halted the TX queue processing (by
114117
// returning an error) at some point in the past, now is the time to try walking the
115118
// TX queue again.
116119
if self.process_tx()? {
117-
self.signal_used_queue(TXQ_INDEX)
118-
.expect("vsock: Could not trigger device interrupt or TX queue");
120+
used_queues.push(TXQ_INDEX.try_into().unwrap());
119121
}
120122
if self.backend.has_pending_rx() && self.process_rx()? {
121-
self.signal_used_queue(RXQ_INDEX)
122-
.expect("vsock: Could not trigger device interrupt or RX queue");
123+
used_queues.push(RXQ_INDEX.try_into().unwrap())
123124
}
124125

125-
Ok(())
126+
Ok(used_queues)
126127
}
127128

128129
fn register_runtime_events(&self, ops: &mut EventOps) {
@@ -190,14 +191,25 @@ where
190191
let evset = event.event_set();
191192

192193
if self.is_activated() {
193-
match source {
194-
Self::PROCESS_ACTIVATE => self.handle_activate_event(ops),
194+
let used_queues = match source {
195+
Self::PROCESS_ACTIVATE => {
196+
self.handle_activate_event(ops);
197+
Vec::new()
198+
}
195199
Self::PROCESS_RXQ => self.handle_rxq_event(evset),
196200
Self::PROCESS_TXQ => self.handle_txq_event(evset),
197-
Self::PROCESS_EVQ => self.handle_evq_event(evset),
201+
Self::PROCESS_EVQ => {
202+
self.handle_evq_event(evset);
203+
Vec::new()
204+
}
198205
Self::PROCESS_NOTIFY_BACKEND => self.notify_backend(evset).unwrap(),
199-
_ => warn!("Unexpected vsock event received: {:?}", source),
206+
_ => {
207+
warn!("Unexpected vsock event received: {:?}", source);
208+
Vec::new()
209+
}
200210
};
211+
self.signal_used_queues(&used_queues)
212+
.expect("vsock: Could not trigger device interrupt");
201213
} else {
202214
warn!(
203215
"Vsock: The device is not yet activated. Spurious event received: {:?}",

0 commit comments

Comments
 (0)