Skip to content
Snippets Groups Projects
Commit 4c1dd505 authored by Nadav Har'El's avatar Nadav Har'El Committed by Dor Laor
Browse files

rewrite virtio_driver::wait_for_queue

In my memcached tests (with mc_benchmark as the driver), I saw
virtio_driver::wait_for_queue appears to have some bug or race condition -
in some cases it hangs on waiting for the rx queue - and simply never
returns.

I can't say I understand what the bug in this code is, however.
Instead, I just wrote it from scratch in a different way, which I think
is much clearer - and this code no longer exhibits this bug.

I can't put my finger on why my new version is more correct than
the old one - or even just difference... Dor, maybe you can find a
difference? But it definitely behaves differently.
parent 0cdb5741
No related branches found
No related tags found
No related merge requests found
......@@ -192,24 +192,23 @@ vring* virtio_driver::get_virt_queue(unsigned idx)
void virtio_driver::wait_for_queue(vring* queue, bool (vring::*pred)() const)
{
sched::thread::wait_until([queue,pred] {
bool have_elements = (queue->*pred)();
if (!have_elements) {
queue->enable_interrupts();
// we must check that the ring is not empty *after*
// we enable interrupts to avoid a race where a packet
// may have been delivered between queue->used_ring_not_empty()
// and queue->enable_interrupts() above
have_elements = (queue->*pred)();
if (have_elements) {
queue->disable_interrupts();
}
}
trace_virtio_wait_for_queue(queue, have_elements);
return have_elements;
if ((queue->*pred)()) {
return;
}
queue->enable_interrupts();
// It is possible that after checking pred() and before enabling
// interrupts, the delivered us a packet, for which we will not get an
// interrupt. So we much check again.
if ((queue->*pred)()) {
queue->disable_interrupts();
return;
}
sched::thread::wait_until([&] {
// Need to re-enable interrupts as the interrupt disabled them.
queue->enable_interrupts();
return (queue->*pred)();
});
queue->disable_interrupts();
}
u32 virtio_driver::get_device_features(void)
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment