Re: [Qemu-devel] [PATCH 2/2 v3] virtio-rng: Serve pending request if any after timer bumps up quota.

2015-07-17 Thread Amit Shah 
On (Thu) 16 Jul 2015 [02:34:34], Pankaj Gupta wrote:
  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.

I think we can apply patch 1 for 2.4, since it reduces the wakeups we
cause.  If you can post powertop numbers, that'll be great.

I'll do a pull req in the meantime.

Thanks,

Amit

Re: [Qemu-devel] [PATCH 2/2 v3] virtio-rng: Serve pending request if any after timer bumps up quota.

2015-07-16 Thread Amit Shah 
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 pagu...@redhat.com
 ---
  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

Re: [Qemu-devel] [PATCH 2/2 v3] virtio-rng: Serve pending request if any after timer bumps up quota.

2015-07-16 Thread Pankaj Gupta 


- Original Message -
 From: Amit Shah amit.s...@redhat.com
 To: Pankaj Gupta pagu...@redhat.com
 Cc: qemu-devel@nongnu.org, m...@redhat.com
 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 pagu...@redhat.com
  ---
   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

[Qemu-devel] [PATCH 2/2 v3] virtio-rng: Serve pending request if any after timer bumps up quota.

2015-07-15 Thread Pankaj Gupta 
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 pagu...@redhat.com
---
 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;
 }
 
-- 
1.9.3