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 15:16:53 +0800	[thread overview]
Message-ID: <34f2c690-7cb2-f9ea-2ce9-40f4ccb594c9@redhat.com> (raw)
In-Reply-To: <20180508064409.kcn6amhsxu7nkuuc@debian>



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;
     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;

     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
>>

  reply	other threads:[~2018-05-08  7:17 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 [this message]
2018-05-08  9:16                               ` Tiwei Bie
2018-05-08  9:34                                 ` Jason Wang
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=34f2c690-7cb2-f9ea-2ce9-40f4ccb594c9@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).