----- Original Message -----
> From: "Amit Shah" <amit.s...@redhat.com>
> To: "Pankaj Gupta" <pagu...@redhat.com>
> Cc: qemu-devel@nongnu.org, m...@redhat.com
> Sent: Wednesday, 15 July, 2015 12:09:57 PM
> Subject: Re: [Qemu-devel] [PATCH 2/2 v2] virtio-rng: Serve pending request if
> any after timer bumps up quota.
>
> On (Tue) 14 Jul 2015 [13:03:10], 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 rearm the timer else we don't do any thing.
> >
> > This patch also moves out 'request size' logic out to
> > 'check_request' function so that can be re-used.
> >
> > Signed-off-by: Pankaj Gupta <pagu...@redhat.com>
> > ---
> > hw/virtio/virtio-rng.c | 36 ++++++++++++++++++++++++------------
> > 1 file changed, 24 insertions(+), 12 deletions(-)
> >
> > diff --git a/hw/virtio/virtio-rng.c b/hw/virtio/virtio-rng.c
> > index 8774a0c..dca5064 100644
> > --- a/hw/virtio/virtio-rng.c
> > +++ b/hw/virtio/virtio-rng.c
> > @@ -37,6 +37,24 @@ static size_t get_request_size(VirtQueue *vq, unsigned
> > quota)
> > return in;
> > }
> >
> > +static size_t check_request(VirtIORNG *vrng)
> > +{
> > + size_t size;
> > + unsigned quota;
> > +
> > + if (vrng->quota_remaining < 0) {
> > + quota = 0;
> > + } else {
> > + quota = MIN((uint64_t)vrng->quota_remaining,
> > (uint64_t)UINT32_MAX);
> > + }
> > + size = get_request_size(vrng->vq, quota);
> > +
> > + trace_virtio_rng_request(vrng, size, quota);
> > +
> > + size = MIN(vrng->quota_remaining, size);
> > + return size;
> > +}
> > +
> > static void virtio_rng_process(VirtIORNG *vrng);
> >
> > /* Send data from a char device over to the guest */
> > @@ -72,7 +90,6 @@ static void chr_read(void *opaque, const void *buf,
> > size_t size)
> > static void virtio_rng_process(VirtIORNG *vrng)
> > {
> > size_t size;
> > - unsigned quota;
> >
> > if (!is_guest_ready(vrng)) {
> > return;
> > @@ -83,17 +100,8 @@ static void virtio_rng_process(VirtIORNG *vrng)
> > qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) +
> > vrng->conf.period_ms);
> > vrng->activate_timer = false;
> > }
> > + size = check_request(vrng);
> >
> > - if (vrng->quota_remaining < 0) {
> > - quota = 0;
> > - } else {
> > - quota = MIN((uint64_t)vrng->quota_remaining,
> > (uint64_t)UINT32_MAX);
> > - }
> > - size = get_request_size(vrng->vq, quota);
> > -
> > - trace_virtio_rng_request(vrng, size, quota);
> > -
> > - size = MIN(vrng->quota_remaining, size);
> > if (size) {
> > rng_backend_request_entropy(vrng->rng, size, chr_read, vrng);
> > }
> > @@ -142,9 +150,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 = check_request(vrng);
> > + if (size > 0) {
> > + virtio_rng_process(vrng);
> > + }
>
> Why is it necessary to call check_request() here, instead of just
> calling get_request_size()? We want to call virtio_rng_process() in
> case the guest has queued up another buffer, and for that,
> get_request_size() is sufficient. check_request() is anyway something
> that virtio_rng_process() is going to do as soon as it's called...
get_request_size calls 'virtqueue_get_avail_bytes'
both take quota, which we are setting to zero when 'vrng->quota_remaining' < 0.
virtqueue_get_avail_bytes(VirtQueue *vq, unsigned int *in_bytes,
unsigned int *out_bytes,
unsigned max_in_bytes, unsigned max_out_bytes)
|
----> quota
We read descriptors as soon as we get input request size > quota (0 for -ve
quota_remaining)
if (in_total >= max_in_bytes && out_total >= max_out_bytes) {
goto done;
}
So, I thought better to keep same logic even if we get -ve quota value(which we
might not get here)
and reuse the code.
>
> Amit
>
>
