From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47950) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZFckh-0005hM-6z for qemu-devel@nongnu.org; Thu, 16 Jul 2015 02:34:40 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZFcke-0006sP-13 for qemu-devel@nongnu.org; Thu, 16 Jul 2015 02:34:39 -0400 Received: from mx3-phx2.redhat.com ([209.132.183.24]:58402) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZFckd-0006rq-QC for qemu-devel@nongnu.org; Thu, 16 Jul 2015 02:34:35 -0400 Date: Thu, 16 Jul 2015 02:34:34 -0400 (EDT) From: Pankaj Gupta Message-ID: <1122658158.33764399.1437028474948.JavaMail.zimbra@redhat.com> In-Reply-To: <20150716060536.GW10280@grmbl.mre> References: <1436962608-9961-1-git-send-email-pagupta@redhat.com> <1436962608-9961-3-git-send-email-pagupta@redhat.com> <20150716060536.GW10280@grmbl.mre> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 2/2 v3] virtio-rng: Serve pending request if any after timer bumps up quota. List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Amit Shah Cc: qemu-devel@nongnu.org, mst@redhat.com ----- Original Message ----- > From: "Amit Shah" > To: "Pankaj Gupta" > Cc: qemu-devel@nongnu.org, mst@redhat.com > Sent: Thursday, 16 July, 2015 11:35:36 AM > Subject: Re: [Qemu-devel] [PATCH 2/2 v3] virtio-rng: Serve pending reques= t if any after timer bumps up quota. >=20 > 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. > >=20 > > Signed-off-by: Pankaj Gupta > > --- > > =C2=A0hw/virtio/virtio-rng.c | 6 +++++- > > =C2=A01 file changed, 5 insertions(+), 1 deletion(-) > >=20 > > 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 *opaq= ue, > > int version_id) > > =C2=A0static void check_rate_limit(void *opaque) > > =C2=A0{ > > =C2=A0 =C2=A0 =C2=A0VirtIORNG *vrng =3D opaque; > > + =C2=A0 =C2=A0size_t size; > > =C2=A0 > > =C2=A0 =C2=A0 =C2=A0vrng->quota_remaining =3D vrng->conf.max_bytes; > > - =C2=A0 =C2=A0virtio_rng_process(vrng); > > + =C2=A0 =C2=A0size =3D get_request_size(vrng->vq, 0); > > + =C2=A0 =C2=A0if (size > 0) { > > + =C2=A0 =C2=A0 =C2=A0 =C2=A0virtio_rng_process(vrng); > > + =C2=A0 =C2=A0} > > =C2=A0 =C2=A0 =C2=A0vrng->activate_timer =3D true; >=20 > Ah, this isn't helping us much here; we might as well return in a > similar way from virtio_rng_process. >=20 > What I had written earlier was slightly different than this: >=20 > > 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. =C2=A0If 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. =C2=A0Else, 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. >=20 > 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 >=20 > Thanks, >=20 >=20 > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0Amit >=20 >=20