Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Wed, 3 Apr 2019 13:56:53 -0400
From:   Joel Fernandes <joel@joelfernandes.org>
To:     Daniel Colascione <dancol@google.com>
Cc:     Masahiro Yamada <yamada.masahiro@socionext.com>,
        Alexei Starovoitov <alexei.starovoitov@gmail.com>,
        Networking <netdev@vger.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Andrew Morton <akpm@linux-foundation.org>,
        Alexei Starovoitov <ast@kernel.org>,
        atish patra <atishp04@gmail.com>,
        Dan Williams <dan.j.williams@intel.com>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Jonathan Corbet <corbet@lwn.net>,
        Karim Yaghmour <karim.yaghmour@opersys.com>,
        Kees Cook <keescook@chromium.org>,
        "open list:DOCUMENTATION" <linux-doc@vger.kernel.org>,
        "open list:KERNEL SELFTEST FRAMEWORK" 
        <linux-kselftest@vger.kernel.org>,
        Manoj Rao <linux@manojrajarao.com>,
        Randy Dunlap <rdunlap@infradead.org>
Subject: Re: [PATCH v2 1/2] Provide in-kernel headers for making it easy to
 extend the kernel
Message-ID: <20190403175653.GA60809@google.com>
References: <20190211143600.15021-1-joel@joelfernandes.org>
 <20190215031926.ljzluy2cfxp64u6o@ast-mbp>
 <20190325134947.GA187133@google.com>
 <20190327173103.GA205980@google.com>
 <CAK7LNARcaVtgHvco-Q=E5jhOXduk=55g9AC7nFn2esFxnk9Naw@mail.gmail.com>
 <20190403172031.GA260189@google.com>
 <CAKOZueszGpij1ZLqQfNggY-7t7BV+SksApVvmWE+kuExYvv+aA@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CAKOZueszGpij1ZLqQfNggY-7t7BV+SksApVvmWE+kuExYvv+aA@mail.gmail.com>
User-Agent: Mutt/1.10.1 (2018-07-13)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Wed, Apr 03, 2019 at 10:46:01AM -0700, Daniel Colascione wrote:
> On Wed, Apr 3, 2019 at 10:20 AM Joel Fernandes <joel@joelfernandes.org> wrote:
> >
> > On Wed, Apr 03, 2019 at 04:48:37PM +0900, Masahiro Yamada wrote:
> > > On Thu, Mar 28, 2019 at 2:32 AM Joel Fernandes <joel@joelfernandes.org> wrote:
> > > >
> > > > On Mon, Mar 25, 2019 at 09:49:47AM -0400, Joel Fernandes wrote:
> > > > > On Thu, Feb 14, 2019 at 07:19:29PM -0800, Alexei Starovoitov wrote:
> > > > > > On Mon, Feb 11, 2019 at 09:35:59AM -0500, Joel Fernandes (Google) wrote:
> > > > > > > Introduce in-kernel headers and other artifacts which are made available
> > > > > > > as an archive through proc (/proc/kheaders.txz file). This archive makes
> > > > > > > it possible to build kernel modules, run eBPF programs, and other
> > > > > > > tracing programs that need to extend the kernel for tracing purposes
> > > > > > > without any dependency on the file system having headers and build
> > > > > > > artifacts.
> > > > > > >
> > > > > > > On Android and embedded systems, it is common to switch kernels but not
> > > > > > > have kernel headers available on the file system. Raw kernel headers
> > > > > > > also cannot be copied into the filesystem like they can be on other
> > > > > > > distros, due to licensing and other issues. There's no linux-headers
> > > > > > > package on Android. Further once a different kernel is booted, any
> > > > > > > headers stored on the file system will no longer be useful. By storing
> > > > > > > the headers as a compressed archive within the kernel, we can avoid these
> > > > > > > issues that have been a hindrance for a long time.
> > > > > >
> > > > > > The set looks good to me and since the main use case is building bpf progs
> > > > > > I can route it via bpf-next tree if there are no objections.
> > > > > > Masahiro, could you please ack it?
> > > > >
> > > > > FYI, Masahiro's comments were all address by v5:
> > > > > https://lore.kernel.org/patchwork/project/lkml/list/?series=387311
> > > > >
> > > > > I believe aren't more outstanding concerns. Could we consider it for v5.2?
> > > >
> > > > Just to highlight the problem, today I booted v5.0 on an emulated Android
> > > > device for some testing, that didn't have a set of prebuilt headers that we
> > > > have been packaging on well known kernels, to get around this issue. This
> > > > caused great pain and issues with what I was doing. I know me and many others
> > > > really want this. With this I can boot an emulated Android device with
> > > > IKCONFIG_PROC=y and run BCC with that that. Also I want to do the BCC side of
> > > > the development, but first want to know if we can merge this upstream.
> > > >
> > > > Masahiro, I believe due diligence has been done in solidifying it as posted
> > > > in the v5.  Anything else we need to do here? Are you with the patches?
> > >
> > >
> > > As you said, these updates are "cosmetic".
> > > Nobody is worried about (or interested in) them.
> > > Even if we miss something, they are fixable later.
> > >
> > > I accept embedding headers in the kernel,
> > > but I do not like the part about external module build.
> > >  - kernel embeds build artifacts that depend on a
> > >    particular host-arch
> > >  - users do not know which compiler to use
> > > Perhaps, you may remember my concerns.
> > > https://lore.kernel.org/patchwork/patch/1046307/#1239501
> >
> > Masahiro,
> > I think we already discussed this right. The compiler issue is a user-issue -
> > it is not a problem that has arisen because this patch. Anyone can build a
> > kernel module today without this patch - using a compiler that is different
> > from the running kernel. So I did not fully understand your concern. This
> > patch does not ship a compiler in the archive. The compiler is upto the user
> > based on user environment. They can already shoot themself in foot without
> > this patch.
> >
> > Greg,
> > Would be great to get your thoughts here as well about Masami's concern.
> >
> > Honestly, the "kernel module building artifacts" bit is a bonus with this
> > patch. The main thing we are adding or that we need is really the headers
> > (for BCC). I just thought shipping the kernel build artifacts would also be
> > awesome because people can now build kernel modules without needing a kernel
> > tree at all.
> >
> > But I also want this stuff merged so if folks are really unhappy with the
> > module build artifacts and only want headers - then that's also fine with me
> > and I can yank them - that would also mean though that this work cannot be a
> > replacement for linux-headers package on distros, so that would be sad.
> >
> > thanks!
> 
> IMHO, including the module build stuff is fine, and the stuff that
> Masahiro mentions doesn't matter much. To be specific about the
> concerns:
> 
> >> > >  - kernel embeds build artifacts that depend on a
> > >    particular host-arch
> 
> What are these artifacts? We build the kernel as a standalone system.
> It's not as if we statically link host glibc into it or something.

There are some scripts/ that are built in the host-arch ABI. These are also put
in the archive because they are needed to build modules. So if you
cross-compile then the archive will have scripts/ that are in the host arch,
not the target arch. This makes it not possible to build modules on the
target using the archive. This is not much of an issue because I already
proved that such kernel modules can be built using a chroot in this thread:
https://lkml.org/lkml/2019/2/28/1313
And in the non cross-compile case, it will be immensely useful for distros..

> > >  - users do not know which compiler to use
> 
> Does that matter much? Do things ever break if the kernel proper is
> built with GCC x.y.z and we build a module with GCC x.y.(z+1)? Why
> would it? Structure layout is invariant across compiler version, or at
> least ought to be.  And don't we have exactly the same problem with
> any kernel headers package? I don't think it'd hurt to include the
> output of $(CC) --version though.

Apparently there were some issues in some posting where Greg said we don't
support anything but modules built with the same compiler as the kernel its
being loaded into, even if such modules do work in practice. I don't recall
the details, I can dig up that thread if you want. My point is, whether
that's an issue or not - it is not an issue introduced by this patch. That's
just a module building issue which we neither solve here, nor introduce.

> Like Joel, I'd like to see this change merged. It'll make life easier
> for a lot of people.

Thanks.

- Joel