This doesn't work for eventidx:
1) vringh_notify_enable_user() gets called because the ring is empty
(vrh->last_avail_idx == vrh->vring.avail->idx).
2) It puts vrh->last_avail_idx into vring_avail_event().
3) It sets ->listening to true.
4) Meanwhile, the other side has done more work, updating vrh->vring.avail->idx.
5) It notices vrh->vring.avail->idx has changed, so returns true to
make us do more work.
But since the value we wrote into vring_avail_event() is past, we
won't get notified. This happens rarely: only once I'd optimized
add_buf could I trigger this in about 1 in 10 runs.
Since it's so rare, just remove the listening flag: it's fine to do
this twice anyway.
Signed-off-by: Rusty Russell <rusty@xxxxxxxxxxxxxxx>
diff --git a/drivers/vhost/vringh.c b/drivers/vhost/vringh.c
index 8234f2b..d4413d9 100644
--- a/drivers/vhost/vringh.c
+++ b/drivers/vhost/vringh.c
@@ -476,12 +476,6 @@ static inline bool __vringh_notify_enable(struct vringh *vrh,
{
u16 avail;
- /* Already enabled? */
- if (vrh->listening)
- return false;
-
- vrh->listening = true;
-
if (!vrh->event_indices) {
/* Old-school; update flags. */
if (putu16(&vrh->vring.used->flags, 0) != 0) {
@@ -515,11 +509,6 @@ static inline bool __vringh_notify_enable(struct vringh *vrh,
static inline void __vringh_notify_disable(struct vringh *vrh,
int (*putu16)(u16 *p, u16 val))
{
- /* Already disabled? */
- if (!vrh->listening)
- return;
-
- vrh->listening = false;
if (!vrh->event_indices) {
/* Old-school; update flags. */
if (putu16(&vrh->vring.used->flags, VRING_USED_F_NO_NOTIFY)) {
@@ -593,7 +582,6 @@ int vringh_init_user(struct vringh *vrh, u32 features,
vrh->event_indices = (features & (1 << VIRTIO_RING_F_EVENT_IDX));
vrh->weak_barriers = weak_barriers;
- vrh->listening = false;
vrh->completed = 0;
vrh->last_avail_idx = 0;
vrh->last_used_idx = 0;
@@ -853,7 +841,6 @@ int vringh_init_kern(struct vringh *vrh, u32 features,
vrh->event_indices = (features & (1 << VIRTIO_RING_F_EVENT_IDX));
vrh->weak_barriers = weak_barriers;
- vrh->listening = false;
vrh->completed = 0;
vrh->last_avail_idx = 0;
vrh->last_used_idx = 0;
diff --git a/include/linux/vringh.h b/include/linux/vringh.h
index ab41185..44b94cd 100644
--- a/include/linux/vringh.h
+++ b/include/linux/vringh.h
@@ -33,9 +33,6 @@ struct vringh {
/* Guest publishes used event idx (note: we always do). */
bool event_indices;
- /* Have we told the other end we want to be notified? */
- bool listening;
-
/* Can we get away with weak barriers? */
bool weak_barriers;
_______________________________________________
Virtualization mailing list
Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
