* [PATCH] pktgen: reduce stack usage
@ 2005-02-18 21:42 Randy.Dunlap
2005-02-18 22:11 ` Francois Romieu
2005-02-19 2:56 ` Ben Greear
0 siblings, 2 replies; 10+ messages in thread
From: Randy.Dunlap @ 2005-02-18 21:42 UTC (permalink / raw)
To: robert.olsson; +Cc: netdev
(resend)
Any comments this time?
pktgen: proc_if_write: reduce stack usage (on i386)
from 776 to 296 bytes by combining/reusing locals.
Signed-off-by: Randy Dunlap <rddunlap@osdl.org>
diffstat:=
net/core/pktgen.c | 94 ++++++++++++++++++++++++------------------------------
1 files changed, 42 insertions(+), 52 deletions(-)
diff -Naurp ./net/core/pktgen.c~pktgen_stack ./net/core/pktgen.c
--- ./net/core/pktgen.c~pktgen_stack 2005-01-27 16:31:49.000000000 -0800
+++ ./net/core/pktgen.c 2005-01-30 19:07:55.252712936 -0800
@@ -811,6 +811,8 @@ static int proc_if_write(struct file *fi
struct pktgen_dev *pkt_dev = (struct pktgen_dev*)(data);
char* pg_result = NULL;
int tmp = 0;
+ char buf4[IP_NAME_SZ];
+ char buf6[128];
pg_result = &(pkt_dev->result[0]);
@@ -1071,16 +1073,15 @@ static int proc_if_write(struct file *fi
return count;
}
if (!strcmp(name, "dst_min") || !strcmp(name, "dst")) {
- char buf[IP_NAME_SZ];
len = strn_len(&user_buffer[i], sizeof(pkt_dev->dst_min) - 1);
if (len < 0) { return len; }
- if (copy_from_user(buf, &user_buffer[i], len))
+ if (copy_from_user(buf4, &user_buffer[i], len))
return -EFAULT;
- buf[len] = 0;
- if (strcmp(buf, pkt_dev->dst_min) != 0) {
+ buf4[len] = 0;
+ if (strcmp(buf4, pkt_dev->dst_min) != 0) {
memset(pkt_dev->dst_min, 0, sizeof(pkt_dev->dst_min));
- strncpy(pkt_dev->dst_min, buf, len);
+ strncpy(pkt_dev->dst_min, buf4, len);
pkt_dev->daddr_min = in_aton(pkt_dev->dst_min);
pkt_dev->cur_daddr = pkt_dev->daddr_min;
}
@@ -1091,17 +1092,16 @@ static int proc_if_write(struct file *fi
return count;
}
if (!strcmp(name, "dst_max")) {
- char buf[IP_NAME_SZ];
len = strn_len(&user_buffer[i], sizeof(pkt_dev->dst_max) - 1);
if (len < 0) { return len; }
- if (copy_from_user(buf, &user_buffer[i], len))
+ if (copy_from_user(buf4, &user_buffer[i], len))
return -EFAULT;
- buf[len] = 0;
- if (strcmp(buf, pkt_dev->dst_max) != 0) {
+ buf4[len] = 0;
+ if (strcmp(buf4, pkt_dev->dst_max) != 0) {
memset(pkt_dev->dst_max, 0, sizeof(pkt_dev->dst_max));
- strncpy(pkt_dev->dst_max, buf, len);
+ strncpy(pkt_dev->dst_max, buf4, len);
pkt_dev->daddr_max = in_aton(pkt_dev->dst_max);
pkt_dev->cur_daddr = pkt_dev->daddr_max;
}
@@ -1112,108 +1112,99 @@ static int proc_if_write(struct file *fi
return count;
}
if (!strcmp(name, "dst6")) {
- char buf[128];
-
len = strn_len(&user_buffer[i], 128 - 1);
if (len < 0) return len;
pkt_dev->flags |= F_IPV6;
- if (copy_from_user(buf, &user_buffer[i], len))
+ if (copy_from_user(buf6, &user_buffer[i], len))
return -EFAULT;
- buf[len] = 0;
+ buf6[len] = 0;
- scan_ip6(buf, pkt_dev->in6_daddr.s6_addr);
- fmt_ip6(buf, pkt_dev->in6_daddr.s6_addr);
+ scan_ip6(buf6, pkt_dev->in6_daddr.s6_addr);
+ fmt_ip6(buf6, pkt_dev->in6_daddr.s6_addr);
ipv6_addr_copy(&pkt_dev->cur_in6_daddr, &pkt_dev->in6_daddr);
if(debug)
- printk("pktgen: dst6 set to: %s\n", buf);
+ printk("pktgen: dst6 set to: %s\n", buf6);
i += len;
- sprintf(pg_result, "OK: dst6=%s", buf);
+ sprintf(pg_result, "OK: dst6=%s", buf6);
return count;
}
if (!strcmp(name, "dst6_min")) {
- char buf[128];
-
len = strn_len(&user_buffer[i], 128 - 1);
if (len < 0) return len;
pkt_dev->flags |= F_IPV6;
- if (copy_from_user(buf, &user_buffer[i], len))
+ if (copy_from_user(buf6, &user_buffer[i], len))
return -EFAULT;
- buf[len] = 0;
+ buf6[len] = 0;
- scan_ip6(buf, pkt_dev->min_in6_daddr.s6_addr);
- fmt_ip6(buf, pkt_dev->min_in6_daddr.s6_addr);
+ scan_ip6(buf6, pkt_dev->min_in6_daddr.s6_addr);
+ fmt_ip6(buf6, pkt_dev->min_in6_daddr.s6_addr);
ipv6_addr_copy(&pkt_dev->cur_in6_daddr, &pkt_dev->min_in6_daddr);
if(debug)
- printk("pktgen: dst6_min set to: %s\n", buf);
+ printk("pktgen: dst6_min set to: %s\n", buf6);
i += len;
- sprintf(pg_result, "OK: dst6_min=%s", buf);
+ sprintf(pg_result, "OK: dst6_min=%s", buf6);
return count;
}
if (!strcmp(name, "dst6_max")) {
- char buf[128];
-
len = strn_len(&user_buffer[i], 128 - 1);
if (len < 0) return len;
pkt_dev->flags |= F_IPV6;
- if (copy_from_user(buf, &user_buffer[i], len))
+ if (copy_from_user(buf6, &user_buffer[i], len))
return -EFAULT;
- buf[len] = 0;
+ buf6[len] = 0;
- scan_ip6(buf, pkt_dev->max_in6_daddr.s6_addr);
- fmt_ip6(buf, pkt_dev->max_in6_daddr.s6_addr);
+ scan_ip6(buf6, pkt_dev->max_in6_daddr.s6_addr);
+ fmt_ip6(buf6, pkt_dev->max_in6_daddr.s6_addr);
if(debug)
- printk("pktgen: dst6_max set to: %s\n", buf);
+ printk("pktgen: dst6_max set to: %s\n", buf6);
i += len;
- sprintf(pg_result, "OK: dst6_max=%s", buf);
+ sprintf(pg_result, "OK: dst6_max=%s", buf6);
return count;
}
if (!strcmp(name, "src6")) {
- char buf[128];
-
len = strn_len(&user_buffer[i], 128 - 1);
if (len < 0) return len;
pkt_dev->flags |= F_IPV6;
- if (copy_from_user(buf, &user_buffer[i], len))
+ if (copy_from_user(buf6, &user_buffer[i], len))
return -EFAULT;
- buf[len] = 0;
+ buf6[len] = 0;
- scan_ip6(buf, pkt_dev->in6_saddr.s6_addr);
- fmt_ip6(buf, pkt_dev->in6_saddr.s6_addr);
+ scan_ip6(buf6, pkt_dev->in6_saddr.s6_addr);
+ fmt_ip6(buf6, pkt_dev->in6_saddr.s6_addr);
ipv6_addr_copy(&pkt_dev->cur_in6_saddr, &pkt_dev->in6_saddr);
if(debug)
- printk("pktgen: src6 set to: %s\n", buf);
+ printk("pktgen: src6 set to: %s\n", buf6);
i += len;
- sprintf(pg_result, "OK: src6=%s", buf);
+ sprintf(pg_result, "OK: src6=%s", buf6);
return count;
}
if (!strcmp(name, "src_min")) {
- char buf[IP_NAME_SZ];
len = strn_len(&user_buffer[i], sizeof(pkt_dev->src_min) - 1);
if (len < 0) { return len; }
- if (copy_from_user(buf, &user_buffer[i], len))
+ if (copy_from_user(buf4, &user_buffer[i], len))
return -EFAULT;
- buf[len] = 0;
- if (strcmp(buf, pkt_dev->src_min) != 0) {
+ buf4[len] = 0;
+ if (strcmp(buf4, pkt_dev->src_min) != 0) {
memset(pkt_dev->src_min, 0, sizeof(pkt_dev->src_min));
- strncpy(pkt_dev->src_min, buf, len);
+ strncpy(pkt_dev->src_min, buf4, len);
pkt_dev->saddr_min = in_aton(pkt_dev->src_min);
pkt_dev->cur_saddr = pkt_dev->saddr_min;
}
@@ -1224,15 +1215,14 @@ static int proc_if_write(struct file *fi
return count;
}
if (!strcmp(name, "src_max")) {
- char buf[IP_NAME_SZ];
len = strn_len(&user_buffer[i], sizeof(pkt_dev->src_max) - 1);
if (len < 0) { return len; }
- if (copy_from_user(buf, &user_buffer[i], len))
+ if (copy_from_user(buf4, &user_buffer[i], len))
return -EFAULT;
- buf[len] = 0;
- if (strcmp(buf, pkt_dev->src_max) != 0) {
+ buf4[len] = 0;
+ if (strcmp(buf4, pkt_dev->src_max) != 0) {
memset(pkt_dev->src_max, 0, sizeof(pkt_dev->src_max));
- strncpy(pkt_dev->src_max, buf, len);
+ strncpy(pkt_dev->src_max, buf4, len);
pkt_dev->saddr_max = in_aton(pkt_dev->src_max);
pkt_dev->cur_saddr = pkt_dev->saddr_max;
}
---
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] pktgen: reduce stack usage
2005-02-18 21:42 [PATCH] pktgen: reduce stack usage Randy.Dunlap
@ 2005-02-18 22:11 ` Francois Romieu
2005-02-18 22:20 ` Randy.Dunlap
2005-02-18 22:37 ` Randy.Dunlap
2005-02-19 2:56 ` Ben Greear
1 sibling, 2 replies; 10+ messages in thread
From: Francois Romieu @ 2005-02-18 22:11 UTC (permalink / raw)
To: Randy.Dunlap; +Cc: robert.olsson, netdev
Randy.Dunlap <rddunlap@osdl.org> :
> (resend)
> Any comments this time?
Did I miss a simultaneous use of both buffers or could it be possible to
save an extra IP_NAME_SZ bytes of data with an evil ugly union ?
Btw, does anyone intend to fix the use of sleepable functions in
proc_thread_write() with spinlock ("thread_lock") held or should
I allocate it some cycles ?
--
Ueimor
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] pktgen: reduce stack usage
2005-02-18 22:11 ` Francois Romieu
@ 2005-02-18 22:20 ` Randy.Dunlap
2005-02-18 22:37 ` Randy.Dunlap
1 sibling, 0 replies; 10+ messages in thread
From: Randy.Dunlap @ 2005-02-18 22:20 UTC (permalink / raw)
To: Francois Romieu; +Cc: robert.olsson, netdev
Francois Romieu wrote:
> Randy.Dunlap <rddunlap@osdl.org> :
>
>>(resend)
>>Any comments this time?
>
>
> Did I miss a simultaneous use of both buffers or could it be possible to
> save an extra IP_NAME_SZ bytes of data with an evil ugly union ?
That's probably doable, I'll double-check it and modify the patch...
> Btw, does anyone intend to fix the use of sleepable functions in
> proc_thread_write() with spinlock ("thread_lock") held or should
> I allocate it some cycles ?
--
~Randy
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] pktgen: reduce stack usage
2005-02-18 22:11 ` Francois Romieu
2005-02-18 22:20 ` Randy.Dunlap
@ 2005-02-18 22:37 ` Randy.Dunlap
2005-02-21 14:13 ` Robert Olsson
1 sibling, 1 reply; 10+ messages in thread
From: Randy.Dunlap @ 2005-02-18 22:37 UTC (permalink / raw)
To: Francois Romieu; +Cc: robert.olsson, netdev
[-- Attachment #1: Type: text/plain, Size: 651 bytes --]
Francois Romieu wrote:
> Randy.Dunlap <rddunlap@osdl.org> :
>
>>(resend)
>>Any comments this time?
>
>
> Did I miss a simultaneous use of both buffers or could it be possible to
> save an extra IP_NAME_SZ bytes of data with an evil ugly union ?
I don't see any simultaneous uses of the 2 buffers, so here's the
union version of the patch (attached this time), although it only
saves 4 bytes... so maybe the compiler had already realized that
usage. Either one accomplishes a large stack reduction, but the
first one is slightly more readable & maintainable (to me),
and less error-prone (or more future-proof) (again, to me).
Thanks,
--
~Randy
[-- Attachment #2: pktgen_stack_v2.patch --]
[-- Type: text/x-patch, Size: 8484 bytes --]
pktgen: proc_if_write: reduce stack usage (on i386)
from 776 to 292 bytes by combining/reusing locals.
Signed-off-by: Randy Dunlap <rddunlap@osdl.org>
diffstat:=
net/core/pktgen.c | 96 ++++++++++++++++++++++++------------------------------
1 files changed, 44 insertions(+), 52 deletions(-)
diff -Naurp ./net/core/pktgen.c~pktgen_stack ./net/core/pktgen.c
--- ./net/core/pktgen.c~pktgen_stack 2005-02-18 13:39:12.309983016 -0800
+++ ./net/core/pktgen.c 2005-02-18 14:28:54.007696064 -0800
@@ -811,6 +811,10 @@ static int proc_if_write(struct file *fi
struct pktgen_dev *pkt_dev = (struct pktgen_dev*)(data);
char* pg_result = NULL;
int tmp = 0;
+ union bufs {
+ char buf4[IP_NAME_SZ];
+ char buf6[128];
+ } bu;
pg_result = &(pkt_dev->result[0]);
@@ -1071,16 +1075,15 @@ static int proc_if_write(struct file *fi
return count;
}
if (!strcmp(name, "dst_min") || !strcmp(name, "dst")) {
- char buf[IP_NAME_SZ];
len = strn_len(&user_buffer[i], sizeof(pkt_dev->dst_min) - 1);
if (len < 0) { return len; }
- if (copy_from_user(buf, &user_buffer[i], len))
+ if (copy_from_user(bu.buf4, &user_buffer[i], len))
return -EFAULT;
- buf[len] = 0;
- if (strcmp(buf, pkt_dev->dst_min) != 0) {
+ bu.buf4[len] = 0;
+ if (strcmp(bu.buf4, pkt_dev->dst_min) != 0) {
memset(pkt_dev->dst_min, 0, sizeof(pkt_dev->dst_min));
- strncpy(pkt_dev->dst_min, buf, len);
+ strncpy(pkt_dev->dst_min, bu.buf4, len);
pkt_dev->daddr_min = in_aton(pkt_dev->dst_min);
pkt_dev->cur_daddr = pkt_dev->daddr_min;
}
@@ -1091,17 +1094,16 @@ static int proc_if_write(struct file *fi
return count;
}
if (!strcmp(name, "dst_max")) {
- char buf[IP_NAME_SZ];
len = strn_len(&user_buffer[i], sizeof(pkt_dev->dst_max) - 1);
if (len < 0) { return len; }
- if (copy_from_user(buf, &user_buffer[i], len))
+ if (copy_from_user(bu.buf4, &user_buffer[i], len))
return -EFAULT;
- buf[len] = 0;
- if (strcmp(buf, pkt_dev->dst_max) != 0) {
+ bu.buf4[len] = 0;
+ if (strcmp(bu.buf4, pkt_dev->dst_max) != 0) {
memset(pkt_dev->dst_max, 0, sizeof(pkt_dev->dst_max));
- strncpy(pkt_dev->dst_max, buf, len);
+ strncpy(pkt_dev->dst_max, bu.buf4, len);
pkt_dev->daddr_max = in_aton(pkt_dev->dst_max);
pkt_dev->cur_daddr = pkt_dev->daddr_max;
}
@@ -1112,108 +1114,99 @@ static int proc_if_write(struct file *fi
return count;
}
if (!strcmp(name, "dst6")) {
- char buf[128];
-
len = strn_len(&user_buffer[i], 128 - 1);
if (len < 0) return len;
pkt_dev->flags |= F_IPV6;
- if (copy_from_user(buf, &user_buffer[i], len))
+ if (copy_from_user(bu.buf6, &user_buffer[i], len))
return -EFAULT;
- buf[len] = 0;
+ bu.buf6[len] = 0;
- scan_ip6(buf, pkt_dev->in6_daddr.s6_addr);
- fmt_ip6(buf, pkt_dev->in6_daddr.s6_addr);
+ scan_ip6(bu.buf6, pkt_dev->in6_daddr.s6_addr);
+ fmt_ip6(bu.buf6, pkt_dev->in6_daddr.s6_addr);
ipv6_addr_copy(&pkt_dev->cur_in6_daddr, &pkt_dev->in6_daddr);
if(debug)
- printk("pktgen: dst6 set to: %s\n", buf);
+ printk("pktgen: dst6 set to: %s\n", bu.buf6);
i += len;
- sprintf(pg_result, "OK: dst6=%s", buf);
+ sprintf(pg_result, "OK: dst6=%s", bu.buf6);
return count;
}
if (!strcmp(name, "dst6_min")) {
- char buf[128];
-
len = strn_len(&user_buffer[i], 128 - 1);
if (len < 0) return len;
pkt_dev->flags |= F_IPV6;
- if (copy_from_user(buf, &user_buffer[i], len))
+ if (copy_from_user(bu.buf6, &user_buffer[i], len))
return -EFAULT;
- buf[len] = 0;
+ bu.buf6[len] = 0;
- scan_ip6(buf, pkt_dev->min_in6_daddr.s6_addr);
- fmt_ip6(buf, pkt_dev->min_in6_daddr.s6_addr);
+ scan_ip6(bu.buf6, pkt_dev->min_in6_daddr.s6_addr);
+ fmt_ip6(bu.buf6, pkt_dev->min_in6_daddr.s6_addr);
ipv6_addr_copy(&pkt_dev->cur_in6_daddr, &pkt_dev->min_in6_daddr);
if(debug)
- printk("pktgen: dst6_min set to: %s\n", buf);
+ printk("pktgen: dst6_min set to: %s\n", bu.buf6);
i += len;
- sprintf(pg_result, "OK: dst6_min=%s", buf);
+ sprintf(pg_result, "OK: dst6_min=%s", bu.buf6);
return count;
}
if (!strcmp(name, "dst6_max")) {
- char buf[128];
-
len = strn_len(&user_buffer[i], 128 - 1);
if (len < 0) return len;
pkt_dev->flags |= F_IPV6;
- if (copy_from_user(buf, &user_buffer[i], len))
+ if (copy_from_user(bu.buf6, &user_buffer[i], len))
return -EFAULT;
- buf[len] = 0;
+ bu.buf6[len] = 0;
- scan_ip6(buf, pkt_dev->max_in6_daddr.s6_addr);
- fmt_ip6(buf, pkt_dev->max_in6_daddr.s6_addr);
+ scan_ip6(bu.buf6, pkt_dev->max_in6_daddr.s6_addr);
+ fmt_ip6(bu.buf6, pkt_dev->max_in6_daddr.s6_addr);
if(debug)
- printk("pktgen: dst6_max set to: %s\n", buf);
+ printk("pktgen: dst6_max set to: %s\n", bu.buf6);
i += len;
- sprintf(pg_result, "OK: dst6_max=%s", buf);
+ sprintf(pg_result, "OK: dst6_max=%s", bu.buf6);
return count;
}
if (!strcmp(name, "src6")) {
- char buf[128];
-
len = strn_len(&user_buffer[i], 128 - 1);
if (len < 0) return len;
pkt_dev->flags |= F_IPV6;
- if (copy_from_user(buf, &user_buffer[i], len))
+ if (copy_from_user(bu.buf6, &user_buffer[i], len))
return -EFAULT;
- buf[len] = 0;
+ bu.buf6[len] = 0;
- scan_ip6(buf, pkt_dev->in6_saddr.s6_addr);
- fmt_ip6(buf, pkt_dev->in6_saddr.s6_addr);
+ scan_ip6(bu.buf6, pkt_dev->in6_saddr.s6_addr);
+ fmt_ip6(bu.buf6, pkt_dev->in6_saddr.s6_addr);
ipv6_addr_copy(&pkt_dev->cur_in6_saddr, &pkt_dev->in6_saddr);
if(debug)
- printk("pktgen: src6 set to: %s\n", buf);
+ printk("pktgen: src6 set to: %s\n", bu.buf6);
i += len;
- sprintf(pg_result, "OK: src6=%s", buf);
+ sprintf(pg_result, "OK: src6=%s", bu.buf6);
return count;
}
if (!strcmp(name, "src_min")) {
- char buf[IP_NAME_SZ];
len = strn_len(&user_buffer[i], sizeof(pkt_dev->src_min) - 1);
if (len < 0) { return len; }
- if (copy_from_user(buf, &user_buffer[i], len))
+ if (copy_from_user(bu.buf4, &user_buffer[i], len))
return -EFAULT;
- buf[len] = 0;
- if (strcmp(buf, pkt_dev->src_min) != 0) {
+ bu.buf4[len] = 0;
+ if (strcmp(bu.buf4, pkt_dev->src_min) != 0) {
memset(pkt_dev->src_min, 0, sizeof(pkt_dev->src_min));
- strncpy(pkt_dev->src_min, buf, len);
+ strncpy(pkt_dev->src_min, bu.buf4, len);
pkt_dev->saddr_min = in_aton(pkt_dev->src_min);
pkt_dev->cur_saddr = pkt_dev->saddr_min;
}
@@ -1224,15 +1217,14 @@ static int proc_if_write(struct file *fi
return count;
}
if (!strcmp(name, "src_max")) {
- char buf[IP_NAME_SZ];
len = strn_len(&user_buffer[i], sizeof(pkt_dev->src_max) - 1);
if (len < 0) { return len; }
- if (copy_from_user(buf, &user_buffer[i], len))
+ if (copy_from_user(bu.buf4, &user_buffer[i], len))
return -EFAULT;
- buf[len] = 0;
- if (strcmp(buf, pkt_dev->src_max) != 0) {
+ bu.buf4[len] = 0;
+ if (strcmp(bu.buf4, pkt_dev->src_max) != 0) {
memset(pkt_dev->src_max, 0, sizeof(pkt_dev->src_max));
- strncpy(pkt_dev->src_max, buf, len);
+ strncpy(pkt_dev->src_max, bu.buf4, len);
pkt_dev->saddr_max = in_aton(pkt_dev->src_max);
pkt_dev->cur_saddr = pkt_dev->saddr_max;
}
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] pktgen: reduce stack usage
2005-02-18 22:37 ` Randy.Dunlap
@ 2005-02-21 14:13 ` Robert Olsson
2005-02-21 16:12 ` Randy.Dunlap
0 siblings, 1 reply; 10+ messages in thread
From: Robert Olsson @ 2005-02-21 14:13 UTC (permalink / raw)
To: Randy.Dunlap; +Cc: Francois Romieu, robert.olsson, netdev
Randy.Dunlap writes:
> I don't see any simultaneous uses of the 2 buffers, so here's the
> union version of the patch (attached this time), although it only
> saves 4 bytes... so maybe the compiler had already realized that
> usage. Either one accomplishes a large stack reduction, but the
Hello!
Here is version with just one buffer. How did you calculate the saving?
--ro
--- net/core/pktgen.c.050221 2005-02-21 14:02:40.000000000 +0100
+++ net/core/pktgen.c 2005-02-21 15:02:44.000000000 +0100
@@ -151,7 +151,7 @@
#include <asm/timex.h>
-#define VERSION "pktgen v2.57: Packet Generator for packet performance testing.\n"
+#define VERSION "pktgen v2.58: Packet Generator for packet performance testing.\n"
/* #define PG_DEBUG(a) a */
#define PG_DEBUG(a)
@@ -811,6 +811,7 @@
struct pktgen_dev *pkt_dev = (struct pktgen_dev*)(data);
char* pg_result = NULL;
int tmp = 0;
+ char buf[128];
pg_result = &(pkt_dev->result[0]);
@@ -1071,7 +1072,6 @@
return count;
}
if (!strcmp(name, "dst_min") || !strcmp(name, "dst")) {
- char buf[IP_NAME_SZ];
len = strn_len(&user_buffer[i], sizeof(pkt_dev->dst_min) - 1);
if (len < 0) { return len; }
@@ -1091,7 +1091,6 @@
return count;
}
if (!strcmp(name, "dst_max")) {
- char buf[IP_NAME_SZ];
len = strn_len(&user_buffer[i], sizeof(pkt_dev->dst_max) - 1);
if (len < 0) { return len; }
@@ -1112,9 +1111,7 @@
return count;
}
if (!strcmp(name, "dst6")) {
- char buf[128];
-
- len = strn_len(&user_buffer[i], 128 - 1);
+ len = strn_len(&user_buffer[i], sizeof(buf) - 1);
if (len < 0) return len;
pkt_dev->flags |= F_IPV6;
@@ -1136,9 +1133,7 @@
return count;
}
if (!strcmp(name, "dst6_min")) {
- char buf[128];
-
- len = strn_len(&user_buffer[i], 128 - 1);
+ len = strn_len(&user_buffer[i], sizeof(buf) - 1);
if (len < 0) return len;
pkt_dev->flags |= F_IPV6;
@@ -1159,9 +1154,7 @@
return count;
}
if (!strcmp(name, "dst6_max")) {
- char buf[128];
-
- len = strn_len(&user_buffer[i], 128 - 1);
+ len = strn_len(&user_buffer[i], sizeof(buf) - 1);
if (len < 0) return len;
pkt_dev->flags |= F_IPV6;
@@ -1181,9 +1174,7 @@
return count;
}
if (!strcmp(name, "src6")) {
- char buf[128];
-
- len = strn_len(&user_buffer[i], 128 - 1);
+ len = strn_len(&user_buffer[i], sizeof(buf) - 1);
if (len < 0) return len;
pkt_dev->flags |= F_IPV6;
@@ -1205,7 +1196,6 @@
return count;
}
if (!strcmp(name, "src_min")) {
- char buf[IP_NAME_SZ];
len = strn_len(&user_buffer[i], sizeof(pkt_dev->src_min) - 1);
if (len < 0) { return len; }
if (copy_from_user(buf, &user_buffer[i], len))
@@ -1224,7 +1214,6 @@
return count;
}
if (!strcmp(name, "src_max")) {
- char buf[IP_NAME_SZ];
len = strn_len(&user_buffer[i], sizeof(pkt_dev->src_max) - 1);
if (len < 0) { return len; }
if (copy_from_user(buf, &user_buffer[i], len))
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] pktgen: reduce stack usage
2005-02-21 14:13 ` Robert Olsson
@ 2005-02-21 16:12 ` Randy.Dunlap
2005-02-22 15:41 ` Robert Olsson
0 siblings, 1 reply; 10+ messages in thread
From: Randy.Dunlap @ 2005-02-21 16:12 UTC (permalink / raw)
To: Robert Olsson; +Cc: Francois Romieu, robert.olsson, netdev
Robert Olsson wrote:
> Randy.Dunlap writes:
>
> > I don't see any simultaneous uses of the 2 buffers, so here's the
> > union version of the patch (attached this time), although it only
> > saves 4 bytes... so maybe the compiler had already realized that
> > usage. Either one accomplishes a large stack reduction, but the
>
> Hello!
>
> Here is version with just one buffer.
Yep, even better: few changes + using sizeof.
> How did you calculate the saving?
objdump the .o file before and after changes. Look at how much
temp space is allocated by "sub const,%esp" (on x86). E.g.:
9e8: 81 ec 24 01 00 00 sub $0x124,%esp
> --ro
>
>
>
> --- net/core/pktgen.c.050221 2005-02-21 14:02:40.000000000 +0100
> +++ net/core/pktgen.c 2005-02-21 15:02:44.000000000 +0100
> @@ -151,7 +151,7 @@
> #include <asm/timex.h>
>
>
> -#define VERSION "pktgen v2.57: Packet Generator for packet performance testing.\n"
> +#define VERSION "pktgen v2.58: Packet Generator for packet performance testing.\n"
>
> /* #define PG_DEBUG(a) a */
> #define PG_DEBUG(a)
> @@ -811,6 +811,7 @@
> struct pktgen_dev *pkt_dev = (struct pktgen_dev*)(data);
> char* pg_result = NULL;
> int tmp = 0;
> + char buf[128];
>
> pg_result = &(pkt_dev->result[0]);
>
> @@ -1071,7 +1072,6 @@
> return count;
> }
> if (!strcmp(name, "dst_min") || !strcmp(name, "dst")) {
> - char buf[IP_NAME_SZ];
> len = strn_len(&user_buffer[i], sizeof(pkt_dev->dst_min) - 1);
> if (len < 0) { return len; }
>
> @@ -1091,7 +1091,6 @@
> return count;
> }
> if (!strcmp(name, "dst_max")) {
> - char buf[IP_NAME_SZ];
> len = strn_len(&user_buffer[i], sizeof(pkt_dev->dst_max) - 1);
> if (len < 0) { return len; }
>
> @@ -1112,9 +1111,7 @@
> return count;
> }
> if (!strcmp(name, "dst6")) {
> - char buf[128];
> -
> - len = strn_len(&user_buffer[i], 128 - 1);
> + len = strn_len(&user_buffer[i], sizeof(buf) - 1);
> if (len < 0) return len;
>
> pkt_dev->flags |= F_IPV6;
> @@ -1136,9 +1133,7 @@
> return count;
> }
> if (!strcmp(name, "dst6_min")) {
> - char buf[128];
> -
> - len = strn_len(&user_buffer[i], 128 - 1);
> + len = strn_len(&user_buffer[i], sizeof(buf) - 1);
> if (len < 0) return len;
>
> pkt_dev->flags |= F_IPV6;
> @@ -1159,9 +1154,7 @@
> return count;
> }
> if (!strcmp(name, "dst6_max")) {
> - char buf[128];
> -
> - len = strn_len(&user_buffer[i], 128 - 1);
> + len = strn_len(&user_buffer[i], sizeof(buf) - 1);
> if (len < 0) return len;
>
> pkt_dev->flags |= F_IPV6;
> @@ -1181,9 +1174,7 @@
> return count;
> }
> if (!strcmp(name, "src6")) {
> - char buf[128];
> -
> - len = strn_len(&user_buffer[i], 128 - 1);
> + len = strn_len(&user_buffer[i], sizeof(buf) - 1);
> if (len < 0) return len;
>
> pkt_dev->flags |= F_IPV6;
> @@ -1205,7 +1196,6 @@
> return count;
> }
> if (!strcmp(name, "src_min")) {
> - char buf[IP_NAME_SZ];
> len = strn_len(&user_buffer[i], sizeof(pkt_dev->src_min) - 1);
> if (len < 0) { return len; }
> if (copy_from_user(buf, &user_buffer[i], len))
> @@ -1224,7 +1214,6 @@
> return count;
> }
> if (!strcmp(name, "src_max")) {
> - char buf[IP_NAME_SZ];
> len = strn_len(&user_buffer[i], sizeof(pkt_dev->src_max) - 1);
> if (len < 0) { return len; }
> if (copy_from_user(buf, &user_buffer[i], len))
--
~Randy
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] pktgen: reduce stack usage
2005-02-21 16:12 ` Randy.Dunlap
@ 2005-02-22 15:41 ` Robert Olsson
2005-02-24 4:31 ` David S. Miller
0 siblings, 1 reply; 10+ messages in thread
From: Robert Olsson @ 2005-02-22 15:41 UTC (permalink / raw)
To: davem; +Cc: Randy.Dunlap, Robert Olsson, Francois Romieu, netdev
Randy.Dunlap writes:
> > Here is version with just one buffer.
>
> Yep, even better: few changes + using sizeof.
Hello.
OK! Then we ask DaveM to apply the patch.
--ro
--- net/core/pktgen.c.050221 2005-02-21 14:02:40.000000000 +0100
+++ net/core/pktgen.c 2005-02-21 15:02:44.000000000 +0100
@@ -151,7 +151,7 @@
#include <asm/timex.h>
-#define VERSION "pktgen v2.57: Packet Generator for packet performance testing.\n"
+#define VERSION "pktgen v2.58: Packet Generator for packet performance testing.\n"
/* #define PG_DEBUG(a) a */
#define PG_DEBUG(a)
@@ -811,6 +811,7 @@
struct pktgen_dev *pkt_dev = (struct pktgen_dev*)(data);
char* pg_result = NULL;
int tmp = 0;
+ char buf[128];
pg_result = &(pkt_dev->result[0]);
@@ -1071,7 +1072,6 @@
return count;
}
if (!strcmp(name, "dst_min") || !strcmp(name, "dst")) {
- char buf[IP_NAME_SZ];
len = strn_len(&user_buffer[i], sizeof(pkt_dev->dst_min) - 1);
if (len < 0) { return len; }
@@ -1091,7 +1091,6 @@
return count;
}
if (!strcmp(name, "dst_max")) {
- char buf[IP_NAME_SZ];
len = strn_len(&user_buffer[i], sizeof(pkt_dev->dst_max) - 1);
if (len < 0) { return len; }
@@ -1112,9 +1111,7 @@
return count;
}
if (!strcmp(name, "dst6")) {
- char buf[128];
-
- len = strn_len(&user_buffer[i], 128 - 1);
+ len = strn_len(&user_buffer[i], sizeof(buf) - 1);
if (len < 0) return len;
pkt_dev->flags |= F_IPV6;
@@ -1136,9 +1133,7 @@
return count;
}
if (!strcmp(name, "dst6_min")) {
- char buf[128];
-
- len = strn_len(&user_buffer[i], 128 - 1);
+ len = strn_len(&user_buffer[i], sizeof(buf) - 1);
if (len < 0) return len;
pkt_dev->flags |= F_IPV6;
@@ -1159,9 +1154,7 @@
return count;
}
if (!strcmp(name, "dst6_max")) {
- char buf[128];
-
- len = strn_len(&user_buffer[i], 128 - 1);
+ len = strn_len(&user_buffer[i], sizeof(buf) - 1);
if (len < 0) return len;
pkt_dev->flags |= F_IPV6;
@@ -1181,9 +1174,7 @@
return count;
}
if (!strcmp(name, "src6")) {
- char buf[128];
-
- len = strn_len(&user_buffer[i], 128 - 1);
+ len = strn_len(&user_buffer[i], sizeof(buf) - 1);
if (len < 0) return len;
pkt_dev->flags |= F_IPV6;
@@ -1205,7 +1196,6 @@
return count;
}
if (!strcmp(name, "src_min")) {
- char buf[IP_NAME_SZ];
len = strn_len(&user_buffer[i], sizeof(pkt_dev->src_min) - 1);
if (len < 0) { return len; }
if (copy_from_user(buf, &user_buffer[i], len))
@@ -1224,7 +1214,6 @@
return count;
}
if (!strcmp(name, "src_max")) {
- char buf[IP_NAME_SZ];
len = strn_len(&user_buffer[i], sizeof(pkt_dev->src_max) - 1);
if (len < 0) { return len; }
if (copy_from_user(buf, &user_buffer[i], len))
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] pktgen: reduce stack usage
2005-02-22 15:41 ` Robert Olsson
@ 2005-02-24 4:31 ` David S. Miller
0 siblings, 0 replies; 10+ messages in thread
From: David S. Miller @ 2005-02-24 4:31 UTC (permalink / raw)
To: Robert Olsson; +Cc: rddunlap, Robert.Olsson, romieu, netdev
On Tue, 22 Feb 2005 16:41:31 +0100
Robert Olsson <Robert.Olsson@data.slu.se> wrote:
> Randy.Dunlap writes:
> > > Here is version with just one buffer.
> >
> > Yep, even better: few changes + using sizeof.
>
> Hello.
>
> OK! Then we ask DaveM to apply the patch.
Applied, thanks guys.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] pktgen: reduce stack usage
2005-02-18 21:42 [PATCH] pktgen: reduce stack usage Randy.Dunlap
2005-02-18 22:11 ` Francois Romieu
@ 2005-02-19 2:56 ` Ben Greear
2005-02-21 16:27 ` Randy.Dunlap
1 sibling, 1 reply; 10+ messages in thread
From: Ben Greear @ 2005-02-19 2:56 UTC (permalink / raw)
To: Randy.Dunlap; +Cc: robert.olsson, netdev
Randy.Dunlap wrote:
> (resend)
> Any comments this time?
>
>
>
> pktgen: proc_if_write: reduce stack usage (on i386)
> from 776 to 296 bytes by combining/reusing locals.
Since these methods are not in the fast path, and the stack
usage is not near 4k, does this really matter?
I'm asking more to be educated than to dismiss the changes....
Thanks,
Ben
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] pktgen: reduce stack usage
2005-02-19 2:56 ` Ben Greear
@ 2005-02-21 16:27 ` Randy.Dunlap
0 siblings, 0 replies; 10+ messages in thread
From: Randy.Dunlap @ 2005-02-21 16:27 UTC (permalink / raw)
To: Ben Greear; +Cc: robert.olsson, netdev
Ben Greear wrote:
> Randy.Dunlap wrote:
>
>> (resend)
>> Any comments this time?
>>
>> pktgen: proc_if_write: reduce stack usage (on i386)
>> from 776 to 296 bytes by combining/reusing locals.
>
> Since these methods are not in the fast path, and the stack
> usage is not near 4k, does this really matter?
It's not critical or near causing a stack overflow AFAICT.
And ideally gcc would recognize this kind of usage and not
allocate multiple stack entries for it.
syscall: sys_write (tiny stack usage)
-> vfs_write (tiny)
-> proc_file_write (tiny)
-> proc_if_write (pktgen: large)
> I'm asking more to be educated than to dismiss the changes....
--
~Randy
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2005-02-24 4:31 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2005-02-18 21:42 [PATCH] pktgen: reduce stack usage Randy.Dunlap
2005-02-18 22:11 ` Francois Romieu
2005-02-18 22:20 ` Randy.Dunlap
2005-02-18 22:37 ` Randy.Dunlap
2005-02-21 14:13 ` Robert Olsson
2005-02-21 16:12 ` Randy.Dunlap
2005-02-22 15:41 ` Robert Olsson
2005-02-24 4:31 ` David S. Miller
2005-02-19 2:56 ` Ben Greear
2005-02-21 16:27 ` Randy.Dunlap
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).