| Message ID | 1457973473-21791-1-git-send-email-dhannawatpooja1@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Mar 14, 2016 at 10:07:53PM +0530, Pooja Dhannawat wrote: > net_socket_send has a huge stack usage of 69712 bytes approx. > Moving large arrays to heap to reduce stack usage. > > Signed-off-by: Pooja Dhannawat <dhannawatpooja1@gmail.com> > --- > net/socket.c | 8 +++++--- > 1 file changed, 5 insertions(+), 3 deletions(-) > > diff --git a/net/socket.c b/net/socket.c > index e32e3cb..3fcd7a6 100644 > --- a/net/socket.c > +++ b/net/socket.c > @@ -147,10 +147,10 @@ static void net_socket_send(void *opaque) > NetSocketState *s = opaque; > int size, err; > unsigned l; > - uint8_t buf1[NET_BUFSIZE]; > + uint8_t *buf1 = g_new(uint8_t, NET_BUFSIZE); You're allocating NET_BUFSIZE worth of uint8_t's > const uint8_t *buf; > > - size = qemu_recv(s->fd, buf1, sizeof(buf1), 0); > + size = qemu_recv(s->fd, (uint8_t *)buf1, sizeof(uint8_t), 0); But only reading 1 byte which is clearly wrong. You likely wanted NET_BUFSIZE here, not sizeof(uint8_t) Regards, Daniel
On Mon, Mar 14, 2016 at 10:20 PM, Daniel P. Berrange <berrange@redhat.com> wrote: > On Mon, Mar 14, 2016 at 10:07:53PM +0530, Pooja Dhannawat wrote: > > net_socket_send has a huge stack usage of 69712 bytes approx. > > Moving large arrays to heap to reduce stack usage. > > > > Signed-off-by: Pooja Dhannawat <dhannawatpooja1@gmail.com> > > --- > > net/socket.c | 8 +++++--- > > 1 file changed, 5 insertions(+), 3 deletions(-) > > > > diff --git a/net/socket.c b/net/socket.c > > index e32e3cb..3fcd7a6 100644 > > --- a/net/socket.c > > +++ b/net/socket.c > > @@ -147,10 +147,10 @@ static void net_socket_send(void *opaque) > > NetSocketState *s = opaque; > > int size, err; > > unsigned l; > > - uint8_t buf1[NET_BUFSIZE]; > > + uint8_t *buf1 = g_new(uint8_t, NET_BUFSIZE); > > You're allocating NET_BUFSIZE worth of uint8_t's > > I didn't get you clear. > > const uint8_t *buf; > > > > - size = qemu_recv(s->fd, buf1, sizeof(buf1), 0); > > + size = qemu_recv(s->fd, (uint8_t *)buf1, sizeof(uint8_t), 0); > > But only reading 1 byte which is clearly wrong. You likely wanted > NET_BUFSIZE here, not sizeof(uint8_t) > > Correct me If I am wrong. This should also work : size = qemu_recv(s->fd, (uint8_t *)buf1, NET_BUFSIZE * sizeof(uint8_t), 0); > Regards, > Daniel > -- > |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ > :| > |: http://libvirt.org -o- http://virt-manager.org > :| > |: http://autobuild.org -o- http://search.cpan.org/~danberr/ > :| > |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc > :| >
On Mon, Mar 14, 2016 at 10:44:22PM +0530, Pooja Dhannawat wrote: > On Mon, Mar 14, 2016 at 10:20 PM, Daniel P. Berrange <berrange@redhat.com> > wrote: > > > On Mon, Mar 14, 2016 at 10:07:53PM +0530, Pooja Dhannawat wrote: > > > net_socket_send has a huge stack usage of 69712 bytes approx. > > > Moving large arrays to heap to reduce stack usage. > > > > > > Signed-off-by: Pooja Dhannawat <dhannawatpooja1@gmail.com> > > > --- > > > net/socket.c | 8 +++++--- > > > 1 file changed, 5 insertions(+), 3 deletions(-) > > > > > > diff --git a/net/socket.c b/net/socket.c > > > index e32e3cb..3fcd7a6 100644 > > > --- a/net/socket.c > > > +++ b/net/socket.c > > > @@ -147,10 +147,10 @@ static void net_socket_send(void *opaque) > > > NetSocketState *s = opaque; > > > int size, err; > > > unsigned l; > > > - uint8_t buf1[NET_BUFSIZE]; > > > + uint8_t *buf1 = g_new(uint8_t, NET_BUFSIZE); > > > > You're allocating NET_BUFSIZE worth of uint8_t's > > > > I didn't get you clear. > > > > > const uint8_t *buf; > > > > > > - size = qemu_recv(s->fd, buf1, sizeof(buf1), 0); > > > + size = qemu_recv(s->fd, (uint8_t *)buf1, sizeof(uint8_t), 0); > > > > But only reading 1 byte which is clearly wrong. You likely wanted > > NET_BUFSIZE here, not sizeof(uint8_t) > > > > Correct me If I am wrong. This should also work : > size = qemu_recv(s->fd, (uint8_t *)buf1, NET_BUFSIZE * sizeof(uint8_t), 0); Sure, if you want to add a pointless 'multiply by 1' to the code, which you didn't bother doing for the g_new call. Regards, Daniel
Pooja Dhannawat <dhannawatpooja1@gmail.com> writes: > On Mon, Mar 14, 2016 at 10:20 PM, Daniel P. Berrange <berrange@redhat.com> > wrote: > >> On Mon, Mar 14, 2016 at 10:07:53PM +0530, Pooja Dhannawat wrote: >> > net_socket_send has a huge stack usage of 69712 bytes approx. >> > Moving large arrays to heap to reduce stack usage. >> > >> > Signed-off-by: Pooja Dhannawat <dhannawatpooja1@gmail.com> >> > --- >> > net/socket.c | 8 +++++--- >> > 1 file changed, 5 insertions(+), 3 deletions(-) >> > >> > diff --git a/net/socket.c b/net/socket.c >> > index e32e3cb..3fcd7a6 100644 >> > --- a/net/socket.c >> > +++ b/net/socket.c >> > @@ -147,10 +147,10 @@ static void net_socket_send(void *opaque) >> > NetSocketState *s = opaque; >> > int size, err; >> > unsigned l; >> > - uint8_t buf1[NET_BUFSIZE]; >> > + uint8_t *buf1 = g_new(uint8_t, NET_BUFSIZE); >> >> You're allocating NET_BUFSIZE worth of uint8_t's >> >> I didn't get you clear. > > >> > const uint8_t *buf; >> > >> > - size = qemu_recv(s->fd, buf1, sizeof(buf1), 0); >> > + size = qemu_recv(s->fd, (uint8_t *)buf1, sizeof(uint8_t), 0); >> >> But only reading 1 byte which is clearly wrong. You likely wanted >> NET_BUFSIZE here, not sizeof(uint8_t) >> >> Correct me If I am wrong. This should also work : > size = qemu_recv(s->fd, (uint8_t *)buf1, NET_BUFSIZE * sizeof(uint8_t), 0); An expression of type "T[]" deteriorates to "T *" without a cast. Therefore, your cast of buf1 to uint8_t * is redundant and clutters the code, please drop it. In general, try hard to avoid casts not just to reduce clutter, but also because casts can hide mistakes from the type checker. sizeof(uint8_t) is always one. Multiplying by it clutters the code, please drop it.
diff --git a/net/socket.c b/net/socket.c index e32e3cb..3fcd7a6 100644 --- a/net/socket.c +++ b/net/socket.c @@ -147,10 +147,10 @@ static void net_socket_send(void *opaque) NetSocketState *s = opaque; int size, err; unsigned l; - uint8_t buf1[NET_BUFSIZE]; + uint8_t *buf1 = g_new(uint8_t, NET_BUFSIZE); const uint8_t *buf; - size = qemu_recv(s->fd, buf1, sizeof(buf1), 0); + size = qemu_recv(s->fd, (uint8_t *)buf1, sizeof(uint8_t), 0); if (size < 0) { err = socket_error(); if (err != EWOULDBLOCK) @@ -170,8 +170,8 @@ static void net_socket_send(void *opaque) s->index = 0; s->packet_len = 0; s->nc.link_down = true; - memset(s->buf, 0, sizeof(s->buf)); memset(s->nc.info_str, 0, sizeof(s->nc.info_str)); + g_free(buf1); return; } @@ -222,6 +222,8 @@ static void net_socket_send(void *opaque) break; } } + + g_free(buf1); } static void net_socket_send_dgram(void *opaque)
net_socket_send has a huge stack usage of 69712 bytes approx. Moving large arrays to heap to reduce stack usage. Signed-off-by: Pooja Dhannawat <dhannawatpooja1@gmail.com> --- net/socket.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)