On 2018年05月03日 10:09, Tiwei Bie wrote:

quoted

quoted

quoted

So how about we use the straightforward way then?

You mean we do new += vq->vring_packed.num instead
of event_idx -= vq->vring_packed.num before calling
vring_need_event()?

The problem is that, the second param (new_idx) of
vring_need_event() will be used for:

(__u16)(new_idx - event_idx - 1)
(__u16)(new_idx - old)

So if we change new, we will need to change old too.

I think that since we have a branch there anyway,
we are better off just special-casing if (wrap_counter != vq->wrap_counter).
Treat is differenty and avoid casts.

quoted

And that would be an ugly hack..

Best regards,
Tiwei Bie

I consider casts and huge numbers with two's complement
games even uglier.

The dependency on two's complement game is introduced
since the split ring.

In packed ring, old is calculated via:

old = vq->next_avail_idx - vq->num_added;

In split ring, old is calculated via:

old = vq->avail_idx_shadow - vq->num_added;

In both cases, when vq->num_added is bigger, old will
be a big number.

Best regards,
Tiwei Bie

How about just do something like vhost:

static u16 vhost_idx_diff(struct vhost_virtqueue *vq, u16 old, u16 new)
{
     if (new > old)
         return new - old;
     return  (new + vq->num - old);
}

static bool vhost_vring_packed_need_event(struct vhost_virtqueue *vq,
                       __u16 event_off, __u16 new,
                       __u16 old)
{
     return (__u16)(vhost_idx_diff(vq, new, event_off) - 1) <
            (__u16)vhost_idx_diff(vq, new, old);
}

?
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization