| Message ID | 20111214204225.7c8ec86c@sf.home |
|---|---|
| State | RFC |
| Delegated to: | David Miller |
| Headers | show |
Le mercredi 14 décembre 2011 à 20:42 +0300, Sergei Trofimovich a écrit : > [ CCed Jakub ] > > >>> Boot time fixup v1.6. 4/Mar/98 Jakub Jelinek (jj@ultra.linux.cz). > >>> Patching kernel for srmmu[Fujitsu TurboSparc]/iommu > >>> Fixup i f029ddfc doesn't refer to a valid instruction at > >>> f00de648[95eea000] > >>> halt, power off > > > I put the broken image up at http://landley.net/sparc-image for the > > moment, but if you build 3.1 with the attached .config and the toolchain > > mentioned last time, it should reproduce for you. It's 100% reliable > > for me... > > Nice! With this config it breaks for me on your and mine toolchains. > The offending function is ext4_kvmalloc (and similar ext4_kvzalloc). > > The usual relocation in sparc looks like a pair of instructions loading > two pats of address in 2 instructions: > > Like that: > > sethi %hi(ext4_fill_super), %o4 !, tmp113 > > or %o4, %lo(ext4_fill_super), %o4 ! tmp113,, tmp28 > > In our case relocatable symbol sits in tail call, so %lo part is in "unusual" > RESTORE instruction: > > > ext4_kvmalloc: > ... > > sethi %hi(___i_page_kernel), %i2 !, tmp112 > > call __vmalloc, 0 ! > > restore %i2, %lo(___i_page_kernel), %o2 ! tmp112,, > ... > > David: is this code correct? Or it's a compiler bug? I am sparc32 newbie. > (C source and asm sources of function are in [1]) > > I think this kind of code is generated only in -Os. > So to workaround it I tried this hack: > > > --- a/fs/ext4/super.c > > +++ b/fs/ext4/super.c > > ret = kmalloc(size, flags); > > if (!ret) > > ret = __vmalloc(size, flags, PAGE_KERNEL); > > + > > + asm __volatile__("nop":::"memory"); > > + > > return ret; > > } > (for both ext4_kvmalloc / ext4_kvzalloc. Attached workaround as a patch.) > > It forces compiler to geterate "usual" pattern for relocation. > > I think of 2 solutions: > > 1. trying to fix sparc/boot/btfixupprep.c and arch/sparc/mm/btfixup.c > to distinct HI22 and LO10 relocations as different ones. > Right now they are merged into one 'i' type and rely on instruction heuristics to fix it. > 2. Add a hack to arch/sparc/mm/btfixup.c to recognize restore instruction as well > > Any others? 3) Adding a memset() in ext4_kvmalloc() and ext4_kvzalloc() to prefault pages ? 4) (Unrelated) : add __GFP_HIGHMEM to __vmalloc() flags, so that high memory pages can be used for large allocations. -- To unsubscribe from this list: send the line "unsubscribe sparclinux" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Sergei Trofimovich <slyich@gmail.com> Date: Wed, 14 Dec 2011 20:42:25 +0300 >> ext4_kvmalloc: > ... >> sethi %hi(___i_page_kernel), %i2 !, tmp112 >> call __vmalloc, 0 ! >> restore %i2, %lo(___i_page_kernel), %o2 ! tmp112,, > ... > > David: is this code correct? Or it's a compiler bug? I am sparc32 newbie. > (C source and asm sources of function are in [1]) It is correct, it's a tail call. -- To unsubscribe from this list: send the line "unsubscribe sparclinux" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Sergei Trofimovich <slyich@gmail.com> Date: Wed, 14 Dec 2011 20:42:25 +0300 > It forces compiler to geterate "usual" pattern for relocation. No, this is wrong. The right fix is to simply teach btfixup to be able to patch the 'restore' just as equally as it would patch an 'or'. -- To unsubscribe from this list: send the line "unsubscribe sparclinux" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/fs/ext4/super.c b/fs/ext4/super.c index 44d0c8d..570d45e 100644 --- a/fs/ext4/super.c +++ b/fs/ext4/super.c @@ -117,6 +117,7 @@ void *ext4_kvmalloc(size_t size, gfp_t flags) ret = kmalloc(size, flags); if (!ret) ret = __vmalloc(size, flags, PAGE_KERNEL); + mb(); /* hack to pessimize code */ return ret; } @@ -127,6 +128,7 @@ void *ext4_kvzalloc(size_t size, gfp_t flags) ret = kzalloc(size, flags); if (!ret) ret = __vmalloc(size, flags | __GFP_ZERO, PAGE_KERNEL); + mb(); /* hack to pessimize code */ return ret; }