From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S1752284AbdBILJw (ORCPT <rfc822;w@1wt.eu>);
        Thu, 9 Feb 2017 06:09:52 -0500
Received: from mailhost.informatik.uni-hamburg.de ([134.100.9.70]:33346 "EHLO
        mailhost.informatik.uni-hamburg.de" rhost-flags-OK-OK-OK-OK)
        by vger.kernel.org with ESMTP id S1752166AbdBILJt (ORCPT
        <rfc822;linux-kernel@vger.kernel.org>);
        Thu, 9 Feb 2017 06:09:49 -0500
Date: Thu, 9 Feb 2017 12:09:45 +0100
From: Sven Schmidt <4sschmid@informatik.uni-hamburg.de>
To: Arnd Bergmann <arnd@arndb.de>
Cc: Andrew Morton <akpm@linux-foundation.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] lz4: fix KERNEL_LZ4 support
Message-ID: <20170209110945.GD3575@bierbaron.springfield.local>
References: <20170208211946.2839649-1-arnd@arndb.de>
 <20170208133459.c694a5bce66ab26c0ee80dee@linux-foundation.org>
 <CAK8P3a1xDpkLJUm+NVZBjG+a6Mqr5pYuHLa9TxO=dwfSe-XCVA@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CAK8P3a1xDpkLJUm+NVZBjG+a6Mqr5pYuHLa9TxO=dwfSe-XCVA@mail.gmail.com>
User-Agent: Mutt/1.7.2 (2016-11-26)
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

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