From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.6 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, MENTIONS_GIT_HOSTING,SPF_PASS,URIBL_BLOCKED,USER_AGENT_MUTT autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 392A0C10F12 for ; Mon, 15 Apr 2019 14:05:36 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 001692087C for ; Mon, 15 Apr 2019 14:05:35 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=joelfernandes.org header.i=@joelfernandes.org header.b="NCEoz5IK" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727585AbfDOOFf (ORCPT ); Mon, 15 Apr 2019 10:05:35 -0400 Received: from mail-pf1-f193.google.com ([209.85.210.193]:42415 "EHLO mail-pf1-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727371AbfDOOFe (ORCPT ); Mon, 15 Apr 2019 10:05:34 -0400 Received: by mail-pf1-f193.google.com with SMTP id w25so8637450pfi.9 for ; Mon, 15 Apr 2019 07:05:34 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=joelfernandes.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=0WDxr7x++wyxZBE+48i+O5ufisb7Dj38JxloaCZIM1g=; b=NCEoz5IKT8jCnPgqYIAWnJK8ysRkcxML7a53/BSGxcv4hkvYLWrjZ9Yscpy5OoTZmA UgA88NzFVWuBs/mro+fu0DELzgUH7DgwJ5zjtM7kJcbbLgKxwADikls2Zbhp3rfQYvmU FalL6hftLUMqTQUZXUZEJVGJILFdgUIyMhags= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=0WDxr7x++wyxZBE+48i+O5ufisb7Dj38JxloaCZIM1g=; b=MrC3mZEQ/SGa3ui65b4rXeUN1IJYUphjUal1adehlXGqY7JJPCxgrGl4YAZi3tTplH se4KkF6AeADKUN5a6F5kOWsDxD0zcJqyozRRY8eijVNAdYtQyrNtRIaeA4rYUcaUSeZi eVN1Gkn0AxwjSuzAkLAGd3uqqimziFUQmblAf3lzIgxIsq97rW90b/qg0TVfvHnxpuz4 /bHkBTgVTvObdyZiMjoe6UASwCBhGvoi+Uf33ZUUAsRmYA8EkkMhCx0skV+muwkPGeU2 XqNncmEhGsm3/J9ZryOdCDWGEdT9S00WYnyxNl5vFMQybDw4V8ddDTWLFwyMzi656kMB 8Jzg== X-Gm-Message-State: APjAAAVHftm2/8hAhsySRolg4pA4KQuDMQT3ZbAwnNkc4AGyCNNrzDha 4KecU0jlVJizNeOtJ6UoAxL/8g== X-Google-Smtp-Source: APXvYqyLhgQVBx81W0JcBdv3yAJYu5l22G1X1P7p/QGvIEXEqRq+ou1HUwmoW8xJZITy8HzRTkNEJg== X-Received: by 2002:a63:5149:: with SMTP id r9mr67474876pgl.177.1555337133723; Mon, 15 Apr 2019 07:05:33 -0700 (PDT) Received: from localhost ([2620:15c:6:12:9c46:e0da:efbf:69cc]) by smtp.gmail.com with ESMTPSA id d69sm35046794pfg.24.2019.04.15.07.05.32 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Mon, 15 Apr 2019 07:05:32 -0700 (PDT) Date: Mon, 15 Apr 2019 10:05:31 -0400 From: Joel Fernandes To: Olof Johansson Cc: Alexei Starovoitov , Linux Kernel Mailing List , Qais Yousef , Dietmar Eggemann , Manoj Rao , Andrew Morton , Alexei Starovoitov , atish patra , Daniel Colascione , Dan Williams , Greg Kroah-Hartman , Guenter Roeck , Jonathan Corbet , Karim Yaghmour , Kees Cook , Android Kernel Team , "open list:DOCUMENTATION" , "open list:KERNEL SELFTEST FRAMEWORK" , linux-trace-devel@vger.kernel.org, Masahiro Yamada , Masami Hiramatsu , Randy Dunlap , Steven Rostedt , Shuah Khan , Yonghong Song Subject: Re: [PATCH v5 1/3] Provide in-kernel headers to make extending kernel easier Message-ID: <20190415140531.GB205801@google.com> References: <20190320163116.39275-1-joel@joelfernandes.org> <20190408203601.GF133872@google.com> <20190411031540.ehezr6kq7ouobpzx@ast-mbp.dhcp.thefacebook.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.10.1 (2018-07-13) Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sun, Apr 14, 2019 at 12:38:34PM -0700, Olof Johansson wrote: > On Wed, Apr 10, 2019 at 8:15 PM Alexei Starovoitov > wrote: > > > > On Wed, Apr 10, 2019 at 09:34:49AM -0700, Olof Johansson wrote: > > > On Wed, Apr 10, 2019 at 8:51 AM Joel Fernandes wrote: > > > > > > > > On Wed, Apr 10, 2019 at 11:07 AM Olof Johansson wrote: > > > > [snip] > > > > > > > Wouldn't it be more convenient to provide it in a standardized format > > > > > > > such that you won't have to take an additional step, and always have > > > > > > > This is that form IMO. > > ... > > > Compared to: > > > - Extract tarball > > > - Build and load > > > - Remove file tree from filesystem > > > > I think there are too many assumptions in this thread in regard to what > > is more convenient for user space. > > I think bcc should try to avoid extracting tarball into file system. > > For example libbcc can uncompress kheader.tar.xz into virtual file system > > of clang front-end. It's more or less std::map > > with key=path, value=content of the file. Access to such in-memory > > 'files' is obviously faster than doing open/read syscalls. > > I think performance is a red herring, especially since you have to > uncompress it on every compiler invocation. With this you'd need to > read/touch/write _all_ header files, not just the one your current > compiler invocation will use. > > In the grand scheme of things, open/mmap syscalls wouldn't necessarily > be slower. Agreed. > > bcc already uses this approach for some bcc internal 'files' that > > it passes to clang during compilation. > > All of /proc/kheaders.tar.xz files can be passed the same way > > without extracting them into real file system. > > This is now a circular argument. Joel was stating that the plain text > headers took up too much memory, but now it's preferred to create such > filesystem in userspace memory on *every compiler invocation*? > That's... not better. And definitely worse if you want to compile in > parallel. The BCC patch does not extract purely into memory, but uses temporary directory: https://github.com/iovisor/bcc/pull/2312 I believe this is a good approach. > From my perspective, this is where we're at: > > This patch seems to have been met with a lot of responses in the tone > of "this is not an appealing solution". Meanwhile, some of the > suggested alternative solutions have not worked out, and we are now at > a point where there's less interest in exploring alternatives and > arguments to merge as-is with only minor adjustments. > > I understand the desire to solve this. It'd be really convenient to > have a way to runtime build against the same structure layouts that > the kernel was built with. But I haven't heard anyone say that they About structure layouts, I'm assuming you mean compiler generated debug info. That does not work for eBPF tools as was mentioned previously in these threads: https://lkml.org/lkml/2019/3/11/1358 https://lkml.org/lkml/2019/3/11/1363 > *like* the solution proposed, and I haven't seen many of those > expressing concerns being converted to support it. IMHO there has been good number of people on both sides of the argument. If it were as strong of an opposition as you think, then I would have personally not wanted this merged tbh. We do want a solution that is clean and works, and I think this is a candidate. [snip] > I'd be a *lot* less hesitant if this went into debugfs or another > location than /proc, which is one of the most regression-sensitive > interfaces we have in the kernel. The solution should be regression-sensitive imho, we don't want the tracing tools to break, people use them. And their usability and robustness is important which prompted these patches. For some time we have been hosting downloadable headers for popular kernels (such as LTS versions) to workaround this issue, but this both a maintenance issue and non-scalable, and not robust (someone boots a custom kernel or we forget to update headers for a future LTS release and the tools break). thanks, - Joel From mboxrd@z Thu Jan 1 00:00:00 1970 From: joel at joelfernandes.org (Joel Fernandes) Date: Mon, 15 Apr 2019 10:05:31 -0400 Subject: [PATCH v5 1/3] Provide in-kernel headers to make extending kernel easier In-Reply-To: References: <20190320163116.39275-1-joel@joelfernandes.org> <20190408203601.GF133872@google.com> <20190411031540.ehezr6kq7ouobpzx@ast-mbp.dhcp.thefacebook.com> Message-ID: <20190415140531.GB205801@google.com> On Sun, Apr 14, 2019 at 12:38:34PM -0700, Olof Johansson wrote: > On Wed, Apr 10, 2019 at 8:15 PM Alexei Starovoitov > wrote: > > > > On Wed, Apr 10, 2019 at 09:34:49AM -0700, Olof Johansson wrote: > > > On Wed, Apr 10, 2019 at 8:51 AM Joel Fernandes wrote: > > > > > > > > On Wed, Apr 10, 2019 at 11:07 AM Olof Johansson wrote: > > > > [snip] > > > > > > > Wouldn't it be more convenient to provide it in a standardized format > > > > > > > such that you won't have to take an additional step, and always have > > > > > > > This is that form IMO. > > ... > > > Compared to: > > > - Extract tarball > > > - Build and load > > > - Remove file tree from filesystem > > > > I think there are too many assumptions in this thread in regard to what > > is more convenient for user space. > > I think bcc should try to avoid extracting tarball into file system. > > For example libbcc can uncompress kheader.tar.xz into virtual file system > > of clang front-end. It's more or less std::map > > with key=path, value=content of the file. Access to such in-memory > > 'files' is obviously faster than doing open/read syscalls. > > I think performance is a red herring, especially since you have to > uncompress it on every compiler invocation. With this you'd need to > read/touch/write _all_ header files, not just the one your current > compiler invocation will use. > > In the grand scheme of things, open/mmap syscalls wouldn't necessarily > be slower. Agreed. > > bcc already uses this approach for some bcc internal 'files' that > > it passes to clang during compilation. > > All of /proc/kheaders.tar.xz files can be passed the same way > > without extracting them into real file system. > > This is now a circular argument. Joel was stating that the plain text > headers took up too much memory, but now it's preferred to create such > filesystem in userspace memory on *every compiler invocation*? > That's... not better. And definitely worse if you want to compile in > parallel. The BCC patch does not extract purely into memory, but uses temporary directory: https://github.com/iovisor/bcc/pull/2312 I believe this is a good approach. > From my perspective, this is where we're at: > > This patch seems to have been met with a lot of responses in the tone > of "this is not an appealing solution". Meanwhile, some of the > suggested alternative solutions have not worked out, and we are now at > a point where there's less interest in exploring alternatives and > arguments to merge as-is with only minor adjustments. > > I understand the desire to solve this. It'd be really convenient to > have a way to runtime build against the same structure layouts that > the kernel was built with. But I haven't heard anyone say that they About structure layouts, I'm assuming you mean compiler generated debug info. That does not work for eBPF tools as was mentioned previously in these threads: https://lkml.org/lkml/2019/3/11/1358 https://lkml.org/lkml/2019/3/11/1363 > *like* the solution proposed, and I haven't seen many of those > expressing concerns being converted to support it. IMHO there has been good number of people on both sides of the argument. If it were as strong of an opposition as you think, then I would have personally not wanted this merged tbh. We do want a solution that is clean and works, and I think this is a candidate. [snip] > I'd be a *lot* less hesitant if this went into debugfs or another > location than /proc, which is one of the most regression-sensitive > interfaces we have in the kernel. The solution should be regression-sensitive imho, we don't want the tracing tools to break, people use them. And their usability and robustness is important which prompted these patches. For some time we have been hosting downloadable headers for popular kernels (such as LTS versions) to workaround this issue, but this both a maintenance issue and non-scalable, and not robust (someone boots a custom kernel or we forget to update headers for a future LTS release and the tools break). thanks, - Joel From mboxrd@z Thu Jan 1 00:00:00 1970 From: joel@joelfernandes.org (Joel Fernandes) Date: Mon, 15 Apr 2019 10:05:31 -0400 Subject: [PATCH v5 1/3] Provide in-kernel headers to make extending kernel easier In-Reply-To: References: <20190320163116.39275-1-joel@joelfernandes.org> <20190408203601.GF133872@google.com> <20190411031540.ehezr6kq7ouobpzx@ast-mbp.dhcp.thefacebook.com> Message-ID: <20190415140531.GB205801@google.com> Content-Type: text/plain; charset="UTF-8" Message-ID: <20190415140531.CvSnKYbAhIRf3Jcjj32TqLDIPWnQHd7wYK0sk9OScGE@z> On Sun, Apr 14, 2019@12:38:34PM -0700, Olof Johansson wrote: > On Wed, Apr 10, 2019 at 8:15 PM Alexei Starovoitov > wrote: > > > > On Wed, Apr 10, 2019@09:34:49AM -0700, Olof Johansson wrote: > > > On Wed, Apr 10, 2019@8:51 AM Joel Fernandes wrote: > > > > > > > > On Wed, Apr 10, 2019@11:07 AM Olof Johansson wrote: > > > > [snip] > > > > > > > Wouldn't it be more convenient to provide it in a standardized format > > > > > > > such that you won't have to take an additional step, and always have > > > > > > > This is that form IMO. > > ... > > > Compared to: > > > - Extract tarball > > > - Build and load > > > - Remove file tree from filesystem > > > > I think there are too many assumptions in this thread in regard to what > > is more convenient for user space. > > I think bcc should try to avoid extracting tarball into file system. > > For example libbcc can uncompress kheader.tar.xz into virtual file system > > of clang front-end. It's more or less std::map > > with key=path, value=content of the file. Access to such in-memory > > 'files' is obviously faster than doing open/read syscalls. > > I think performance is a red herring, especially since you have to > uncompress it on every compiler invocation. With this you'd need to > read/touch/write _all_ header files, not just the one your current > compiler invocation will use. > > In the grand scheme of things, open/mmap syscalls wouldn't necessarily > be slower. Agreed. > > bcc already uses this approach for some bcc internal 'files' that > > it passes to clang during compilation. > > All of /proc/kheaders.tar.xz files can be passed the same way > > without extracting them into real file system. > > This is now a circular argument. Joel was stating that the plain text > headers took up too much memory, but now it's preferred to create such > filesystem in userspace memory on *every compiler invocation*? > That's... not better. And definitely worse if you want to compile in > parallel. The BCC patch does not extract purely into memory, but uses temporary directory: https://github.com/iovisor/bcc/pull/2312 I believe this is a good approach. > From my perspective, this is where we're at: > > This patch seems to have been met with a lot of responses in the tone > of "this is not an appealing solution". Meanwhile, some of the > suggested alternative solutions have not worked out, and we are now at > a point where there's less interest in exploring alternatives and > arguments to merge as-is with only minor adjustments. > > I understand the desire to solve this. It'd be really convenient to > have a way to runtime build against the same structure layouts that > the kernel was built with. But I haven't heard anyone say that they About structure layouts, I'm assuming you mean compiler generated debug info. That does not work for eBPF tools as was mentioned previously in these threads: https://lkml.org/lkml/2019/3/11/1358 https://lkml.org/lkml/2019/3/11/1363 > *like* the solution proposed, and I haven't seen many of those > expressing concerns being converted to support it. IMHO there has been good number of people on both sides of the argument. If it were as strong of an opposition as you think, then I would have personally not wanted this merged tbh. We do want a solution that is clean and works, and I think this is a candidate. [snip] > I'd be a *lot* less hesitant if this went into debugfs or another > location than /proc, which is one of the most regression-sensitive > interfaces we have in the kernel. The solution should be regression-sensitive imho, we don't want the tracing tools to break, people use them. And their usability and robustness is important which prompted these patches. For some time we have been hosting downloadable headers for popular kernels (such as LTS versions) to workaround this issue, but this both a maintenance issue and non-scalable, and not robust (someone boots a custom kernel or we forget to update headers for a future LTS release and the tools break). thanks, - Joel