Hello, On Wed, Feb 08, 2017 at 11:06:34PM +0100, Arnd Bergmann wrote: > On Wed, Feb 8, 2017 at 10:34 PM, Andrew Morton > <a...@linux-foundation.org> wrote: > > On Wed, 8 Feb 2017 22:19:23 +0100 Arnd Bergmann <a...@arndb.de> wrote: > > > >> The updated lz4 library removed the #ifdef guards around the various > >> EXPORT_SYMBOL statements in the original kernel lz4 support, which broke > >> CONFIG_KERNEL_LZ4 on x86: > >> > >> x86_64-linux-ld: -r and -pie may not be used together > >> scripts/Makefile.build:308: recipe for target > >> 'arch/x86/boot/compressed/misc.o' failed > >> > >> This uses a simpler way to do the same thing, by overriding the > >> EXPORT_SYMBOL macro. > >> > > > > hm, why does this CONFIG_KERNEL_LZ4 thing exist? What makes lz4 > > different from a billion other kernel modules? > > This symbol means the kernel (bzImage) itself is compressed with lz4, an we > include the lz4_decompress.c code from arch/x86/boot/compressed/misc.c > > >> index 9bf918233749..a390f63bc475 100644 > >> --- a/lib/lz4/lz4_decompress.c > >> +++ b/lib/lz4/lz4_decompress.c > >> @@ -40,6 +40,11 @@ > >> #include <linux/kernel.h> > >> #include <asm/unaligned.h> > >> > >> +#ifdef STATIC > >> +#undef EXPORT_SYMBOL > >> +#define EXPORT_SYMBOL(x) > >> +#endif > >> + > > > > That is a bit hacky, and somewhat "surprising". Why not do it the old > > fashioned way? > > I was trying to keep the modification small, to simplify adding it the > next time we update the lz4 library. > > > + > > +#ifndef STATIC > > +EXPORT_SYMBOL(LZ4_decompress_safe); > > +EXPORT_SYMBOL(LZ4_decompress_safe_partial); > > +EXPORT_SYMBOL(LZ4_decompress_fast); > > +EXPORT_SYMBOL(LZ4_setStreamDecode); > > +EXPORT_SYMBOL(LZ4_decompress_safe_continue); > > +EXPORT_SYMBOL(LZ4_decompress_fast_continue); > > +EXPORT_SYMBOL(LZ4_decompress_safe_usingDict); > > EXPORT_SYMBOL(LZ4_decompress_fast_usingDict); > > +#endif > > > > This seems fine too, but then we probably want to keep the #ifndef around > the MODULE_LICENSE()/MODULE_DESCRIPTION() macros as well, > like we do in the other decompressors that we can use for boot image. > > lib/inflate.c also marks all functions as STATIC, but the other algorithms > don't, so I'm not sure whether we should do the same here. > > Arnd
this looks reasonable for me. Andrew: What is the usual way of dealing with such fixes? Since there's some feedback left, would I include it directly in my patchseries? Thanks, Sven
