public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] deflate stack usage in lib/inflate.c
@ 2007-04-12  8:25 Jeremy Fitzhardinge
  2007-04-12 20:50 ` [PATCH UPDATE] " Jeremy Fitzhardinge
  0 siblings, 1 reply; 16+ messages in thread
From: Jeremy Fitzhardinge @ 2007-04-12  8:25 UTC (permalink / raw)
  To: Andrew Morton; +Cc: Tim Yamin, Linux Kernel Mailing List

inflate_fixed() and huft_build() together use around 2.7k of stack.  When
using 4k stacks, I saw stack overflows from interrupts arriving while
unpacking the root initrd:

do_IRQ: stack overflow: 384
 [<c0106b64>] show_trace_log_lvl+0x1a/0x30
 [<c01075e6>] show_trace+0x12/0x14
 [<c010763f>] dump_stack+0x16/0x18
 [<c0107ca4>] do_IRQ+0x6d/0xd9
 [<c010202b>] xen_evtchn_do_upcall+0x6e/0xa2
 [<c0106781>] xen_hypervisor_callback+0x25/0x2c
 [<c010116c>] xen_restore_fl+0x27/0x29
 [<c0330f63>] _spin_unlock_irqrestore+0x4a/0x50
 [<c0117aab>] change_page_attr+0x577/0x584
 [<c0117b45>] kernel_map_pages+0x8d/0xb4
 [<c016a314>] cache_alloc_refill+0x53f/0x632
 [<c016a6c2>] __kmalloc+0xc1/0x10d
 [<c0463d34>] malloc+0x10/0x12
 [<c04641c1>] huft_build+0x2a7/0x5fa
 [<c04645a5>] inflate_fixed+0x91/0x136
 [<c04657e2>] unpack_to_rootfs+0x5f2/0x8c1
 [<c0465acf>] populate_rootfs+0x1e/0xe4

(This was under Xen, but there's no reason it couldn't happen on bare
  hardware.)

This patch mallocs the local variables, thereby reducing the stack
usage to sane levels.

Signed-off-by: Jeremy Fitzhardinge <jeremy@xensource.com>
Cc: Tim Yamin <plasmaroo@gentoo.org>

diff -r 4a30275bd596 lib/inflate.c
--- a/lib/inflate.c	Wed Apr 11 17:28:10 2007 -0700
+++ b/lib/inflate.c	Thu Apr 12 00:45:35 2007 -0700
@@ -292,7 +292,6 @@ STATIC int INIT huft_build(
    oversubscribed set of lengths), and three if not enough memory. */
 {
   unsigned a;                   /* counter for codes of length k */
-  unsigned c[BMAX+1];           /* bit length count table */
   unsigned f;                   /* i repeats in table every f entries */
   int g;                        /* maximum code length */
   int h;                        /* table level */
@@ -303,18 +302,33 @@ STATIC int INIT huft_build(
   register unsigned *p;         /* pointer into c[], b[], or v[] */
   register struct huft *q;      /* points to current table */
   struct huft r;                /* table entry for structure assignment */
-  struct huft *u[BMAX];         /* table stack */
-  unsigned v[N_MAX];            /* values in order of bit length */
   register int w;               /* bits before this table == (l * h) */
-  unsigned x[BMAX+1];           /* bit offsets, then code stack */
   unsigned *xp;                 /* pointer into x */
   int y;                        /* number of dummy codes added */
   unsigned z;                   /* number of entries in current table */
+  struct {
+    unsigned c[BMAX+1];           /* bit length count table */
+    struct huft *u[BMAX];         /* table stack */
+    unsigned v[N_MAX];            /* values in order of bit length */
+    unsigned x[BMAX+1];           /* bit offsets, then code stack */
+  } *stk;    
+  unsigned *c, *v, *x;
+  struct huft **u;
+  int ret;
 
 DEBG("huft1 ");
 
+  stk = malloc(sizeof(*stk));
+  if (stk == NULL)
+    return 3;			/* out of memory */
+
+  c = stk->c;
+  v = stk->v;
+  x = stk->x;
+  u = stk->u;
+
   /* Generate counts for each bit length */
-  memzero(c, sizeof(c));
+  memzero(stk->c, sizeof(stk->c));
   p = b;  i = n;
   do {
     Tracecv(*p, (stderr, (n-i >= ' ' && n-i <= '~' ? "%c %d\n" : "0x%x %d\n"), 
@@ -326,7 +340,8 @@ DEBG("huft1 ");
   {
     *t = (struct huft *)NULL;
     *m = 0;
-    return 2;
+    ret = 2;
+    goto out;
   }
 
 DEBG("huft2 ");
@@ -351,10 +366,14 @@ DEBG("huft3 ");
 
   /* Adjust last length count to fill out codes, if needed */
   for (y = 1 << j; j < i; j++, y <<= 1)
-    if ((y -= c[j]) < 0)
-      return 2;                 /* bad input: more codes than bits */
-  if ((y -= c[i]) < 0)
-    return 2;
+    if ((y -= c[j]) < 0) {
+      ret = 2;                 /* bad input: more codes than bits */
+      goto out;
+    }
+  if ((y -= c[i]) < 0) {
+    ret = 2;
+    goto out;
+  }
   c[i] += y;
 
 DEBG("huft4 ");
@@ -428,7 +447,8 @@ DEBG1("3 ");
         {
           if (h)
             huft_free(u[0]);
-          return 3;             /* not enough memory */
+          ret = 3;             /* not enough memory */
+	  goto out;
         }
 DEBG1("4 ");
         hufts += z + 1;         /* track memory usage */
@@ -492,7 +512,11 @@ DEBG("huft7 ");
 DEBG("huft7 ");
 
   /* Return true (1) if we were given an incomplete table */
-  return y != 0 && g != 1;
+  ret = y != 0 && g != 1;
+
+  out:
+  free(stk);
+  return ret;
 }
 
 
@@ -705,9 +729,13 @@ STATIC int noinline INIT inflate_fixed(v
   struct huft *td;      /* distance code table */
   int bl;               /* lookup bits for tl */
   int bd;               /* lookup bits for td */
-  unsigned l[288];      /* length list for huft_build */
+  unsigned *l;          /* length list for huft_build */
 
 DEBG("<fix");
+
+  l = malloc(sizeof(*l) * 288);
+  if (l == NULL)
+    return 3;			/* out of memory */
 
   /* set up literal table */
   for (i = 0; i < 144; i++)
@@ -719,9 +747,10 @@ DEBG("<fix");
   for (; i < 288; i++)          /* make a complete, but wrong code set */
     l[i] = 8;
   bl = 7;
-  if ((i = huft_build(l, 288, 257, cplens, cplext, &tl, &bl)) != 0)
+  if ((i = huft_build(l, 288, 257, cplens, cplext, &tl, &bl)) != 0) {
+    free(l);
     return i;
-
+  }
 
   /* set up distance table */
   for (i = 0; i < 30; i++)      /* make an incomplete code set */
@@ -730,6 +759,7 @@ DEBG("<fix");
   if ((i = huft_build(l, 30, 0, cpdist, cpdext, &td, &bd)) > 1)
   {
     huft_free(tl);
+    free(l);
 
     DEBG(">");
     return i;
@@ -737,11 +767,13 @@ DEBG("<fix");
 
 
   /* decompress until an end-of-block code */
-  if (inflate_codes(tl, td, bl, bd))
+  if (inflate_codes(tl, td, bl, bd)) {
+    free(l);
     return 1;
-
+  }
 
   /* free the decoding tables, return */
+  free(l);
   huft_free(tl);
   huft_free(td);
   return 0;


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH UPDATE] deflate stack usage in lib/inflate.c
  2007-04-12  8:25 [PATCH] deflate stack usage in lib/inflate.c Jeremy Fitzhardinge
@ 2007-04-12 20:50 ` Jeremy Fitzhardinge
  2007-04-12 21:21   ` Andi Kleen
                     ` (2 more replies)
  0 siblings, 3 replies; 16+ messages in thread
From: Jeremy Fitzhardinge @ 2007-04-12 20:50 UTC (permalink / raw)
  To: Jeremy Fitzhardinge
  Cc: Andrew Morton, Tim Yamin, Linux Kernel Mailing List, Andi Kleen

Subject: deflate stack usage in lib/inflate.c

inflate_fixed and huft_build together use around 2.7k of stack.  When
using 4k stacks, I saw stack overflows from interrupts arriving while
unpacking the root initrd:

do_IRQ: stack overflow: 384
 [<c0106b64>] show_trace_log_lvl+0x1a/0x30
 [<c01075e6>] show_trace+0x12/0x14
 [<c010763f>] dump_stack+0x16/0x18
 [<c0107ca4>] do_IRQ+0x6d/0xd9
 [<c010202b>] xen_evtchn_do_upcall+0x6e/0xa2
 [<c0106781>] xen_hypervisor_callback+0x25/0x2c
 [<c010116c>] xen_restore_fl+0x27/0x29
 [<c0330f63>] _spin_unlock_irqrestore+0x4a/0x50
 [<c0117aab>] change_page_attr+0x577/0x584
 [<c0117b45>] kernel_map_pages+0x8d/0xb4
 [<c016a314>] cache_alloc_refill+0x53f/0x632
 [<c016a6c2>] __kmalloc+0xc1/0x10d
 [<c0463d34>] malloc+0x10/0x12
 [<c04641c1>] huft_build+0x2a7/0x5fa
 [<c04645a5>] inflate_fixed+0x91/0x136
 [<c04657e2>] unpack_to_rootfs+0x5f2/0x8c1
 [<c0465acf>] populate_rootfs+0x1e/0xe4

(This was under Xen, but there's no reason it couldn't happen on bare
  hardware.)

This patch mallocs the local variables, thereby reducing the stack
usage to sane levels.

Also, up the heap size for the kernel decompressor to deal with the
extra allocation.

Signed-off-by: Jeremy Fitzhardinge <jeremy@xensource.com>
Cc: Tim Yamin <plasmaroo@gentoo.org>
Cc: Andi Kleen <ak@suse.de>

---
 lib/inflate.c |   66 ++++++++++++++++++++++++++++++++++++++++++---------------
 1 file changed, 49 insertions(+), 17 deletions(-)

diff -r 9b72ff6b8c17 arch/i386/boot/compressed/misc.c
--- a/arch/i386/boot/compressed/misc.c	Tue Apr 10 16:21:56 2007 -0700
+++ b/arch/i386/boot/compressed/misc.c	Thu Apr 12 13:43:52 2007 -0700
@@ -189,7 +189,7 @@ static unsigned long free_mem_ptr;
 static unsigned long free_mem_ptr;
 static unsigned long free_mem_end_ptr;
 
-#define HEAP_SIZE             0x3000
+#define HEAP_SIZE             0x4000
 
 static char *vidmem = (char *)0xb8000;
 static int vidport;
diff -r 9b72ff6b8c17 lib/inflate.c
--- a/lib/inflate.c	Tue Apr 10 16:21:56 2007 -0700
+++ b/lib/inflate.c	Thu Apr 12 13:43:52 2007 -0700
@@ -292,7 +292,6 @@ STATIC int INIT huft_build(
    oversubscribed set of lengths), and three if not enough memory. */
 {
   unsigned a;                   /* counter for codes of length k */
-  unsigned c[BMAX+1];           /* bit length count table */
   unsigned f;                   /* i repeats in table every f entries */
   int g;                        /* maximum code length */
   int h;                        /* table level */
@@ -303,18 +302,33 @@ STATIC int INIT huft_build(
   register unsigned *p;         /* pointer into c[], b[], or v[] */
   register struct huft *q;      /* points to current table */
   struct huft r;                /* table entry for structure assignment */
-  struct huft *u[BMAX];         /* table stack */
-  unsigned v[N_MAX];            /* values in order of bit length */
   register int w;               /* bits before this table == (l * h) */
-  unsigned x[BMAX+1];           /* bit offsets, then code stack */
   unsigned *xp;                 /* pointer into x */
   int y;                        /* number of dummy codes added */
   unsigned z;                   /* number of entries in current table */
+  struct {
+    unsigned c[BMAX+1];           /* bit length count table */
+    struct huft *u[BMAX];         /* table stack */
+    unsigned v[N_MAX];            /* values in order of bit length */
+    unsigned x[BMAX+1];           /* bit offsets, then code stack */
+  } *stk;
+  unsigned *c, *v, *x;
+  struct huft **u;
+  int ret;
 
 DEBG("huft1 ");
 
+  stk = malloc(sizeof(*stk));
+  if (stk == NULL)
+    return 3;			/* out of memory */
+
+  c = stk->c;
+  v = stk->v;
+  x = stk->x;
+  u = stk->u;
+
   /* Generate counts for each bit length */
-  memzero(c, sizeof(c));
+  memzero(stk->c, sizeof(stk->c));
   p = b;  i = n;
   do {
     Tracecv(*p, (stderr, (n-i >= ' ' && n-i <= '~' ? "%c %d\n" : "0x%x %d\n"), 
@@ -326,7 +340,8 @@ DEBG("huft1 ");
   {
     *t = (struct huft *)NULL;
     *m = 0;
-    return 2;
+    ret = 2;
+    goto out;
   }
 
 DEBG("huft2 ");
@@ -351,10 +366,14 @@ DEBG("huft3 ");
 
   /* Adjust last length count to fill out codes, if needed */
   for (y = 1 << j; j < i; j++, y <<= 1)
-    if ((y -= c[j]) < 0)
-      return 2;                 /* bad input: more codes than bits */
-  if ((y -= c[i]) < 0)
-    return 2;
+    if ((y -= c[j]) < 0) {
+      ret = 2;                 /* bad input: more codes than bits */
+      goto out;
+    }
+  if ((y -= c[i]) < 0) {
+    ret = 2;
+    goto out;
+  }
   c[i] += y;
 
 DEBG("huft4 ");
@@ -428,7 +447,8 @@ DEBG1("3 ");
         {
           if (h)
             huft_free(u[0]);
-          return 3;             /* not enough memory */
+          ret = 3;             /* not enough memory */
+	  goto out;
         }
 DEBG1("4 ");
         hufts += z + 1;         /* track memory usage */
@@ -492,7 +512,11 @@ DEBG("huft7 ");
 DEBG("huft7 ");
 
   /* Return true (1) if we were given an incomplete table */
-  return y != 0 && g != 1;
+  ret = y != 0 && g != 1;
+
+  out:
+  free(stk);
+  return ret;
 }
 
 
@@ -705,9 +729,13 @@ STATIC int noinline INIT inflate_fixed(v
   struct huft *td;      /* distance code table */
   int bl;               /* lookup bits for tl */
   int bd;               /* lookup bits for td */
-  unsigned l[288];      /* length list for huft_build */
+  unsigned *l;          /* length list for huft_build */
 
 DEBG("<fix");
+
+  l = malloc(sizeof(*l) * 288);
+  if (l == NULL)
+    return 3;			/* out of memory */
 
   /* set up literal table */
   for (i = 0; i < 144; i++)
@@ -719,9 +747,10 @@ DEBG("<fix");
   for (; i < 288; i++)          /* make a complete, but wrong code set */
     l[i] = 8;
   bl = 7;
-  if ((i = huft_build(l, 288, 257, cplens, cplext, &tl, &bl)) != 0)
+  if ((i = huft_build(l, 288, 257, cplens, cplext, &tl, &bl)) != 0) {
+    free(l);
     return i;
-
+  }
 
   /* set up distance table */
   for (i = 0; i < 30; i++)      /* make an incomplete code set */
@@ -730,6 +759,7 @@ DEBG("<fix");
   if ((i = huft_build(l, 30, 0, cpdist, cpdext, &td, &bd)) > 1)
   {
     huft_free(tl);
+    free(l);
 
     DEBG(">");
     return i;
@@ -737,11 +767,13 @@ DEBG("<fix");
 
 
   /* decompress until an end-of-block code */
-  if (inflate_codes(tl, td, bl, bd))
+  if (inflate_codes(tl, td, bl, bd)) {
+    free(l);
     return 1;
-
+  }
 
   /* free the decoding tables, return */
+  free(l);
   huft_free(tl);
   huft_free(td);
   return 0;



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH UPDATE] deflate stack usage in lib/inflate.c
  2007-04-12 20:50 ` [PATCH UPDATE] " Jeremy Fitzhardinge
@ 2007-04-12 21:21   ` Andi Kleen
  2007-04-12 22:39     ` Jeremy Fitzhardinge
  2007-04-12 22:56     ` Jeremy Fitzhardinge
  2007-04-12 22:25   ` Matt Mackall
  2007-04-13 18:42   ` Matt Mackall
  2 siblings, 2 replies; 16+ messages in thread
From: Andi Kleen @ 2007-04-12 21:21 UTC (permalink / raw)
  To: Jeremy Fitzhardinge; +Cc: Andrew Morton, Tim Yamin, Linux Kernel Mailing List


> (This was under Xen, but there's no reason it couldn't happen on bare
>   hardware.)

Hmm, does Xen perhaps not use interrupt stacks? Normally 2.7k should be still
green as long as there are not too many functions above/below it.

-Andi

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH UPDATE] deflate stack usage in lib/inflate.c
  2007-04-12 20:50 ` [PATCH UPDATE] " Jeremy Fitzhardinge
  2007-04-12 21:21   ` Andi Kleen
@ 2007-04-12 22:25   ` Matt Mackall
  2007-04-12 22:57     ` Jeremy Fitzhardinge
  2007-04-13 18:42   ` Matt Mackall
  2 siblings, 1 reply; 16+ messages in thread
From: Matt Mackall @ 2007-04-12 22:25 UTC (permalink / raw)
  To: Jeremy Fitzhardinge
  Cc: Andrew Morton, Tim Yamin, Linux Kernel Mailing List, Andi Kleen

On Thu, Apr 12, 2007 at 01:50:54PM -0700, Jeremy Fitzhardinge wrote:
> -#define HEAP_SIZE             0x3000
> +#define HEAP_SIZE             0x4000

There are a bunch more of these that'll need fixing.

-- 
Mathematics is the supreme nostalgia of our time.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH UPDATE] deflate stack usage in lib/inflate.c
  2007-04-12 21:21   ` Andi Kleen
@ 2007-04-12 22:39     ` Jeremy Fitzhardinge
  2007-04-12 23:20       ` Jan Engelhardt
  2007-04-12 22:56     ` Jeremy Fitzhardinge
  1 sibling, 1 reply; 16+ messages in thread
From: Jeremy Fitzhardinge @ 2007-04-12 22:39 UTC (permalink / raw)
  To: Andi Kleen; +Cc: Andrew Morton, Tim Yamin, Linux Kernel Mailing List

Andi Kleen wrote:
> Hmm, does Xen perhaps not use interrupt stacks? Normally 2.7k should be still
> green as long as there are not too many functions above/below it.
>   

That's a good point, I'll need to check that.  Still, nearly 3k of stack!

    J

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH UPDATE] deflate stack usage in lib/inflate.c
  2007-04-12 22:57     ` Jeremy Fitzhardinge
@ 2007-04-12 22:53       ` Matt Mackall
  2007-04-16 21:04       ` Russell King
  1 sibling, 0 replies; 16+ messages in thread
From: Matt Mackall @ 2007-04-12 22:53 UTC (permalink / raw)
  To: Jeremy Fitzhardinge
  Cc: Andrew Morton, Tim Yamin, Linux Kernel Mailing List, Andi Kleen

On Thu, Apr 12, 2007 at 03:57:48PM -0700, Jeremy Fitzhardinge wrote:
> Matt Mackall wrote:
> > On Thu, Apr 12, 2007 at 01:50:54PM -0700, Jeremy Fitzhardinge wrote:
> >   
> >> -#define HEAP_SIZE             0x3000
> >> +#define HEAP_SIZE             0x4000
> >>     
> >
> > There are a bunch more of these that'll need fixing.
> >   
> 
> Like this?

I'm not sure what the story is with the platforms that bump this to
0x10000, but this does get the rest of them.

Acked-by: Matt Mackall <mpm@selenic.com>

-- 
Mathematics is the supreme nostalgia of our time.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH UPDATE] deflate stack usage in lib/inflate.c
  2007-04-12 21:21   ` Andi Kleen
  2007-04-12 22:39     ` Jeremy Fitzhardinge
@ 2007-04-12 22:56     ` Jeremy Fitzhardinge
  2007-04-12 23:02       ` Andi Kleen
  2007-04-12 23:06       ` Chuck Ebbert
  1 sibling, 2 replies; 16+ messages in thread
From: Jeremy Fitzhardinge @ 2007-04-12 22:56 UTC (permalink / raw)
  To: Andi Kleen; +Cc: Andrew Morton, Tim Yamin, Linux Kernel Mailing List

Andi Kleen wrote:
>> (This was under Xen, but there's no reason it couldn't happen on bare
>>   hardware.)
>>     
>
> Hmm, does Xen perhaps not use interrupt stacks?

Looks like that's all done in do_IRQ, so it should be independent of
whether its Xen or not.  And the stack overflow check is performed on
the main stack, before switching to the interrupt stack.

    J

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH UPDATE] deflate stack usage in lib/inflate.c
  2007-04-12 22:25   ` Matt Mackall
@ 2007-04-12 22:57     ` Jeremy Fitzhardinge
  2007-04-12 22:53       ` Matt Mackall
  2007-04-16 21:04       ` Russell King
  0 siblings, 2 replies; 16+ messages in thread
From: Jeremy Fitzhardinge @ 2007-04-12 22:57 UTC (permalink / raw)
  To: Matt Mackall
  Cc: Andrew Morton, Tim Yamin, Linux Kernel Mailing List, Andi Kleen

Matt Mackall wrote:
> On Thu, Apr 12, 2007 at 01:50:54PM -0700, Jeremy Fitzhardinge wrote:
>   
>> -#define HEAP_SIZE             0x3000
>> +#define HEAP_SIZE             0x4000
>>     
>
> There are a bunch more of these that'll need fixing.
>   

Like this?

diff -r 2ad8a0729f26 arch/alpha/boot/misc.c
--- a/arch/alpha/boot/misc.c	Thu Apr 12 13:44:02 2007 -0700
+++ b/arch/alpha/boot/misc.c	Thu Apr 12 15:48:43 2007 -0700
@@ -98,7 +98,7 @@ static ulg free_mem_ptr;
 static ulg free_mem_ptr;
 static ulg free_mem_ptr_end;
 
-#define HEAP_SIZE 0x2000
+#define HEAP_SIZE 0x3000
 
 #include "../../../lib/inflate.c"
 
diff -r 2ad8a0729f26 arch/arm/boot/compressed/misc.c
--- a/arch/arm/boot/compressed/misc.c	Thu Apr 12 13:44:02 2007 -0700
+++ b/arch/arm/boot/compressed/misc.c	Thu Apr 12 15:48:43 2007 -0700
@@ -239,7 +239,7 @@ static ulg free_mem_ptr;
 static ulg free_mem_ptr;
 static ulg free_mem_ptr_end;
 
-#define HEAP_SIZE 0x2000
+#define HEAP_SIZE 0x3000
 
 #include "../../../../lib/inflate.c"
 
diff -r 2ad8a0729f26 arch/arm26/boot/compressed/misc.c
--- a/arch/arm26/boot/compressed/misc.c	Thu Apr 12 13:44:02 2007 -0700
+++ b/arch/arm26/boot/compressed/misc.c	Thu Apr 12 15:48:43 2007 -0700
@@ -182,7 +182,7 @@ static ulg free_mem_ptr;
 static ulg free_mem_ptr;
 static ulg free_mem_ptr_end;
 
-#define HEAP_SIZE 0x2000
+#define HEAP_SIZE 0x3000
 
 #include "../../../../lib/inflate.c"
 
diff -r 2ad8a0729f26 arch/x86_64/boot/compressed/misc.c
--- a/arch/x86_64/boot/compressed/misc.c	Thu Apr 12 13:44:02 2007 -0700
+++ b/arch/x86_64/boot/compressed/misc.c	Thu Apr 12 15:48:43 2007 -0700
@@ -189,7 +189,7 @@ static long free_mem_ptr;
 static long free_mem_ptr;
 static long free_mem_end_ptr;
 
-#define HEAP_SIZE             0x6000
+#define HEAP_SIZE             0x7000
 
 static char *vidmem = (char *)0xb8000;
 static int vidport;


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH UPDATE] deflate stack usage in lib/inflate.c
  2007-04-12 22:56     ` Jeremy Fitzhardinge
@ 2007-04-12 23:02       ` Andi Kleen
  2007-04-13  0:00         ` Jeremy Fitzhardinge
  2007-04-12 23:06       ` Chuck Ebbert
  1 sibling, 1 reply; 16+ messages in thread
From: Andi Kleen @ 2007-04-12 23:02 UTC (permalink / raw)
  To: Jeremy Fitzhardinge; +Cc: Andrew Morton, Tim Yamin, Linux Kernel Mailing List

On Friday 13 April 2007 00:56:56 Jeremy Fitzhardinge wrote:
> Andi Kleen wrote:
> >> (This was under Xen, but there's no reason it couldn't happen on bare
> >>   hardware.)
> >>     
> >
> > Hmm, does Xen perhaps not use interrupt stacks?
> 
> Looks like that's all done in do_IRQ, so it should be independent of
> whether its Xen or not.  And the stack overflow check is performed on
> the main stack, before switching to the interrupt stack.

Yes, but then we should have seen more frequently, shouldn't we? I always
run with the stack overflow check enabled and I don't think I ever saw 
warnings in inflate.

Something must be different in the Xen setup. Dunno if it's a bug,
but such differences could cause more problems later.

-Andi




^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH UPDATE] deflate stack usage in lib/inflate.c
  2007-04-12 22:56     ` Jeremy Fitzhardinge
  2007-04-12 23:02       ` Andi Kleen
@ 2007-04-12 23:06       ` Chuck Ebbert
  1 sibling, 0 replies; 16+ messages in thread
From: Chuck Ebbert @ 2007-04-12 23:06 UTC (permalink / raw)
  To: Jeremy Fitzhardinge
  Cc: Andi Kleen, Andrew Morton, Tim Yamin, Linux Kernel Mailing List

Jeremy Fitzhardinge wrote:
> Andi Kleen wrote:
>>> (This was under Xen, but there's no reason it couldn't happen on bare
>>>   hardware.)
>>>     
>> Hmm, does Xen perhaps not use interrupt stacks?
> 
> Looks like that's all done in do_IRQ, so it should be independent of
> whether its Xen or not.  And the stack overflow check is performed on
> the main stack, before switching to the interrupt stack.
> 

Yeah, the do_IRQ thing is misleading because it makes you think the
interrupt caused an overflow when all it did was detect a
near-overflow condition. (The number printed is the amount of space
left.)

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH UPDATE] deflate stack usage in lib/inflate.c
  2007-04-12 22:39     ` Jeremy Fitzhardinge
@ 2007-04-12 23:20       ` Jan Engelhardt
  2007-04-12 23:31         ` Jeremy Fitzhardinge
  2007-04-12 23:37         ` Andi Kleen
  0 siblings, 2 replies; 16+ messages in thread
From: Jan Engelhardt @ 2007-04-12 23:20 UTC (permalink / raw)
  To: Jeremy Fitzhardinge
  Cc: Andi Kleen, Andrew Morton, Tim Yamin, Linux Kernel Mailing List


On Apr 12 2007 15:39, Jeremy Fitzhardinge wrote:
>Andi Kleen wrote:
>> Hmm, does Xen perhaps not use interrupt stacks? Normally 2.7k should be still
>> green as long as there are not too many functions above/below it.
>
>That's a good point, I'll need to check that.  Still, nearly 3k of stack!

I bite. Would compressing the vmlinux binary with LZO or LZMA make an
improvement to the bootstrap uncompress stack usage?


Jan
-- 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH UPDATE] deflate stack usage in lib/inflate.c
  2007-04-12 23:20       ` Jan Engelhardt
@ 2007-04-12 23:31         ` Jeremy Fitzhardinge
  2007-04-12 23:37         ` Andi Kleen
  1 sibling, 0 replies; 16+ messages in thread
From: Jeremy Fitzhardinge @ 2007-04-12 23:31 UTC (permalink / raw)
  To: Jan Engelhardt
  Cc: Andi Kleen, Andrew Morton, Tim Yamin, Linux Kernel Mailing List

Jan Engelhardt wrote:
> On Apr 12 2007 15:39, Jeremy Fitzhardinge wrote:
>   
>> Andi Kleen wrote:
>>     
>>> Hmm, does Xen perhaps not use interrupt stacks? Normally 2.7k should be still
>>> green as long as there are not too many functions above/below it.
>>>       
>> That's a good point, I'll need to check that.  Still, nearly 3k of stack!
>>     
>
> I bite. Would compressing the vmlinux binary with LZO or LZMA make an
> improvement to the bootstrap uncompress stack usage?
>   

Well, the thread started with my patch to fix inflate.  The stack usage
of LZO or LZMA decompressors will primarily depend on how they're
implemented rather than any inherent property of the algorithms.

    J

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH UPDATE] deflate stack usage in lib/inflate.c
  2007-04-12 23:20       ` Jan Engelhardt
  2007-04-12 23:31         ` Jeremy Fitzhardinge
@ 2007-04-12 23:37         ` Andi Kleen
  1 sibling, 0 replies; 16+ messages in thread
From: Andi Kleen @ 2007-04-12 23:37 UTC (permalink / raw)
  To: Jan Engelhardt
  Cc: Jeremy Fitzhardinge, Andrew Morton, Tim Yamin,
	Linux Kernel Mailing List

On Friday 13 April 2007 01:20:40 Jan Engelhardt wrote:
> 
> On Apr 12 2007 15:39, Jeremy Fitzhardinge wrote:
> >Andi Kleen wrote:
> >> Hmm, does Xen perhaps not use interrupt stacks? Normally 2.7k should be still
> >> green as long as there are not too many functions above/below it.
> >
> >That's a good point, I'll need to check that.  Still, nearly 3k of stack!
> 
> I bite. Would compressing the vmlinux binary with LZO or LZMA make an
> improvement to the bootstrap uncompress stack usage?

We don't care about the stack usage, as long as it doesn't overflow.
It's a very limited piece of code that doesn't run on top or below
other subsystems.

-Andi

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH UPDATE] deflate stack usage in lib/inflate.c
  2007-04-12 23:02       ` Andi Kleen
@ 2007-04-13  0:00         ` Jeremy Fitzhardinge
  0 siblings, 0 replies; 16+ messages in thread
From: Jeremy Fitzhardinge @ 2007-04-13  0:00 UTC (permalink / raw)
  To: Andi Kleen; +Cc: Andrew Morton, Tim Yamin, Linux Kernel Mailing List

Andi Kleen wrote:
> Yes, but then we should have seen more frequently, shouldn't we? I always
> run with the stack overflow check enabled and I don't think I ever saw 
> warnings in inflate.
>   

I guess the window is just while decompressing the root filesystem. 
Interrupts under Xen might be using a little more stack (~40-50 bytes?),
but its not a qualitative difference.  It might have more to do with
different timing.

    J

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH UPDATE] deflate stack usage in lib/inflate.c
  2007-04-12 20:50 ` [PATCH UPDATE] " Jeremy Fitzhardinge
  2007-04-12 21:21   ` Andi Kleen
  2007-04-12 22:25   ` Matt Mackall
@ 2007-04-13 18:42   ` Matt Mackall
  2 siblings, 0 replies; 16+ messages in thread
From: Matt Mackall @ 2007-04-13 18:42 UTC (permalink / raw)
  To: Jeremy Fitzhardinge
  Cc: Andrew Morton, Tim Yamin, Linux Kernel Mailing List, Andi Kleen

On Thu, Apr 12, 2007 at 01:50:54PM -0700, Jeremy Fitzhardinge wrote:
> Subject: deflate stack usage in lib/inflate.c
> 
> inflate_fixed and huft_build together use around 2.7k of stack.  When
> using 4k stacks, I saw stack overflows from interrupts arriving while
> unpacking the root initrd:
> 
> do_IRQ: stack overflow: 384
>  [<c0106b64>] show_trace_log_lvl+0x1a/0x30
>  [<c01075e6>] show_trace+0x12/0x14
>  [<c010763f>] dump_stack+0x16/0x18
>  [<c0107ca4>] do_IRQ+0x6d/0xd9
>  [<c010202b>] xen_evtchn_do_upcall+0x6e/0xa2
>  [<c0106781>] xen_hypervisor_callback+0x25/0x2c
>  [<c010116c>] xen_restore_fl+0x27/0x29
>  [<c0330f63>] _spin_unlock_irqrestore+0x4a/0x50
>  [<c0117aab>] change_page_attr+0x577/0x584
>  [<c0117b45>] kernel_map_pages+0x8d/0xb4
>  [<c016a314>] cache_alloc_refill+0x53f/0x632
>  [<c016a6c2>] __kmalloc+0xc1/0x10d
>  [<c0463d34>] malloc+0x10/0x12
>  [<c04641c1>] huft_build+0x2a7/0x5fa
>  [<c04645a5>] inflate_fixed+0x91/0x136
>  [<c04657e2>] unpack_to_rootfs+0x5f2/0x8c1
>  [<c0465acf>] populate_rootfs+0x1e/0xe4
> 
> (This was under Xen, but there's no reason it couldn't happen on bare
>   hardware.)
> 
> This patch mallocs the local variables, thereby reducing the stack
> usage to sane levels.
> 
> Also, up the heap size for the kernel decompressor to deal with the
> extra allocation.

Lost the non-x86 bits again.

-- 
Mathematics is the supreme nostalgia of our time.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH UPDATE] deflate stack usage in lib/inflate.c
  2007-04-12 22:57     ` Jeremy Fitzhardinge
  2007-04-12 22:53       ` Matt Mackall
@ 2007-04-16 21:04       ` Russell King
  1 sibling, 0 replies; 16+ messages in thread
From: Russell King @ 2007-04-16 21:04 UTC (permalink / raw)
  To: Jeremy Fitzhardinge
  Cc: Matt Mackall, Andrew Morton, Tim Yamin, Linux Kernel Mailing List,
	Andi Kleen

On Thu, Apr 12, 2007 at 03:57:48PM -0700, Jeremy Fitzhardinge wrote:
> diff -r 2ad8a0729f26 arch/arm/boot/compressed/misc.c
> --- a/arch/arm/boot/compressed/misc.c	Thu Apr 12 13:44:02 2007 -0700
> +++ b/arch/arm/boot/compressed/misc.c	Thu Apr 12 15:48:43 2007 -0700
> @@ -239,7 +239,7 @@ static ulg free_mem_ptr;
>  static ulg free_mem_ptr;
>  static ulg free_mem_ptr_end;
>  
> -#define HEAP_SIZE 0x2000
> +#define HEAP_SIZE 0x3000

Not used on ARM, this macro can be deleted.  For the record, the space
available to the decompressor on ARM is 64K.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2007-04-16 21:04 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-04-12  8:25 [PATCH] deflate stack usage in lib/inflate.c Jeremy Fitzhardinge
2007-04-12 20:50 ` [PATCH UPDATE] " Jeremy Fitzhardinge
2007-04-12 21:21   ` Andi Kleen
2007-04-12 22:39     ` Jeremy Fitzhardinge
2007-04-12 23:20       ` Jan Engelhardt
2007-04-12 23:31         ` Jeremy Fitzhardinge
2007-04-12 23:37         ` Andi Kleen
2007-04-12 22:56     ` Jeremy Fitzhardinge
2007-04-12 23:02       ` Andi Kleen
2007-04-13  0:00         ` Jeremy Fitzhardinge
2007-04-12 23:06       ` Chuck Ebbert
2007-04-12 22:25   ` Matt Mackall
2007-04-12 22:57     ` Jeremy Fitzhardinge
2007-04-12 22:53       ` Matt Mackall
2007-04-16 21:04       ` Russell King
2007-04-13 18:42   ` Matt Mackall

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox