Revert "virtio: turn vq->notification into a nested counter"
This reverts commit aff8fd18f1
.
Both virtio-net and virtio-crypto do not balance
virtio_queue_set_notification() enable and disable calls. This makes
the notifications_disabled counter unreliable and Doug Goldstein
reported the following assertion failure:
#3 0x00007ffff44d1c62 in __GI___assert_fail (
assertion=assertion@entry=0x555555ae8e8a "vq->notification_disabled > 0",
file=file@entry=0x555555ae89c0 "/home/doug/work/qemu/hw/virtio/virtio.c",
line=line@entry=215,
function=function@entry=0x555555ae9630 <__PRETTY_FUNCTION__.43707>
"virtio_queue_set_notification") at assert.c:101
#4 0x00005555557f25d6 in virtio_queue_set_notification (vq=0x55555666aa90,
enable=enable@entry=1) at /home/doug/work/qemu/hw/virtio/virtio.c:215
#5 0x00005555557dc311 in virtio_net_has_buffers (q=<optimized out>,
q=<optimized out>, bufsize=102)
at /home/doug/work/qemu/hw/net/virtio-net.c:1008
#6 virtio_net_receive (nc=<optimized out>, buf=0x555557386b88 "", size=102)
at /home/doug/work/qemu/hw/net/virtio-net.c:1148
#7 0x00005555559cad33 in nc_sendv_compat (flags=<optimized out>, iovcnt=1,
iov=0x7fffead746d0, nc=0x55555788b340) at net/net.c:705
#8 qemu_deliver_packet_iov (sender=<optimized out>, flags=<optimized out>,
iov=0x7fffead746d0, iovcnt=1, opaque=0x55555788b340) at net/net.c:732
#9 0x00005555559cd929 in qemu_net_queue_deliver (size=<optimized out>,
data=<optimized out>, flags=<optimized out>, sender=<optimized out>,
queue=0x55555788b550) at net/queue.c:164
#10 qemu_net_queue_flush (queue=0x55555788b550) at net/queue.c:261
This patch is safe to revert since it's just an optimization for
virtqueue polling. The next patch will improve the situation again
without resorting to nesting.
Reported-by: Doug Goldstein <cardoe@cardoe.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Tested-by: Richard Henderson <rth@twiddle.net>
Tested-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
This commit is contained in:
parent
4a3f03ba8d
commit
332fa82d09
|
@ -88,8 +88,8 @@ struct VirtQueue
|
||||||
/* Last used index value we have signalled on */
|
/* Last used index value we have signalled on */
|
||||||
bool signalled_used_valid;
|
bool signalled_used_valid;
|
||||||
|
|
||||||
/* Nested host->guest notification disabled counter */
|
/* Notification enabled? */
|
||||||
unsigned int notification_disabled;
|
bool notification;
|
||||||
|
|
||||||
uint16_t queue_index;
|
uint16_t queue_index;
|
||||||
|
|
||||||
|
@ -202,7 +202,7 @@ static inline void vring_used_flags_unset_bit(VirtQueue *vq, int mask)
|
||||||
static inline void vring_set_avail_event(VirtQueue *vq, uint16_t val)
|
static inline void vring_set_avail_event(VirtQueue *vq, uint16_t val)
|
||||||
{
|
{
|
||||||
hwaddr pa;
|
hwaddr pa;
|
||||||
if (vq->notification_disabled) {
|
if (!vq->notification) {
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
pa = vq->vring.used + offsetof(VRingUsed, ring[vq->vring.num]);
|
pa = vq->vring.used + offsetof(VRingUsed, ring[vq->vring.num]);
|
||||||
|
@ -211,13 +211,7 @@ static inline void vring_set_avail_event(VirtQueue *vq, uint16_t val)
|
||||||
|
|
||||||
void virtio_queue_set_notification(VirtQueue *vq, int enable)
|
void virtio_queue_set_notification(VirtQueue *vq, int enable)
|
||||||
{
|
{
|
||||||
if (enable) {
|
vq->notification = enable;
|
||||||
assert(vq->notification_disabled > 0);
|
|
||||||
vq->notification_disabled--;
|
|
||||||
} else {
|
|
||||||
vq->notification_disabled++;
|
|
||||||
}
|
|
||||||
|
|
||||||
if (virtio_vdev_has_feature(vq->vdev, VIRTIO_RING_F_EVENT_IDX)) {
|
if (virtio_vdev_has_feature(vq->vdev, VIRTIO_RING_F_EVENT_IDX)) {
|
||||||
vring_set_avail_event(vq, vring_avail_idx(vq));
|
vring_set_avail_event(vq, vring_avail_idx(vq));
|
||||||
} else if (enable) {
|
} else if (enable) {
|
||||||
|
@ -1020,7 +1014,7 @@ void virtio_reset(void *opaque)
|
||||||
virtio_queue_set_vector(vdev, i, VIRTIO_NO_VECTOR);
|
virtio_queue_set_vector(vdev, i, VIRTIO_NO_VECTOR);
|
||||||
vdev->vq[i].signalled_used = 0;
|
vdev->vq[i].signalled_used = 0;
|
||||||
vdev->vq[i].signalled_used_valid = false;
|
vdev->vq[i].signalled_used_valid = false;
|
||||||
vdev->vq[i].notification_disabled = 0;
|
vdev->vq[i].notification = true;
|
||||||
vdev->vq[i].vring.num = vdev->vq[i].vring.num_default;
|
vdev->vq[i].vring.num = vdev->vq[i].vring.num_default;
|
||||||
vdev->vq[i].inuse = 0;
|
vdev->vq[i].inuse = 0;
|
||||||
}
|
}
|
||||||
|
@ -1831,7 +1825,7 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f, int version_id)
|
||||||
vdev->vq[i].vring.desc = qemu_get_be64(f);
|
vdev->vq[i].vring.desc = qemu_get_be64(f);
|
||||||
qemu_get_be16s(f, &vdev->vq[i].last_avail_idx);
|
qemu_get_be16s(f, &vdev->vq[i].last_avail_idx);
|
||||||
vdev->vq[i].signalled_used_valid = false;
|
vdev->vq[i].signalled_used_valid = false;
|
||||||
vdev->vq[i].notification_disabled = 0;
|
vdev->vq[i].notification = true;
|
||||||
|
|
||||||
if (vdev->vq[i].vring.desc) {
|
if (vdev->vq[i].vring.desc) {
|
||||||
/* XXX virtio-1 devices */
|
/* XXX virtio-1 devices */
|
||||||
|
|
Loading…
Reference in a new issue