Re: [RFC v3 4/5] virtio_ring: add event idx support in packed ring

[Jason Wang]

On 2018å05æ03æ 10:09, Tiwei Bie wrote:
> > > > 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.
> >
> > > 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);
}

?