[PATCH] lz4: fix KERNEL_LZ4 support

[PATCH] lz4: fix KERNEL_LZ4 support

[Arnd Bergmann]

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.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 lib/lz4/lz4_decompress.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/lib/lz4/lz4_decompress.c b/lib/lz4/lz4_decompress.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
+
 /*-*****************************
  *	Decompression functions
  *******************************/
-- 
2.9.0

Re: [PATCH] lz4: fix KERNEL_LZ4 support

[Andrew Morton]

On Wed,  8 Feb 2017 22:19:23 +0100 Arnd Bergmann <arnd@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?

> 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?

--- a/lib/lz4/lz4_decompress.c~lz4-fix-kernel_lz4-support-fix
+++ a/lib/lz4/lz4_decompress.c
@@ -328,7 +323,6 @@ int LZ4_decompress_safe(const char *sour
 		maxDecompressedSize, endOnInputSize, full, 0,
 		noDict, (BYTE *)dest, NULL, 0);
 }
-EXPORT_SYMBOL(LZ4_decompress_safe);
 
 int LZ4_decompress_safe_partial(const char *source, char *dest,
 	int compressedSize, int targetOutputSize, int maxDecompressedSize)
@@ -337,7 +331,6 @@ int LZ4_decompress_safe_partial(const ch
 		maxDecompressedSize, endOnInputSize, partial,
 		targetOutputSize, noDict, (BYTE *)dest, NULL, 0);
 }
-EXPORT_SYMBOL(LZ4_decompress_safe_partial);
 
 int LZ4_decompress_fast(const char *source, char *dest, int originalSize)
 {
@@ -345,7 +338,6 @@ int LZ4_decompress_fast(const char *sour
 		endOnOutputSize, full, 0, withPrefix64k,
 		(BYTE *)(dest - 64 * KB), NULL, 64 * KB);
 }
-EXPORT_SYMBOL(LZ4_decompress_fast);
 
 int LZ4_setStreamDecode(LZ4_streamDecode_t *LZ4_streamDecode,
 	const char *dictionary, int dictSize)
@@ -358,7 +350,6 @@ int LZ4_setStreamDecode(LZ4_streamDecode
 	lz4sd->extDictSize	= 0;
 	return 1;
 }
-EXPORT_SYMBOL(LZ4_setStreamDecode);
 
 /*
  * *_continue() :
@@ -406,7 +397,6 @@ int LZ4_decompress_safe_continue(LZ4_str
 
 	return result;
 }
-EXPORT_SYMBOL(LZ4_decompress_safe_continue);
 
 int LZ4_decompress_fast_continue(LZ4_streamDecode_t *LZ4_streamDecode,
 	const char *source, char *dest, int originalSize)
@@ -441,7 +431,6 @@ int LZ4_decompress_fast_continue(LZ4_str
 
 	return result;
 }
-EXPORT_SYMBOL(LZ4_decompress_fast_continue);
 
 /*
  * Advanced decoding functions :
@@ -478,7 +467,6 @@ int LZ4_decompress_safe_usingDict(const
 	return LZ4_decompress_usingDict_generic(source, dest,
 		compressedSize, maxOutputSize, 1, dictStart, dictSize);
 }
-EXPORT_SYMBOL(LZ4_decompress_safe_usingDict);
 
 int LZ4_decompress_fast_usingDict(const char *source, char *dest,
 	int originalSize, const char *dictStart, int dictSize)
@@ -486,7 +474,17 @@ int LZ4_decompress_fast_usingDict(const
 	return LZ4_decompress_usingDict_generic(source, dest, 0,
 		originalSize, 0, dictStart, dictSize);
 }
+
+#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
 
 MODULE_LICENSE("Dual BSD/GPL");
 MODULE_DESCRIPTION("LZ4 decompressor");
_

Re: [PATCH] lz4: fix KERNEL_LZ4 support

[Arnd Bergmann]

On Wed, Feb 8, 2017 at 10:34 PM, Andrew Morton
<akpm@linux-foundation.org> wrote:
> On Wed,  8 Feb 2017 22:19:23 +0100 Arnd Bergmann <arnd@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

Re: [PATCH] lz4: fix KERNEL_LZ4 support

[Sven Schmidt]

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
> <akpm@linux-foundation.org> wrote:
> > On Wed,  8 Feb 2017 22:19:23 +0100 Arnd Bergmann <arnd@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