Skip to content

Commit 1d032f7

Browse files
committed
fix(block): only notify guest once
Currently block device has a guest notification logic inside it's request processing loop. This can create a situation when guest can continuously add more requests to the queue, making the whole request processing loop arbitrary long. This is an issue, since it block any other IO from being processed. The solution is to simply notify guest one time, after all current requests are processed. Signed-off-by: Egor Lazarchuk <[email protected]> Signed-off-by: Riccardo Mancini <[email protected]>
1 parent 6b7dea5 commit 1d032f7

File tree

1 file changed

+33
-34
lines changed
  • src/vmm/src/devices/virtio/block/virtio

1 file changed

+33
-34
lines changed

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

Lines changed: 33 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -384,24 +384,6 @@ impl VirtioBlock {
384384
}
385385
}
386386

387-
fn add_used_descriptor(
388-
queue: &mut Queue,
389-
index: u16,
390-
len: u32,
391-
irq_trigger: &IrqTrigger,
392-
block_metrics: &BlockDeviceMetrics,
393-
) {
394-
queue.add_used(index, len).unwrap_or_else(|err| {
395-
error!("Failed to add available descriptor head {}: {}", index, err)
396-
});
397-
398-
if queue.prepare_kick() {
399-
irq_trigger.trigger_irq(IrqType::Vring).unwrap_or_else(|_| {
400-
block_metrics.event_fails.inc();
401-
});
402-
}
403-
}
404-
405387
/// Device specific function for peaking inside a queue and processing descriptors.
406388
pub fn process_queue(&mut self, queue_index: usize) {
407389
// This is safe since we checked in the event handler that the device is activated.
@@ -422,7 +404,6 @@ impl VirtioBlock {
422404
break;
423405
}
424406

425-
used_any = true;
426407
request.process(&mut self.disk, head.index, mem, &self.metrics)
427408
}
428409
Err(err) => {
@@ -443,17 +424,27 @@ impl VirtioBlock {
443424
break;
444425
}
445426
ProcessingResult::Executed(finished) => {
446-
Self::add_used_descriptor(
447-
queue,
448-
head.index,
449-
finished.num_bytes_to_mem,
450-
&self.irq_trigger,
451-
&self.metrics,
452-
);
427+
used_any = true;
428+
queue
429+
.add_used(head.index, finished.num_bytes_to_mem)
430+
.unwrap_or_else(|err| {
431+
error!(
432+
"Failed to add available descriptor head {}: {}",
433+
head.index, err
434+
)
435+
});
453436
}
454437
}
455438
}
456439

440+
if used_any && queue.prepare_kick() {
441+
self.irq_trigger
442+
.trigger_irq(IrqType::Vring)
443+
.unwrap_or_else(|_| {
444+
self.metrics.event_fails.inc();
445+
});
446+
}
447+
457448
if let FileEngine::Async(ref mut engine) = self.disk.file_engine {
458449
if let Err(err) = engine.kick_submission_queue() {
459450
error!("BlockError submitting pending block requests: {:?}", err);
@@ -493,17 +484,25 @@ impl VirtioBlock {
493484
),
494485
};
495486
let finished = pending.finish(mem, res, &self.metrics);
496-
497-
Self::add_used_descriptor(
498-
queue,
499-
finished.desc_idx,
500-
finished.num_bytes_to_mem,
501-
&self.irq_trigger,
502-
&self.metrics,
503-
);
487+
queue
488+
.add_used(finished.desc_idx, finished.num_bytes_to_mem)
489+
.unwrap_or_else(|err| {
490+
error!(
491+
"Failed to add available descriptor head {}: {}",
492+
finished.desc_idx, err
493+
)
494+
});
504495
}
505496
}
506497
}
498+
499+
if queue.prepare_kick() {
500+
self.irq_trigger
501+
.trigger_irq(IrqType::Vring)
502+
.unwrap_or_else(|_| {
503+
self.metrics.event_fails.inc();
504+
});
505+
}
507506
}
508507

509508
pub fn process_async_completion_event(&mut self) {

0 commit comments

Comments
 (0)