[PATCH v2] Documentation/locking/lockdep: Drop last two chars of sample states

2019-03-01 Thread Geert Uytterhoeven
Since the removal of FS_RECLAIM annotations, lockdep states contain four
characters, not six.

Fixes: e5684bbfc3f03480 ("Documentation/locking/lockdep: Update info about 
states")
Fixes: d92a8cfcb37ecd13 ("locking/lockdep: Rework FS_RECLAIM annotation")
Signed-off-by: Geert Uytterhoeven 
---
v2:
  - Fix silly before/after inversion in patch description.
---
 Documentation/RCU/lockdep-splat.txt  | 6 +++---
 Documentation/locking/lockdep-design.txt | 4 ++--
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/Documentation/RCU/lockdep-splat.txt 
b/Documentation/RCU/lockdep-splat.txt
index 238e9f61352f6187..9423b633526d14df 100644
--- a/Documentation/RCU/lockdep-splat.txt
+++ b/Documentation/RCU/lockdep-splat.txt
@@ -24,11 +24,11 @@ other info that might help us debug this:
 
 rcu_scheduler_active = 1, debug_locks = 0
 3 locks held by scsi_scan_6/1552:
- #0:  (&shost->scan_mutex){+.+.+.}, at: []
+ #0:  (&shost->scan_mutex){+.+.}, at: []
 scsi_scan_host_selected+0x5a/0x150
- #1:  (&eq->sysfs_lock){+.+...}, at: []
+ #1:  (&eq->sysfs_lock){+.+.}, at: []
 elevator_exit+0x22/0x60
- #2:  (&(&q->__queue_lock)->rlock){-.-...}, at: []
+ #2:  (&(&q->__queue_lock)->rlock){-.-.}, at: []
 cfq_exit_queue+0x43/0x190
 
 stack backtrace:
diff --git a/Documentation/locking/lockdep-design.txt 
b/Documentation/locking/lockdep-design.txt
index 49f58a07ee7b19c8..39fae143c9cbf5ff 100644
--- a/Documentation/locking/lockdep-design.txt
+++ b/Documentation/locking/lockdep-design.txt
@@ -45,10 +45,10 @@ When locking rules are violated, these state bits are 
presented in the
 locking error messages, inside curlies. A contrived example:
 
modprobe/2287 is trying to acquire lock:
-(&sio_locks[i].lock){-.-...}, at: [] mutex_lock+0x21/0x24
+(&sio_locks[i].lock){-.-.}, at: [] mutex_lock+0x21/0x24
 
but task is already holding lock:
-(&sio_locks[i].lock){-.-...}, at: [] mutex_lock+0x21/0x24
+(&sio_locks[i].lock){-.-.}, at: [] mutex_lock+0x21/0x24
 
 
 The bit position indicates STATE, STATE-read, for each of the states listed
-- 
2.17.1



Re: [PATCH v3 1/2] Provide in-kernel headers for making it easy to extend the kernel

2019-03-01 Thread Qais Yousef
On 02/28/19 22:26, Joel Fernandes wrote:
> On Fri, Mar 01, 2019 at 11:28:26AM +0900, Masami Hiramatsu wrote:
> > Hi Joel,
> 
> Hi Masami,
> 
> > On Thu, 28 Feb 2019 10:00:54 -0500
> > Joel Fernandes  wrote:
> > 
> > > > Hmm, isn't it easier to add kernel-headers package on Android?
> > > 
> > > I have already been down that road. In the Android ecosystem, the Android
> > > teams only provide a "userspace system image" which goes on the system
> > > partition of the flash (and a couple other images are also provided but
> > > system is the main one). The system image cannot contain GPL source code. 
> > > It
> > > is also not possible to put kernel headers for every kernel version on the
> > > system images that ship and is not practical. Android boots on 1000s of 
> > > forked
> > > kernels. It does not make sense to provide headers on the system image for
> > > every kernel version and I already had many discussions on the subject 
> > > with
> > > the teams, it is something that is just not done. Now for kernel modules,
> > > there's another image called the "vendor image" which is flashed onto the
> > > vendor parition, this is where kernel modules go.  This vendor image is 
> > > not
> > > provided by Google for non-Pixel devices. So we have no control over what
> > > goes there BUT we do know that kernel modules that are enabled will go 
> > > there,
> > > and we do have control over enforcing that certain kernel modules should 
> > > be
> > > built and available as they are mandatory for Android to function 
> > > properly.
> > > We would also possibly make it a built-in option as well. Anyway my point 
> > > is
> > > keeping it in the kernel is really the easiest and the smartest choice 
> > > IMO.
> > 
> > Sorry, I'm not convinced yet. This sounds like "because Android decided not
> > to put the header files on vendor partition, but kernel module is OK"
> > Why don't google ask vendors to put their kernel headers (or header tarball)
> > on vendor partition instead?
> 
> May be Google can do that, but I think you missed the point of the patches.
> Making it a module is not mandatory, people can build it into the kernel as
> well (CONFIG_IKHEADERS_PROC=y). In this case, the proc entry will be
> available on boot without any dependency on the filesystem. If you go through
> the other threads such as folks from ARM who replied, they just boot a kernel
> image for debug purpose and want headers on device available without any
> additional step of copying headers to the filesystem. And folks from Google
> also said that they wanted a built-in option.

Yes I do see this patch a useful addition. When you need the header and you're
dealing with multiple devices and kernel trees and versions - having the
headers part of the kernel just makes it easier to use them when you need them.
Especially sometimes when you change a config option (like enabling a new
syscall e.g: bpf) it's very easy to forget that the headers has changed as well
and you need to push an updated copy.

FWIW most of the time I run on non-android systems for development/debugging
purposes. I use the built-in version although I can see a module version
helpful too. You can save some space if your kernel image partition is small
and it's easy to install the module along with all other modules when
rebuilding the kernel than remembering to update the headers. It's a (very)
nice convenience.

--
Qais Yousef

> 
> There are many usecases for this, I have often run into issues with Linux
> over the years not only with Android, but other distros, where I boot custom
> kernels with no linux-headers package. This is quite painful. It is
> convenient to have it as /proc file since the file is dependent on kernel
> being booted up and this will work across all Linux distros and systems. I
> feel that if you can keep an open mind about it, you will see that a lot of
> people will use this feature if it is accepted and there is a lot of positive
> feedback in earlier posts of this set.
> 
> > > > > The code to read the headers is based on /proc/config.gz code and uses
> > > > > the same technique to embed the headers.
> > > > > 
> > > > > To build a module, the below steps have been tested on an x86 machine:
> > > > > modprobe kheaders
> > > > > rm -rf $HOME/headers
> > > > > mkdir -p $HOME/headers
> > > > > tar -xvf /proc/kheaders.tar.xz -C $HOME/headers >/dev/null
> > > > > cd my-kernel-module
> > > > > make -C $HOME/headers M=$(pwd) modules
> > > > > rmmod kheaders
> > > > 
> > > > It seems a bit complex, but no difference from compared with carrying
> > > > kheaders.tar.gz. I think we would better have a psudo filesystem
> > > > which can mount this compressed header file directly :) Then it becomes
> > > > simpler, like
> > > > 
> > > > modprobe headerfs
> > > > mkdir $HOME/headers
> > > > mount -t headerfs $HOME/headers
> > > > 
> > > > And this doesn't consume any disk-space.
> > > 
> > > I felt using a compressed tar is really the easiest way be

Re: [PATCH v2] Documentation/locking/lockdep: Drop last two chars of sample states

2019-03-01 Thread Will Deacon
On Fri, Mar 01, 2019 at 10:40:52AM +0100, Geert Uytterhoeven wrote:
> Since the removal of FS_RECLAIM annotations, lockdep states contain four
> characters, not six.
> 
> Fixes: e5684bbfc3f03480 ("Documentation/locking/lockdep: Update info about 
> states")
> Fixes: d92a8cfcb37ecd13 ("locking/lockdep: Rework FS_RECLAIM annotation")
> Signed-off-by: Geert Uytterhoeven 
> ---
> v2:
>   - Fix silly before/after inversion in patch description.
> ---
>  Documentation/RCU/lockdep-splat.txt  | 6 +++---
>  Documentation/locking/lockdep-design.txt | 4 ++--
>  2 files changed, 5 insertions(+), 5 deletions(-)

Looks sensible to me:

Acked-by: Will Deacon 

Will


[PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel

2019-03-01 Thread Joel Fernandes (Google)
Introduce in-kernel headers and other artifacts which are made available
as an archive through proc (/proc/kheaders.tar.xz 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 feature is also buildable as a module just in case the user desires
it not being part of the kernel image. This makes it possible to load
and unload the headers on demand. A tracing program, or a kernel module
builder can load the module, do its operations, and then unload the
module to save kernel memory. The total memory needed is 3.8MB.

The code to read the headers is based on /proc/config.gz code and uses
the same technique to embed the headers.

To build a module, the below steps have been tested on an x86 machine:
modprobe kheaders
rm -rf $HOME/headers
mkdir -p $HOME/headers
tar -xvf /proc/kheaders.tar.xz -C $HOME/headers >/dev/null
cd my-kernel-module
make -C $HOME/headers M=$(pwd) modules
rmmod kheaders

Additional notes:
(1) external modules must be built on the same arch as the host that
built vmlinux. This can be done either in a qemu emulated chroot on the
target, or natively. This is due to host arch dependency of kernel
scripts.

(2)
A limitation of module building with this is, since Module.symvers is
not available in the archive due to a cyclic dependency with building of
the archive into the kernel or module binaries, the modules built using
the archive will not contain symbol versioning (modversion). This is
usually not an issue since the idea of this patch is to build a kernel
module on the fly and load it into the same kernel. An appropriate
warning is already printed by the kernel to alert the user of modules
not having modversions when built using the archive. For building with
modversions, the user can use traditional header packages. For our
tracing usecases, we build modules on the fly with this so it is not a
concern.

(3) I have left IKHD_ST and IKHD_ED markers as is to facilitate
future patches that would extract the headers from a kernel or module
image.

Signed-off-by: Joel Fernandes (Google) 
---

Changes since v3:
- Blank tar was being generated because of a one line I
  forgot to push. It is updated now.
- Added module.lds since arm64 needs it to build modules.

Changes since v2:
(Thanks to Masahiro Yamada for several excellent suggestions)
- Added support for out of tree builds.
- Added incremental build support bringing down build time of
  incremental builds from 50 seconds to 5 seconds.
- Fixed various small nits / cleanups.
- clean ups to kheaders.c pointed by Alexey Dobriyan.
- Fixed MODULE_LICENSE in test module and kheaders.c
- Dropped Module.symvers from archive due to circular dependency.

Changes since v1:
- removed IKH_EXTRA variable, not needed (Masahiro Yamada)
- small fix ups to selftest
   - added target to main Makefile etc
   - added MODULE_LICENSE to test module
   - made selftest more quiet

Changes since RFC:
Both changes bring size down to 3.8MB:
- use xz for compression
- strip comments except SPDX lines
- Call out the module name in Kconfig
- Also added selftests in second patch to ensure headers are always
working.

Other notes:
By the way I still see this error (without the patch) when doing a clean
build: Makefile:594: include/config/auto.conf: No such file or directory

It appears to be because of commit 0a16d2e8cb7e ("kbuild: use 'include'
directive to load auto.conf from top Makefile")

 Documentation/dontdiff|  1 +
 init/Kconfig  | 11 ++
 kernel/.gitignore |  3 ++
 kernel/Makefile   | 37 +++
 kernel/kheaders.c | 72 
 scripts/gen_ikh_data.sh   | 78 +++
 scripts/strip-comments.pl |  8 
 7 files changed, 210 insertions(+)
 create mode 100644 kernel/kheaders.c
 create mode 100755 scripts/gen_ikh_data.sh
 create mode 100755 scripts/strip-comments.pl

diff --git a/Documentation/dontdiff b/Documentation/dontdiff
index 2228fcc8e29f..05a2319ee2a2 100644
--- a/Documentation/dontdiff
+++ b/Documentation/dontdiff
@@ -151,6 +151,7 @@ int8.c
 kallsyms
 kconfig
 keywords.c
+kheaders_data.h*
 ksym.c*
 ksym.h*
 kxgettext
diff --git 

[PATCH v4 2/2] Add selftests for module build using in-kernel headers

2019-03-01 Thread Joel Fernandes (Google)
This test tries to build a module successfully using the in-kernel
headers found in /proc/kheaders.tar.xz.

Verified pass and fail scenarios by running:
make -C tools/testing/selftests TARGETS=kheaders run_tests

Signed-off-by: Joel Fernandes (Google) 
---
 tools/testing/selftests/Makefile  |  1 +
 tools/testing/selftests/kheaders/Makefile |  5 +
 tools/testing/selftests/kheaders/config   |  1 +
 .../kheaders/run_kheaders_modbuild.sh | 18 +
 .../selftests/kheaders/testmod/Makefile   |  3 +++
 .../testing/selftests/kheaders/testmod/test.c | 20 +++
 6 files changed, 48 insertions(+)
 create mode 100644 tools/testing/selftests/kheaders/Makefile
 create mode 100644 tools/testing/selftests/kheaders/config
 create mode 100755 tools/testing/selftests/kheaders/run_kheaders_modbuild.sh
 create mode 100644 tools/testing/selftests/kheaders/testmod/Makefile
 create mode 100644 tools/testing/selftests/kheaders/testmod/test.c

diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
index 400ee81a3043..5a9287fddd0d 100644
--- a/tools/testing/selftests/Makefile
+++ b/tools/testing/selftests/Makefile
@@ -20,6 +20,7 @@ TARGETS += intel_pstate
 TARGETS += ipc
 TARGETS += ir
 TARGETS += kcmp
+TARGETS += kheaders
 TARGETS += kvm
 TARGETS += lib
 TARGETS += membarrier
diff --git a/tools/testing/selftests/kheaders/Makefile 
b/tools/testing/selftests/kheaders/Makefile
new file mode 100644
index ..51035ab0732b
--- /dev/null
+++ b/tools/testing/selftests/kheaders/Makefile
@@ -0,0 +1,5 @@
+# SPDX-License-Identifier: GPL-2.0
+
+TEST_PROGS := run_kheaders_modbuild.sh
+
+include ../lib.mk
diff --git a/tools/testing/selftests/kheaders/config 
b/tools/testing/selftests/kheaders/config
new file mode 100644
index ..5221f9fb5e79
--- /dev/null
+++ b/tools/testing/selftests/kheaders/config
@@ -0,0 +1 @@
+CONFIG_IKHEADERS_PROC=y
diff --git a/tools/testing/selftests/kheaders/run_kheaders_modbuild.sh 
b/tools/testing/selftests/kheaders/run_kheaders_modbuild.sh
new file mode 100755
index ..f001568e08b0
--- /dev/null
+++ b/tools/testing/selftests/kheaders/run_kheaders_modbuild.sh
@@ -0,0 +1,18 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0
+
+HEADERS_XZ=/proc/kheaders.tar.xz
+TMP_DIR_HEADERS=$(mktemp -d)
+TMP_DIR_MODULE=$(mktemp -d)
+SPATH="$(dirname "$(readlink -f "$0")")"
+
+tar -xvf $HEADERS_XZ -C $TMP_DIR_HEADERS > /dev/null
+
+cp -r $SPATH/testmod $TMP_DIR_MODULE/
+
+pushd $TMP_DIR_MODULE/testmod > /dev/null
+make -C $TMP_DIR_HEADERS M=$(pwd) modules
+popd > /dev/null
+
+rm -rf $TMP_DIR_HEADERS
+rm -rf $TMP_DIR_MODULE
diff --git a/tools/testing/selftests/kheaders/testmod/Makefile 
b/tools/testing/selftests/kheaders/testmod/Makefile
new file mode 100644
index ..7083e28706e8
--- /dev/null
+++ b/tools/testing/selftests/kheaders/testmod/Makefile
@@ -0,0 +1,3 @@
+# SPDX-License-Identifier: GPL-2.0
+
+obj-m += test.o
diff --git a/tools/testing/selftests/kheaders/testmod/test.c 
b/tools/testing/selftests/kheaders/testmod/test.c
new file mode 100644
index ..6eb0b8492ffa
--- /dev/null
+++ b/tools/testing/selftests/kheaders/testmod/test.c
@@ -0,0 +1,20 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include 
+#include 
+#include 
+
+static int __init hello_init(void)
+{
+   printk(KERN_INFO "Hello, world\n");
+   return 0;
+}
+
+static void __exit hello_exit(void)
+{
+   printk(KERN_INFO "Goodbye, world\n");
+}
+
+module_init(hello_init);
+module_exit(hello_exit);
+MODULE_LICENSE("GPL v2");
-- 
2.21.0.352.gf09ad66450-goog



Re: [PATCH v10 07/12] fs, arm64: untag user pointers in fs/userfaultfd.c

2019-03-01 Thread Catalin Marinas
On Tue, Feb 26, 2019 at 03:39:08PM +0100, Andrey Konovalov wrote:
> On Sat, Feb 23, 2019 at 12:06 AM Dave Hansen  wrote:
> >
> > On 2/22/19 4:53 AM, Andrey Konovalov wrote:
> > > userfaultfd_register() and userfaultfd_unregister() use provided user
> > > pointers for vma lookups, which can only by done with untagged pointers.
> >
> > So, we have to patch all these sites before the tagged values get to the
> > point of hitting the vma lookup functions.  Dumb question: Why don't we
> > just patch the vma lookup functions themselves instead of all of these
> > callers?
> 
> That might be a working approach as well. We'll still need to fix up
> places where the vma fields are accessed directly. Catalin, what do
> you think?

Most callers of find_vma*() always follow it by a check of
vma->vma_start against some tagged address ('end' in the
userfaultfd_(un)register()) case. So it's not sufficient to untag it in
find_vma().

-- 
Catalin


Re: [PATCH v2] Documentation/locking/lockdep: Drop last two chars of sample states

2019-03-01 Thread Paul E. McKenney
On Fri, Mar 01, 2019 at 10:40:52AM +0100, Geert Uytterhoeven wrote:
> Since the removal of FS_RECLAIM annotations, lockdep states contain four
> characters, not six.
> 
> Fixes: e5684bbfc3f03480 ("Documentation/locking/lockdep: Update info about 
> states")
> Fixes: d92a8cfcb37ecd13 ("locking/lockdep: Rework FS_RECLAIM annotation")
> Signed-off-by: Geert Uytterhoeven 

Acked-by: Paul E. McKenney 

Or let me know if you would rather me take this via the -rcu tree, either
way works for me!

Thanx, Paul

> ---
> v2:
>   - Fix silly before/after inversion in patch description.
> ---
>  Documentation/RCU/lockdep-splat.txt  | 6 +++---
>  Documentation/locking/lockdep-design.txt | 4 ++--
>  2 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/RCU/lockdep-splat.txt 
> b/Documentation/RCU/lockdep-splat.txt
> index 238e9f61352f6187..9423b633526d14df 100644
> --- a/Documentation/RCU/lockdep-splat.txt
> +++ b/Documentation/RCU/lockdep-splat.txt
> @@ -24,11 +24,11 @@ other info that might help us debug this:
>  
>  rcu_scheduler_active = 1, debug_locks = 0
>  3 locks held by scsi_scan_6/1552:
> - #0:  (&shost->scan_mutex){+.+.+.}, at: []
> + #0:  (&shost->scan_mutex){+.+.}, at: []
>  scsi_scan_host_selected+0x5a/0x150
> - #1:  (&eq->sysfs_lock){+.+...}, at: []
> + #1:  (&eq->sysfs_lock){+.+.}, at: []
>  elevator_exit+0x22/0x60
> - #2:  (&(&q->__queue_lock)->rlock){-.-...}, at: []
> + #2:  (&(&q->__queue_lock)->rlock){-.-.}, at: []
>  cfq_exit_queue+0x43/0x190
>  
>  stack backtrace:
> diff --git a/Documentation/locking/lockdep-design.txt 
> b/Documentation/locking/lockdep-design.txt
> index 49f58a07ee7b19c8..39fae143c9cbf5ff 100644
> --- a/Documentation/locking/lockdep-design.txt
> +++ b/Documentation/locking/lockdep-design.txt
> @@ -45,10 +45,10 @@ When locking rules are violated, these state bits are 
> presented in the
>  locking error messages, inside curlies. A contrived example:
>  
> modprobe/2287 is trying to acquire lock:
> -(&sio_locks[i].lock){-.-...}, at: [] mutex_lock+0x21/0x24
> +(&sio_locks[i].lock){-.-.}, at: [] mutex_lock+0x21/0x24
>  
> but task is already holding lock:
> -(&sio_locks[i].lock){-.-...}, at: [] mutex_lock+0x21/0x24
> +(&sio_locks[i].lock){-.-.}, at: [] mutex_lock+0x21/0x24
>  
>  
>  The bit position indicates STATE, STATE-read, for each of the states listed
> -- 
> 2.17.1
> 



Re: [PATCH v3 1/2] Provide in-kernel headers for making it easy to extend the kernel

2019-03-01 Thread Joel Fernandes
On Fri, Mar 01, 2019 at 03:25:05PM +0900, Masahiro Yamada wrote:
[...]
> > > I am guessing the user will run these commands
> > > on the target system.
> > > In other words, external modules are native-compiled.
> > > So,
> > >
> > >   target-arch: arm64
> > >   host-arch:   arm64
> > >
> > >
> > > Is this correct?
> > >
> > >
> > > If I understood the assumed use-case correctly,
> > > kheaders.tar.xw will contain host-programs compiled for x86,
> > > which will not work on the target system.
> > >
> >
> > You are right, the above commands in the commit message work only if the
> > host/target are same arch due to scripts.
> >
> > However we can build with arm64 device connected to a host, like this (which
> > I tested):
> >
> > adb shell modprobe kheaders; adb pull /proc/kheaders.tar.xz
> > rm -rf $HOME/headers; mkdir -p $HOME/headers
> > tar -xvf /proc/kheaders.tar.xz -C $HOME/headers >/dev/null
> > cd my-kernel-module
> > make -C $HOME/headers M=$(pwd) ARCH=arm64 CROSS_COMPILE=aarch64- modules
> > adb push test.ko /data/; adb shell rmmod kheaders
> >
> > The other way we can make this work is using x86 usermode emulation inside a
> > chroot on the Android device which will make the earlier commands work. One
> > thing to note is that Android also runs on x86 hardware so the commands in
> > the commit message will work even for x86 Android targets already.
> >
> > Also note that this the "module building" part is really only one of the
> > usecases. eBPF is another which needs the headers - and the headers are vast
> > majority of the archive. Headers take 3.1MB out of 3.6MB of the archive on
> > arm64 builds.
> >
> > How do you want to proceed here, should I mention these points in the commit
> > message?
> 
> 
> 
> I do not request a re-spin just for a matter of commit log,
> but this version produces an empty tarball.
> So, you will have a chance to update the patch anyway.
> 
> In the next version, it would be nice to note that
> "external modules must be built on the same host arch
> as built vmlinux".

Ok, I respun it with 1 more minor nit for arm64 building. Please take a look.

> Let me ask one more question.
> 
> I guess this patch is motivated by
> how difficult to convey kernel headers
> from vendors to users.
> 
> In that situation, how will the user find
> the right compiler to use for building external modules?
> 
> 
> 
> 
> Greg KH said:
> 
> We don't ever support the system of loading a module built with anything
> other than the _exact_ same compiler than the kernel was.
> 
> 
> For the full context, see this:
> https://lore.kernel.org/patchwork/patch/836247/#1031547

IMO this issue is not related to this patch but is just an issue with
building external modules in general. It is up to the user to use the right
compiler, etc. I will let Greg comment more on that.

thanks,

 - Joel



Re: [PATCH v3 1/2] Provide in-kernel headers for making it easy to extend the kernel

2019-03-01 Thread Joel Fernandes
On Fri, Mar 01, 2019 at 04:03:09PM +0900, Masami Hiramatsu wrote:
> Hi Joel,
> 
> On Thu, 28 Feb 2019 22:26:11 -0500
> Joel Fernandes  wrote:
> 
> > On Fri, Mar 01, 2019 at 11:28:26AM +0900, Masami Hiramatsu wrote:
[..]
> > There are many usecases for this, I have often run into issues with Linux
> > over the years not only with Android, but other distros, where I boot custom
> > kernels with no linux-headers package. This is quite painful. It is
> > convenient to have it as /proc file since the file is dependent on kernel
> > being booted up and this will work across all Linux distros and systems. I
> > feel that if you can keep an open mind about it, you will see that a lot of
> > people will use this feature if it is accepted and there is a lot of 
> > positive
> > feedback in earlier posts of this set.
> 
> I don't complain about having headers for custom boot kernel. I agree with you
> that having kernel headers for debugging is always good. :)
> So google recommends built-in, it is reasonable.

Ok, thanks :)

> > > > > > The code to read the headers is based on /proc/config.gz code and 
> > > > > > uses
> > > > > > the same technique to embed the headers.
> > > > > > 
> > > > > > To build a module, the below steps have been tested on an x86 
> > > > > > machine:
> > > > > > modprobe kheaders
> > > > > > rm -rf $HOME/headers
> > > > > > mkdir -p $HOME/headers
> > > > > > tar -xvf /proc/kheaders.tar.xz -C $HOME/headers >/dev/null
> > > > > > cd my-kernel-module
> > > > > > make -C $HOME/headers M=$(pwd) modules
> > > > > > rmmod kheaders
> > > > > 
> > > > > It seems a bit complex, but no difference from compared with carrying
> > > > > kheaders.tar.gz. I think we would better have a psudo filesystem
> > > > > which can mount this compressed header file directly :) Then it 
> > > > > becomes
> > > > > simpler, like
> > > > > 
> > > > > modprobe headerfs
> > > > > mkdir $HOME/headers
> > > > > mount -t headerfs $HOME/headers
> > > > > 
> > > > > And this doesn't consume any disk-space.
> > > > 
> > > > I felt using a compressed tar is really the easiest way because of all 
> > > > the
> > > > tools are already available.
> > > 
> > > As I asked above, if the pure tarball is useful, you can simply ask 
> > > vendors
> > > to put the header tarball on their vendor directory. I feel making it as
> > > a module is not a right way.
> > 
> > I don't see what is the drawback of making it a module, it makes it well
> > integrated into kernel build and ecosystem. I also didn't see any
> > justification you're providing about why it cannot be a module. If you go
> > through this and earlier threads, a lot of people are Ok with having a 
> > module
> > option. And I asked several top kernel maintainers at LPC and many people
> > suggested having it as a module.
> 
> I meant, if we have a tarball, we don't need any operation of 
> loading/unloading
> kmodules. But if we have this as built-in, yes, this would be much easier to
> deploy it to device.
> 
> Anyway, having that option (make it as a module) is not bad. IMHO, that may
> be more complicated than just have a tarball file, but it is a user's choice.
> 
> OK, now I understand it.

Sounds good. :) Just sent out v4.

thanks,

 - Joel



Re: [PATCH v10 07/12] fs, arm64: untag user pointers in fs/userfaultfd.c

2019-03-01 Thread Dave Hansen
On 3/1/19 8:59 AM, Catalin Marinas wrote:
>>> So, we have to patch all these sites before the tagged values get to the
>>> point of hitting the vma lookup functions.  Dumb question: Why don't we
>>> just patch the vma lookup functions themselves instead of all of these
>>> callers?
>> That might be a working approach as well. We'll still need to fix up
>> places where the vma fields are accessed directly. Catalin, what do
>> you think?
> Most callers of find_vma*() always follow it by a check of
> vma->vma_start against some tagged address ('end' in the
> userfaultfd_(un)register()) case. So it's not sufficient to untag it in
> find_vma().

If that's truly the common case, sounds like we should have a find_vma()
that does the vma_end checking as well.  Then at least the common case
would not have to worry about tagging.


Re: [PATCH 0/2] doc: net: ieee802154: move from plain text to rst

2019-03-01 Thread David Miller
From: Stefan Schmidt 
Date: Wed, 27 Feb 2019 22:20:58 +0100

> Hello Jon.
> 
> On 27.02.19 21:18, Jonathan Corbet wrote:
>> On Wed, 27 Feb 2019 20:59:12 +0100
>> Stefan Schmidt  wrote:
>> 
>>> The patches are based on net-next, but they only touch the networking book 
>>> so I
>>> would not expect and trouble. From what I have seen they would go through
>>> Jonathan's tree after being acked by Dave? If you want this patches against 
>>> a
>>> different tree let me know.
>> 
>> Usually Dave takes networking documentation patches directly, so that is
>> what I would expect here.
> 
> OK, so I got that wrong. Works for me. Dave, you want to take them
> directly, if nothing else comes up during review, or should I apply them
> to my tree and send them with the next pull request?

I'll take them, thanks.


Re: [PATCH 0/2] doc: net: ieee802154: move from plain text to rst

2019-03-01 Thread David Miller
From: Stefan Schmidt 
Date: Wed, 27 Feb 2019 20:59:12 +0100

> The ieee802154 subsystem doc was still in plain text. With the networking book
> taking shape I thought it was time to do the first step and move it over to 
> rst.
> This really is only the minimal conversion. I need to take some time to update
> and extend the docs.
> 
> The patches are based on net-next, but they only touch the networking book so 
> I
> would not expect and trouble. From what I have seen they would go through
> Jonathan's tree after being acked by Dave? If you want this patches against a
> different tree let me know.

Series applied, thanks Stefan.


Re: [PATCH v3 1/2] Provide in-kernel headers for making it easy to extend the kernel

2019-03-01 Thread Masahiro Yamada
On Sat, Mar 2, 2019 at 3:03 AM Joel Fernandes  wrote:
>
> On Fri, Mar 01, 2019 at 03:25:05PM +0900, Masahiro Yamada wrote:
> [...]
> > > > I am guessing the user will run these commands
> > > > on the target system.
> > > > In other words, external modules are native-compiled.
> > > > So,
> > > >
> > > >   target-arch: arm64
> > > >   host-arch:   arm64
> > > >
> > > >
> > > > Is this correct?
> > > >
> > > >
> > > > If I understood the assumed use-case correctly,
> > > > kheaders.tar.xw will contain host-programs compiled for x86,
> > > > which will not work on the target system.
> > > >
> > >
> > > You are right, the above commands in the commit message work only if the
> > > host/target are same arch due to scripts.
> > >
> > > However we can build with arm64 device connected to a host, like this 
> > > (which
> > > I tested):
> > >
> > > adb shell modprobe kheaders; adb pull /proc/kheaders.tar.xz
> > > rm -rf $HOME/headers; mkdir -p $HOME/headers
> > > tar -xvf /proc/kheaders.tar.xz -C $HOME/headers >/dev/null
> > > cd my-kernel-module
> > > make -C $HOME/headers M=$(pwd) ARCH=arm64 CROSS_COMPILE=aarch64- modules
> > > adb push test.ko /data/; adb shell rmmod kheaders
> > >
> > > The other way we can make this work is using x86 usermode emulation 
> > > inside a
> > > chroot on the Android device which will make the earlier commands work. 
> > > One
> > > thing to note is that Android also runs on x86 hardware so the commands in
> > > the commit message will work even for x86 Android targets already.
> > >
> > > Also note that this the "module building" part is really only one of the
> > > usecases. eBPF is another which needs the headers - and the headers are 
> > > vast
> > > majority of the archive. Headers take 3.1MB out of 3.6MB of the archive on
> > > arm64 builds.
> > >
> > > How do you want to proceed here, should I mention these points in the 
> > > commit
> > > message?
> >
> >
> >
> > I do not request a re-spin just for a matter of commit log,
> > but this version produces an empty tarball.
> > So, you will have a chance to update the patch anyway.
> >
> > In the next version, it would be nice to note that
> > "external modules must be built on the same host arch
> > as built vmlinux".
>
> Ok, I respun it with 1 more minor nit for arm64 building. Please take a look.


I have not checked code-diff in v3 yet.

Anyway, I will add comments to v4
if I notice something.


> > Let me ask one more question.
> >
> > I guess this patch is motivated by
> > how difficult to convey kernel headers
> > from vendors to users.
> >
> > In that situation, how will the user find
> > the right compiler to use for building external modules?
> >
> >
> >
> >
> > Greg KH said:
> >
> > We don't ever support the system of loading a module built with anything
> > other than the _exact_ same compiler than the kernel was.
> >
> >
> > For the full context, see this:
> > https://lore.kernel.org/patchwork/patch/836247/#1031547
>
> IMO this issue is not related to this patch but is just an issue with
> building external modules in general.


I do not think it is an issue of the build system, at least.

As far as I understood Greg's comment, it is troublesome
without the assumption that vmlinux and modules are built
by the same compiler.

It is related to this patch since this patch assumes use-cases
where external modules are built in a completely different environment,
where a different compiler is probably installed.



> It is up to the user to use the right
> compiler, etc. I will let Greg comment more on that.







> thanks,
>
>  - Joel
>


-- 
Best Regards
Masahiro Yamada


Re: [PATCH v3 1/2] Provide in-kernel headers for making it easy to extend the kernel

2019-03-01 Thread Joel Fernandes
On Sat, Mar 02, 2019 at 11:13:07AM +0900, Masahiro Yamada wrote:
> On Sat, Mar 2, 2019 at 3:03 AM Joel Fernandes  wrote:
> >
> > On Fri, Mar 01, 2019 at 03:25:05PM +0900, Masahiro Yamada wrote:
> > [...]
> > > > > I am guessing the user will run these commands
> > > > > on the target system.
> > > > > In other words, external modules are native-compiled.
> > > > > So,
> > > > >
> > > > >   target-arch: arm64
> > > > >   host-arch:   arm64
> > > > >
> > > > >
> > > > > Is this correct?
> > > > >
> > > > >
> > > > > If I understood the assumed use-case correctly,
> > > > > kheaders.tar.xw will contain host-programs compiled for x86,
> > > > > which will not work on the target system.
> > > > >
> > > >
> > > > You are right, the above commands in the commit message work only if the
> > > > host/target are same arch due to scripts.
> > > >
> > > > However we can build with arm64 device connected to a host, like this 
> > > > (which
> > > > I tested):
> > > >
> > > > adb shell modprobe kheaders; adb pull /proc/kheaders.tar.xz
> > > > rm -rf $HOME/headers; mkdir -p $HOME/headers
> > > > tar -xvf /proc/kheaders.tar.xz -C $HOME/headers >/dev/null
> > > > cd my-kernel-module
> > > > make -C $HOME/headers M=$(pwd) ARCH=arm64 CROSS_COMPILE=aarch64- modules
> > > > adb push test.ko /data/; adb shell rmmod kheaders
> > > >
> > > > The other way we can make this work is using x86 usermode emulation 
> > > > inside a
> > > > chroot on the Android device which will make the earlier commands work. 
> > > > One
> > > > thing to note is that Android also runs on x86 hardware so the commands 
> > > > in
> > > > the commit message will work even for x86 Android targets already.
> > > >
> > > > Also note that this the "module building" part is really only one of the
> > > > usecases. eBPF is another which needs the headers - and the headers are 
> > > > vast
> > > > majority of the archive. Headers take 3.1MB out of 3.6MB of the archive 
> > > > on
> > > > arm64 builds.
> > > >
> > > > How do you want to proceed here, should I mention these points in the 
> > > > commit
> > > > message?
> > >
> > >
> > >
> > > I do not request a re-spin just for a matter of commit log,
> > > but this version produces an empty tarball.
> > > So, you will have a chance to update the patch anyway.
> > >
> > > In the next version, it would be nice to note that
> > > "external modules must be built on the same host arch
> > > as built vmlinux".
> >
> > Ok, I respun it with 1 more minor nit for arm64 building. Please take a 
> > look.
> 
> 
> I have not checked code-diff in v3 yet.
> 
> Anyway, I will add comments to v4
> if I notice something.

Ok. Since all your comments from previous series were addressed, it would be
nice to get your Acked-by tag for v4 unless you have further comments or
concerns.

> > > Let me ask one more question.
> > >
> > > I guess this patch is motivated by
> > > how difficult to convey kernel headers
> > > from vendors to users.
> > >
> > > In that situation, how will the user find
> > > the right compiler to use for building external modules?
> > >
> > >
> > >
> > >
> > > Greg KH said:
> > >
> > > We don't ever support the system of loading a module built with anything
> > > other than the _exact_ same compiler than the kernel was.
> > >
> > >
> > > For the full context, see this:
> > > https://lore.kernel.org/patchwork/patch/836247/#1031547
> >
> > IMO this issue is not related to this patch but is just an issue with
> > building external modules in general.
> 
> 
> I do not think it is an issue of the build system, at least.
> 
> As far as I understood Greg's comment, it is troublesome
> without the assumption that vmlinux and modules are built
> by the same compiler.
> It is related to this patch since this patch assumes use-cases
> where external modules are built in a completely different environment,
> where a different compiler is probably installed.

Yes, but what I'm trying to say is the same issue exists with all other
solutions today that do this. Such as debian you have linux-headers package.
A user could totally use the build artifacts obtained from somewhere to build
a kernel module with a completely different compiler. That issue has just to
do with the reality, and isn't an issue caused by any one solution such as
this one.  I agree care must be taken whenever user is building external
kernel modules independent of kernel sources.  Did I miss something else?

thanks a lot,

 - Joel