| Message ID | 1461633961-31713-1-git-send-email-zhoujie2011@cn.fujitsu.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 04/26/2016 09:26 AM, Zhou Jie wrote: > net_init_tap has a huge stack usage of 8192 bytes approx. > Moving large arrays to heap to reduce stack usage. > > Signed-off-by: Zhou Jie <zhoujie2011@cn.fujitsu.com> > --- > net/tap.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/net/tap.c b/net/tap.c > index 740e8a2..49817c7 100644 > --- a/net/tap.c > +++ b/net/tap.c > @@ -769,8 +769,8 @@ int net_init_tap(const NetClientOptions *opts, const char *name, > return -1; > } > } else if (tap->has_fds) { > - char *fds[MAX_TAP_QUEUES]; > - char *vhost_fds[MAX_TAP_QUEUES]; > + char **fds = g_new(char *, MAX_TAP_QUEUES); > + char **vhost_fds = g_new(char *, MAX_TAP_QUEUES); > int nfds, nvhosts; > > if (tap->has_ifname || tap->has_script || tap->has_downscript || > @@ -818,6 +818,8 @@ int net_init_tap(const NetClientOptions *opts, const char *name, > return -1; > } > } > + g_free(fds); > + g_free(vhost_fds); > } else if (tap->has_helper) { > if (tap->has_ifname || tap->has_script || tap->has_downscript || > tap->has_vnet_hdr || tap->has_queues || tap->has_vhostfds) { Apply to net-next. Thanks
On 04/26/2016 03:26 AM, Zhou Jie wrote: > net_init_tap has a huge stack usage of 8192 bytes approx. > Moving large arrays to heap to reduce stack usage. I am wondering. Why is 8k a problem for a user space program? Please note that malloc/new like allocations are much more expensive than stack allocation in terms of performance. This does not matter here, but in your other patch that deals with the xmit function, I would not be surprised if that actually harms performance. Christian > > Signed-off-by: Zhou Jie <zhoujie2011@cn.fujitsu.com> > --- > net/tap.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/net/tap.c b/net/tap.c > index 740e8a2..49817c7 100644 > --- a/net/tap.c > +++ b/net/tap.c > @@ -769,8 +769,8 @@ int net_init_tap(const NetClientOptions *opts, const char *name, > return -1; > } > } else if (tap->has_fds) { > - char *fds[MAX_TAP_QUEUES]; > - char *vhost_fds[MAX_TAP_QUEUES]; > + char **fds = g_new(char *, MAX_TAP_QUEUES); > + char **vhost_fds = g_new(char *, MAX_TAP_QUEUES); > int nfds, nvhosts; > > if (tap->has_ifname || tap->has_script || tap->has_downscript || > @@ -818,6 +818,8 @@ int net_init_tap(const NetClientOptions *opts, const char *name, > return -1; > } > } > + g_free(fds); > + g_free(vhost_fds); > } else if (tap->has_helper) { > if (tap->has_ifname || tap->has_script || tap->has_downscript || > tap->has_vnet_hdr || tap->has_queues || tap->has_vhostfds) { >
On 2016/4/26 15:45, Christian Borntraeger wrote: > On 04/26/2016 03:26 AM, Zhou Jie wrote: >> net_init_tap has a huge stack usage of 8192 bytes approx. >> Moving large arrays to heap to reduce stack usage. > > I am wondering. Why is 8k a problem for a user space program? For 64bit machine it will be 16k. > Please note that malloc/new like allocations are much more expensive > than stack allocation in terms of performance. This does not matter > here, but in your other patch that deals with the xmit function, I would > not be surprised if that actually harms performance. OK. I will note it. Sincerely, Zhou Jie > > Christian > > > >> >> Signed-off-by: Zhou Jie <zhoujie2011@cn.fujitsu.com> >> --- >> net/tap.c | 6 ++++-- >> 1 file changed, 4 insertions(+), 2 deletions(-) >> >> diff --git a/net/tap.c b/net/tap.c >> index 740e8a2..49817c7 100644 >> --- a/net/tap.c >> +++ b/net/tap.c >> @@ -769,8 +769,8 @@ int net_init_tap(const NetClientOptions *opts, const char *name, >> return -1; >> } >> } else if (tap->has_fds) { >> - char *fds[MAX_TAP_QUEUES]; >> - char *vhost_fds[MAX_TAP_QUEUES]; >> + char **fds = g_new(char *, MAX_TAP_QUEUES); >> + char **vhost_fds = g_new(char *, MAX_TAP_QUEUES); >> int nfds, nvhosts; >> >> if (tap->has_ifname || tap->has_script || tap->has_downscript || >> @@ -818,6 +818,8 @@ int net_init_tap(const NetClientOptions *opts, const char *name, >> return -1; >> } >> } >> + g_free(fds); >> + g_free(vhost_fds); >> } else if (tap->has_helper) { >> if (tap->has_ifname || tap->has_script || tap->has_downscript || >> tap->has_vnet_hdr || tap->has_queues || tap->has_vhostfds) { >> > > > >
On 04/26/2016 10:26 AM, Zhou Jie wrote: > On 2016/4/26 15:45, Christian Borntraeger wrote: >> On 04/26/2016 03:26 AM, Zhou Jie wrote: >>> net_init_tap has a huge stack usage of 8192 bytes approx. >>> Moving large arrays to heap to reduce stack usage. >> >> I am wondering. Why is 8k a problem for a user space program? > For 64bit machine it will be 16k. Even that does not matter. The userspace stack is on most Linuxes somewhere between 8MB and unlimited. Reducing stack usage makes sense for the kernel as the kernel stack is limited to 8k or 16k. For userspace it does also make sense for things like thread stacks or coroutines, where we have a much smaller stack (well, still 1MB for coroutines as of today) and if we are not on a hot path. Regarding hot pathes: There is a reason why people use jemalloc or tcmalloc. The allocators try to use per-thread arena, but still each allocation might cause significant cross cpu/thread traffic. And having a allocator that scales well across many CPUs is not trivial. > >> Please note that malloc/new like allocations are much more expensive >> than stack allocation in terms of performance. This does not matter >> here, but in your other patch that deals with the xmit function, I would >> not be surprised if that actually harms performance. > OK. I will note it. > > Sincerely, > Zhou Jie > >> >> Christian >> >> >> >>> >>> Signed-off-by: Zhou Jie <zhoujie2011@cn.fujitsu.com> >>> --- >>> net/tap.c | 6 ++++-- >>> 1 file changed, 4 insertions(+), 2 deletions(-) >>> >>> diff --git a/net/tap.c b/net/tap.c >>> index 740e8a2..49817c7 100644 >>> --- a/net/tap.c >>> +++ b/net/tap.c >>> @@ -769,8 +769,8 @@ int net_init_tap(const NetClientOptions *opts, const char *name, >>> return -1; >>> } >>> } else if (tap->has_fds) { >>> - char *fds[MAX_TAP_QUEUES]; >>> - char *vhost_fds[MAX_TAP_QUEUES]; >>> + char **fds = g_new(char *, MAX_TAP_QUEUES); >>> + char **vhost_fds = g_new(char *, MAX_TAP_QUEUES); >>> int nfds, nvhosts; >>> >>> if (tap->has_ifname || tap->has_script || tap->has_downscript || >>> @@ -818,6 +818,8 @@ int net_init_tap(const NetClientOptions *opts, const char *name, >>> return -1; >>> } >>> } >>> + g_free(fds); >>> + g_free(vhost_fds); >>> } else if (tap->has_helper) { >>> if (tap->has_ifname || tap->has_script || tap->has_downscript || >>> tap->has_vnet_hdr || tap->has_queues || tap->has_vhostfds) { >>> >> >> >> >> >
diff --git a/net/tap.c b/net/tap.c index 740e8a2..49817c7 100644 --- a/net/tap.c +++ b/net/tap.c @@ -769,8 +769,8 @@ int net_init_tap(const NetClientOptions *opts, const char *name, return -1; } } else if (tap->has_fds) { - char *fds[MAX_TAP_QUEUES]; - char *vhost_fds[MAX_TAP_QUEUES]; + char **fds = g_new(char *, MAX_TAP_QUEUES); + char **vhost_fds = g_new(char *, MAX_TAP_QUEUES); int nfds, nvhosts; if (tap->has_ifname || tap->has_script || tap->has_downscript || @@ -818,6 +818,8 @@ int net_init_tap(const NetClientOptions *opts, const char *name, return -1; } } + g_free(fds); + g_free(vhost_fds); } else if (tap->has_helper) { if (tap->has_ifname || tap->has_script || tap->has_downscript || tap->has_vnet_hdr || tap->has_queues || tap->has_vhostfds) {
net_init_tap has a huge stack usage of 8192 bytes approx. Moving large arrays to heap to reduce stack usage. Signed-off-by: Zhou Jie <zhoujie2011@cn.fujitsu.com> --- net/tap.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)