[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: |
Pankaj Gupta |
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 02:34:34 -0400 (EDT) |
----- Original Message -----
> From: "Amit Shah" <address@hidden>
> To: "Pankaj Gupta" <address@hidden>
> Cc: address@hidden, address@hidden
> Sent: Thursday, 16 July, 2015 11:35:36 AM
> Subject: Re: [Qemu-devel] [PATCH 2/2 v3] virtio-rng: Serve pending request if
> any after timer bumps up quota.
>
> 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.
Sure.
Thanks,
Pankaj
>
> Thanks,
>
>
> Amit
>
>