[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 2/2 v3] virtio-rng: Serve pending request if any
From: |
Amit Shah |
Subject: |
Re: [Qemu-devel] [PATCH 2/2 v3] virtio-rng: Serve pending request if any after timer bumps up quota. |
Date: |
Thu, 16 Jul 2015 11:35:36 +0530 |
On (Wed) 15 Jul 2015 [17:46:48], Pankaj Gupta wrote:
> We are arming timer when we get first request from guest.
> Even if guest pulls all the data we will be serving guest
> only when timer bumps up new quota. When timer expires
> we check if we have a pending request from guest, we
> serve it and re-arm the timer else we don't do any thing.
>
> Signed-off-by: Pankaj Gupta <address@hidden>
> ---
> hw/virtio/virtio-rng.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/hw/virtio/virtio-rng.c b/hw/virtio/virtio-rng.c
> index ae04352..3d9a002 100644
> --- a/hw/virtio/virtio-rng.c
> +++ b/hw/virtio/virtio-rng.c
> @@ -142,9 +142,13 @@ static int virtio_rng_load(QEMUFile *f, void *opaque,
> int version_id)
> static void check_rate_limit(void *opaque)
> {
> VirtIORNG *vrng = opaque;
> + size_t size;
>
> vrng->quota_remaining = vrng->conf.max_bytes;
> - virtio_rng_process(vrng);
> + size = get_request_size(vrng->vq, 0);
> + if (size > 0) {
> + virtio_rng_process(vrng);
> + }
> vrng->activate_timer = true;
Ah, this isn't helping us much here; we might as well return in a
similar way from virtio_rng_process.
What I had written earlier was slightly different than this:
> So now with your changes, here is what we can do: instead of just
> activating the timer in check_rate_limit(), we can issue a
> get_request_size() call. If the return value is > 0, it means we have
> a buffer queued up by the guest, and we can then call
> virtio_rng_process() and set activated to true. Else, no need to call
> virtio_rng_process at all, and the job for the timer is done, since
> there are no more outstanding requests from the guest.
Anyway we can look at that later, patch 1 is already a big improvement
so I think we should just stick with that, and think about other
things in a different series.
Thanks,
Amit