From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43921) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ala09-0002gt-VR for qemu-devel@nongnu.org; Thu, 31 Mar 2016 06:38:59 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ala08-0002Mr-Iy for qemu-devel@nongnu.org; Thu, 31 Mar 2016 06:38:57 -0400 Received: from mail-ig0-x242.google.com ([2607:f8b0:4001:c05::242]:35721) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ala08-0002Mk-DE for qemu-devel@nongnu.org; Thu, 31 Mar 2016 06:38:56 -0400 Received: by mail-ig0-x242.google.com with SMTP id ya17so6025193igc.2 for ; Thu, 31 Mar 2016 03:38:56 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <20160331095849.GB20286@stefanha-x1.localdomain> References: <1459168451-2626-1-git-send-email-dhannawatpooja1@gmail.com> <20160331095849.GB20286@stefanha-x1.localdomain> Date: Thu, 31 Mar 2016 16:08:55 +0530 Message-ID: From: Pooja Dhannawat Content-Type: multipart/alternative; boundary=089e013c69d28573e5052f55de57 Subject: Re: [Qemu-devel] [PATCH v5] net: Allocating Large sized arrays to heap List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi Cc: jasowang@redhat.com, QEMU Developers --089e013c69d28573e5052f55de57 Content-Type: text/plain; charset=UTF-8 On Thu, Mar 31, 2016 at 3:28 PM, Stefan Hajnoczi wrote: > On Mon, Mar 28, 2016 at 06:04:11PM +0530, Pooja Dhannawat wrote: > > nc_sendv_compat has a huge stack usage of 69680 bytes approx. > > Moving large arrays to heap to reduce stack usage. > > > > Reviewed-by: Stefan Hajnoczi > > Signed-off-by: Pooja Dhannawat > > --- > > net/net.c | 13 +++++++++---- > > 1 file changed, 9 insertions(+), 4 deletions(-) > > > > diff --git a/net/net.c b/net/net.c > > index b0c832e..663da13 100644 > > --- a/net/net.c > > +++ b/net/net.c > > @@ -709,23 +709,28 @@ ssize_t qemu_send_packet_raw(NetClientState *nc, > const uint8_t *buf, int size) > > static ssize_t nc_sendv_compat(NetClientState *nc, const struct iovec > *iov, > > int iovcnt, unsigned flags) > > { > > - uint8_t buf[NET_BUFSIZE]; > > + uint8_t *buf = NULL; > > uint8_t *buffer; > > size_t offset; > > + ssize_t ret; > > > > if (iovcnt == 1) { > > buffer = iov[0].iov_base; > > offset = iov[0].iov_len; > > } else { > > + buf = g_new(uint8_t, NET_BUFSIZE); > > buffer = buf; > > - offset = iov_to_buf(iov, iovcnt, 0, buf, sizeof(buf)); > > + offset = iov_to_buf(iov, iovcnt, 0, buf, NET_BUFSIZE); > > } > > > > if (flags & QEMU_NET_PACKET_FLAG_RAW && nc->info->receive_raw) { > > - return nc->info->receive_raw(nc, buffer, offset); > > + ret = nc->info->receive_raw(nc, buffer, offset); > > } else { > > - return nc->info->receive(nc, buffer, offset); > > + ret = nc->info->receive(nc, buffer, offset); > > } > > + > > + g_free(buf); > > + return ret; > > } > > > > ssize_t qemu_deliver_packet_iov(NetClientState *sender, > > -- > > 2.5.0 > > CCing Jason Wang, net subsystem maintainer. Please use > scripts/get_maintainer.pl to find the right people to CC in future > patches. > > Yes, I am sorry about that. Fam also pointed out this thing but I forgot this time also. I will keep that mind from onwards. > We may also want to keep a smaller stack buffer so that reasonably-sized > packets (e.g. up to 2 KB) can be send without the performance cost of > g_malloc(). > --089e013c69d28573e5052f55de57 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable


On Thu, Mar 31, 2016 at 3:28 PM, Stefan Hajnoczi <= stefanha@gmail.com<= /a>> wrote:
On Mon, Mar 28, 2016 at 06:04:11PM +0530, Pooja Dhanna= wat wrote:
> nc_sendv_compat has a huge stack usage of 69680 bytes approx.
> Moving large arrays to heap to reduce stack usage.
>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> Signed-off-by: Pooja Dhannawat <dhannawatpooja1@gmail.com>
> ---
>=C2=A0 net/net.c | 13 +++++++++----
>=C2=A0 1 file changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/net/net.c b/net/net.c
> index b0c832e..663da13 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -709,23 +709,28 @@ ssize_t qemu_send_packet_raw(NetClientState *nc,= const uint8_t *buf, int size)
>=C2=A0 static ssize_t nc_sendv_compat(NetClientState *nc, const struct = iovec *iov,
>=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=A0 =C2=A0int iovcnt, unsigned flags)=
>=C2=A0 {
> -=C2=A0 =C2=A0 uint8_t buf[NET_BUFSIZE];
> +=C2=A0 =C2=A0 uint8_t *buf =3D NULL;
>=C2=A0 =C2=A0 =C2=A0 uint8_t *buffer;
>=C2=A0 =C2=A0 =C2=A0 size_t offset;
> +=C2=A0 =C2=A0 ssize_t ret;
>
>=C2=A0 =C2=A0 =C2=A0 if (iovcnt =3D=3D 1) {
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 buffer =3D iov[0].iov_base;
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 offset =3D iov[0].iov_len;
>=C2=A0 =C2=A0 =C2=A0 } else {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 buf =3D g_new(uint8_t, NET_BUFSIZE);
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 buffer =3D buf;
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 offset =3D iov_to_buf(iov, iovcnt, 0, buf= , sizeof(buf));
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 offset =3D iov_to_buf(iov, iovcnt, 0, buf= , NET_BUFSIZE);
>=C2=A0 =C2=A0 =C2=A0 }
>
>=C2=A0 =C2=A0 =C2=A0 if (flags & QEMU_NET_PACKET_FLAG_RAW &&= ; nc->info->receive_raw) {
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 return nc->info->receive_raw(nc, bu= ffer, offset);
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 ret =3D nc->info->receive_raw(nc, b= uffer, offset);
>=C2=A0 =C2=A0 =C2=A0 } else {
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 return nc->info->receive(nc, buffer= , offset);
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 ret =3D nc->info->receive(nc, buffe= r, offset);
>=C2=A0 =C2=A0 =C2=A0 }
> +
> +=C2=A0 =C2=A0 g_free(buf);
> +=C2=A0 =C2=A0 return ret;
>=C2=A0 }
>
>=C2=A0 ssize_t qemu_deliver_packet_iov(NetClientState *sender,
> --
> 2.5.0

CCing Jason Wang, net subsystem maintainer.=C2=A0 Please use scripts/get_maintainer.pl to find the right people to CC in future
patches.

Yes, I am sorry about that. Fam also pointed out this= thing but I forgot this time also.
I will keep that mind fr= om onwards.
=C2=A0
We may also want to keep a smaller stack buffer so that reasonably-sized packets (e.g. up to 2 KB) can be send without the performance cost of
g_malloc().

--089e013c69d28573e5052f55de57--