LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Jason Wang <jasowang@redhat.com>
To: Tiwei Bie <tiwei.bie@intel.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>,
virtualization@lists.linux-foundation.org,
linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
wexu@redhat.com, jfreimann@redhat.com
Subject: Re: [RFC v3 4/5] virtio_ring: add event idx support in packed ring
Date: Tue, 8 May 2018 17:34:40 +0800 [thread overview]
Message-ID: <122277c6-d103-e1f6-d695-4d64e6934a51@redhat.com> (raw)
In-Reply-To: <20180508091628.d7jzpopqopq4abhy@debian>
On 2018年05月08日 17:16, Tiwei Bie wrote:
> On Tue, May 08, 2018 at 03:16:53PM +0800, Jason Wang wrote:
>> On 2018年05月08日 14:44, Tiwei Bie wrote:
>>> On Tue, May 08, 2018 at 01:40:40PM +0800, Jason Wang wrote:
>>>> On 2018年05月08日 11:05, Jason Wang wrote:
>>>>>> Because in virtqueue_enable_cb_delayed(), we may set an
>>>>>> event_off which is bigger than new and both of them have
>>>>>> wrapped. And in this case, although new is smaller than
>>>>>> event_off (i.e. the third param -- old), new shouldn't
>>>>>> add vq->num, and actually we are expecting a very big
>>>>>> idx diff.
>>>>> Yes, so to calculate distance correctly between event and new, we just
>>>>> need to compare the warp counter and return false if it doesn't match
>>>>> without the need to try to add vq.num here.
>>>>>
>>>>> Thanks
>>>> Sorry, looks like the following should work, we need add vq.num if
>>>> used_wrap_counter does not match:
>>>>
>>>> static bool vhost_vring_packed_need_event(struct vhost_virtqueue *vq,
>>>> __u16 off_wrap, __u16 new,
>>>> __u16 old)
>>>> {
>>>> bool wrap = off_wrap >> 15;
>>>> int off = off_wrap & ~(1 << 15);
>>>> __u16 d1, d2;
>>>>
>>>> if (wrap != vq->used_wrap_counter)
>>>> d1 = new + vq->num - off - 1;
>>> Just to draw your attention (maybe you have already
>>> noticed this).
>> I miss this, thanks!
>>
>>> In this case (i.e. wrap != vq->used_wrap_counter),
>>> it's also possible that (off < new) is true. Because,
>>>
>>> when virtqueue_enable_cb_delayed_packed() is used,
>>> `off` is calculated in driver in a way like this:
>>>
>>> off = vq->last_used_idx + bufs;
>>> if (off >= vq->vring_packed.num) {
>>> off -= vq->vring_packed.num;
>>> wrap_counter ^= 1;
>>> }
>>>
>>> And when `new` (in vhost) is close to vq->num. The
>>> vq->last_used_idx + bufs (in driver) can be bigger
>>> than vq->vring_packed.num, and:
>>>
>>> 1. `off` will wrap;
>>> 2. wrap counters won't match;
>>> 3. off < new;
>>>
>>> And d1 (i.e. new + vq->num - off - 1) will be a value
>>> bigger than vq->num. I'm okay with this, although it's
>>> a bit weird.
>>
>> So I'm considering something more compact by reusing vring_need_event() by
>> pretending a larger queue size and adding vq->num back when necessary:
>>
>> static bool vhost_vring_packed_need_event(struct vhost_virtqueue *vq,
>> __u16 off_wrap, __u16 new,
>> __u16 old)
>> {
>> bool wrap = vq->used_wrap_counter;
> If the wrap counter is obtained from the vq,
> I think `new` should also be obtained from
> the vq. Or the wrap counter should be carried
> in `new`.
>
>> int off = off_wrap & ~(1 << 15);
>> __u16 d1, d2;
>>
>> if (new < old) {
>> new += vq->num;
>> wrap ^= 1;
>> }
>>
>> if (wrap != off_wrap >> 15)
>> off += vq->num;
> When `new` and `old` wraps, and `off` doesn't wrap,
> wrap != (off_wrap >> 15) will be true. In this case,
> `off` is bigger than `new`, and what we should do
> is `off -= vq->num` instead of `off += vq->num`.
If I understand this correctly, if we track old correctly, it won't
happen if guest driver behave correctly. That means it should only
happen for a buggy driver (e.g trying to move off_wrap back).
Thanks
>
> Best regards,
> Tiwei Bie
>
>> return vring_need_event(off, new, old);
>> }
>>
>>
>>> Best regards,
>>> Tiwei Bie
>>>
>>>> else
>>>> d1 = new - off - 1;
>>>>
>>>> if (new > old)
>>>> d2 = new - old;
>>>> else
>>>> d2 = new + vq->num - old;
>>>>
>>>> return d1 < d2;
>>>> }
>>>>
>>>> Thanks
>>>>
next prev parent reply other threads:[~2018-05-08 9:35 UTC|newest]
Thread overview: 37+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-04-25 5:15 [RFC v3 0/5] virtio: support " Tiwei Bie
2018-04-25 5:15 ` [RFC v3 1/5] virtio: add packed ring definitions Tiwei Bie
2018-04-25 5:15 ` [RFC v3 2/5] virtio_ring: support creating packed ring Tiwei Bie
2018-04-25 5:15 ` [RFC v3 3/5] virtio_ring: add packed ring support Tiwei Bie
2018-05-10 7:32 ` Jason Wang
2018-05-10 7:34 ` Jason Wang
2018-05-10 8:56 ` Tiwei Bie
2018-05-10 9:49 ` Jason Wang
2018-05-10 10:50 ` Tiwei Bie
2018-04-25 5:15 ` [RFC v3 4/5] virtio_ring: add event idx support in packed ring Tiwei Bie
2018-05-02 2:51 ` Jason Wang
2018-05-02 7:28 ` Tiwei Bie
2018-05-02 13:51 ` Michael S. Tsirkin
2018-05-02 15:12 ` Tiwei Bie
2018-05-02 15:42 ` Michael S. Tsirkin
2018-05-03 1:11 ` Tiwei Bie
2018-05-03 1:44 ` Michael S. Tsirkin
2018-05-03 2:09 ` Tiwei Bie
2018-05-03 7:25 ` Jason Wang
2018-05-03 13:54 ` Tiwei Bie
2018-05-08 3:05 ` Jason Wang
2018-05-08 5:40 ` Jason Wang
2018-05-08 6:44 ` Tiwei Bie
2018-05-08 7:16 ` Jason Wang
2018-05-08 9:16 ` Tiwei Bie
2018-05-08 9:34 ` Jason Wang [this message]
2018-05-08 9:44 ` Tiwei Bie
2018-05-09 3:43 ` Jason Wang
2018-05-16 5:01 ` Jason Wang
2018-05-16 5:55 ` Tiwei Bie
2018-04-25 5:15 ` [RFC v3 5/5] virtio_ring: enable " Tiwei Bie
2018-04-27 3:56 ` [RFC v3 0/5] virtio: support " Jason Wang
2018-04-27 4:18 ` Michael S. Tsirkin
2018-04-27 6:17 ` Jason Wang
2018-04-27 9:12 ` Tiwei Bie
2018-04-28 2:45 ` Jason Wang
2018-05-02 3:49 ` Jason Wang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=122277c6-d103-e1f6-d695-4d64e6934a51@redhat.com \
--to=jasowang@redhat.com \
--cc=jfreimann@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mst@redhat.com \
--cc=netdev@vger.kernel.org \
--cc=tiwei.bie@intel.com \
--cc=virtualization@lists.linux-foundation.org \
--cc=wexu@redhat.com \
--subject='Re: [RFC v3 4/5] virtio_ring: add event idx support in packed ring' \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).