Re: [PATCH] lz4: fix KERNEL_LZ4 support

From: Arnd Bergmann
Date: Wed Feb 08 2017 - 17:07:49 EST


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