From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:34601) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UVj9e-0004Cn-Kk for qemu-devel@nongnu.org; Fri, 26 Apr 2013 09:57:40 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UVj9c-00076p-5j for qemu-devel@nongnu.org; Fri, 26 Apr 2013 09:57:38 -0400 Received: from mx1.redhat.com ([209.132.183.28]:53073) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UVj9b-00076J-VM for qemu-devel@nongnu.org; Fri, 26 Apr 2013 09:57:36 -0400 Message-ID: <517A88B1.4040809@redhat.com> Date: Fri, 26 Apr 2013 16:01:21 +0200 From: Hans de Goede MIME-Version: 1.0 References: <1366885267-9694-1-git-send-email-hdegoede@redhat.com> <517A73CC.8040205@redhat.com> In-Reply-To: <517A73CC.8040205@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] uhci: Use an intermediate buffer for usb packet data List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Gerd Hoffmann Cc: Jan Kiszka , qemu-devel@nongnu.org Hi, On 04/26/2013 02:32 PM, Gerd Hoffmann wrote: > Hi, > >> + if (max_len <= sizeof(async->static_buf)) { >> + async->buf = async->static_buf; >> + } else { >> + async->buf = g_malloc(max_len); >> + } > > Do we need this? I think we should simply make the static buffer big > enough for the maximum allowed packet size (isn't that big on usb 1.1, > isn't it?) or allocate dynamically unconditionally. The maximum size in the USB spec is 1023 bytes, uhci spec allows upto 1280, but mentions in the uhci spec that the usb spec limits things to 1023, this is only for isoc endpoints, for all others the maximum packet size is 64, hence I opted for the static buffer of 64 bytes, which means avoiding the malloc / free for all but isoc endpoints, while not wasting 960 bytes / packet for the common case. I'm fine with making the static buf 1024 bytes. >> + qemu_iovec_add(&async->packet.iov, async->buf, max_len); > > There is usb_packet_addbuf() ... Will fix as soon we know what we want to do with the buffer. Regards, Hans