From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([208.118.235.92]:41510)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <stefanha@redhat.com>) id 1UbrPo-0001LL-Bi
	for qemu-devel@nongnu.org; Mon, 13 May 2013 07:59:43 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <stefanha@redhat.com>) id 1UbrPl-000538-JP
	for qemu-devel@nongnu.org; Mon, 13 May 2013 07:59:40 -0400
Received: from mx1.redhat.com ([209.132.183.28]:63526)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <stefanha@redhat.com>) id 1UbrFO-0001ZM-Sa
	for qemu-devel@nongnu.org; Mon, 13 May 2013 07:48:55 -0400
Received: from int-mx09.intmail.prod.int.phx2.redhat.com
	(int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22])
	by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r4DBmq9x029552
	(version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK)
	for <qemu-devel@nongnu.org>; Mon, 13 May 2013 07:48:53 -0400
Date: Mon, 13 May 2013 13:48:50 +0200
From: Stefan Hajnoczi <stefanha@redhat.com>
Message-ID: <20130513114850.GA21733@stefanha-thinkpad.redhat.com>
References: <1368188849-5647-1-git-send-email-pbonzini@redhat.com>
	<1368188849-5647-3-git-send-email-pbonzini@redhat.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1368188849-5647-3-git-send-email-pbonzini@redhat.com>
Subject: Re: [Qemu-devel] [PATCH 2/2] dataplane: change vring API to use
	VirtQueueElement
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: qemu-devel@nongnu.org

On Fri, May 10, 2013 at 02:27:29PM +0200, Paolo Bonzini wrote:
> @@ -305,29 +285,28 @@ static void handle_notify(EventNotifier *e)
>          vring_disable_notification(s->vdev, &s->vring);
>  
>          for (;;) {
> -            head = vring_pop(s->vdev, &s->vring, iov, end, &out_num, &in_num);
> -            if (head < 0) {
> +            ret = vring_pop(s->vdev, &s->vring, &elem);
> +            if (ret < 0) {
>                  break; /* no more requests */

elem is leaked - we don't vring_push(elem)!

>              }
>  
> -            trace_virtio_blk_data_plane_process_request(s, out_num, in_num,
> -                                                        head);
> +            trace_virtio_blk_data_plane_process_request(s, elem->out_num,
> +                                                        elem->in_num, elem->index);
>  
> -            if (process_request(&s->ioqueue, iov, out_num, in_num, head) < 0) {
> +            if (process_request(&s->ioqueue, elem) < 0) {
>                  vring_set_broken(&s->vring);
>                  break;

elem is leaked on -EFAULT.

> @@ -268,6 +270,10 @@ int vring_pop(VirtIODevice *vdev, Vring *vring,
>       * the index we've seen. */
>      head = vring->vr.avail->ring[last_avail_idx % num];
>  
> +    elem = *p_elem = g_slice_new(VirtQueueElement);
> +    memset(elem, 0, sizeof(*elem));

VirtQueueElement is 48 KB.  We cannot affort to memset, see
de6c8042ec55da18702fa51f09072fcaa315edc3.  At that time dd if=/dev/vda
of=/dev/null iflag=direct bs=8k resulted in 10% host CPU utilization in
memset.