Re: [PATCH 13/14] kdbus: add walk-through user space example

Re: [PATCH 13/14] kdbus: add walk-through user space example

[Jiri Slaby]

Ccing kbuild fellows (see the very bottom).

On 03/24/2015, 06:15 PM, David Herrmann wrote:
> Hi
> 
> On Tue, Mar 24, 2015 at 5:46 PM, Jiri Slaby <jslaby@suse.cz> wrote:
>> On 03/09/2015, 02:09 PM, Greg Kroah-Hartman wrote:
>>> --- /dev/null
>>> +++ b/samples/kdbus/Makefile
>>> @@ -0,0 +1,10 @@
>>> +# kbuild trick to avoid linker error. Can be omitted if a module is built.
>>> +obj- := dummy.o
>>> +
>>> +hostprogs-y += kdbus-workers
>>> +
>>> +always := $(hostprogs-y)
>>
>> Errr, no. Not only it causes build failures (even with KDBUS=n), it
>> definitely should not be built for everyone.
> 
> It's only built if CONFIG_SAMPLES is set, right?

Yes, but I build other samples selected with separate CONFIG_ options
like CONFIG_SAMPLE_LIVEPATCH. So this guy should have its own CONFIG_
option too, because I (and likely others) don't want to build it. All of
the samples should be protected at least by their respective kernel
CONFIG_ option IMO.

> What build-failures does it cause? linux/kdbus.h is not optional based
> on CONFIG_KDBUS, so the samples should build just fine. Can you tell
> me what kind of errors you get? The kbuild-robots didn't report
> anything so far.

The output is this:
In file included from samples/kdbus/kdbus-workers.c:79:0:
/home/latest/linux/samples/kdbus/kdbus-api.h:5:25: fatal error:
linux/kdbus.h: No such file or directory
 #include <linux/kdbus.h>
                         ^
compilation terminated.

I now know that I have to install_headers *if* I want to build the
sample. But I don't want to build it in the first place. (So the config
option above should be all we need.)

>> And why is it a host prog? It's a sample prog for the kernel I am
>> building, i.e. for the destination arch, like all the other samples.
> 
> It's modeled after the other user-space examples in ./samples/, which
> all use hostprogs (see samples/{bpf,hidraw,seccomp,uhid}/Makefile). I
> have no idea how to build programs that run on the target
> architecture. Documentation/kbuild/makefiles.txt doesn't list it,
> which is, I guess, the reason why everyone used hostprogs so far. And
> given that autotools calls the target architecture "--host", I
> actually thought this is what hostprogs does.. apparently that's not
> the case, sorry.

Oh, it's cut&paste, I see. This does not look correct though. The hack
inclusive. Host progs are intended to be run on the host where the
kernel is built. During the compilation or such (like x/menuconfig).
Quite misleading naming if you are used to the autotools one.

thanks,
-- 
js
suse labs

Re: [PATCH 13/14] kdbus: add walk-through user space example

[Michal Marek]

Dne 24.3.2015 v 18:37 Jiri Slaby napsal(a):
> On 03/24/2015, 06:15 PM, David Herrmann wrote:
>> On Tue, Mar 24, 2015 at 5:46 PM, Jiri Slaby <jslaby@suse.cz> wrote:
>>> Errr, no. Not only it causes build failures (even with KDBUS=n), it
>>> definitely should not be built for everyone.
>>
>> It's only built if CONFIG_SAMPLES is set, right?
> 
> Yes, but I build other samples selected with separate CONFIG_ options
> like CONFIG_SAMPLE_LIVEPATCH. So this guy should have its own CONFIG_
> option too, because I (and likely others) don't want to build it. All of

Definitely.


>> It's modeled after the other user-space examples in ./samples/, which
>> all use hostprogs (see samples/{bpf,hidraw,seccomp,uhid}/Makefile). I
>> have no idea how to build programs that run on the target
>> architecture. Documentation/kbuild/makefiles.txt doesn't list it,
>> which is, I guess, the reason why everyone used hostprogs so far. And
>> given that autotools calls the target architecture "--host", I
>> actually thought this is what hostprogs does.. apparently that's not
>> the case, sorry.
> 
> Oh, it's cut&paste, I see. This does not look correct though. The hack
> inclusive. Host progs are intended to be run on the host where the
> kernel is built. During the compilation or such (like x/menuconfig).
> Quite misleading naming if you are used to the autotools one.

when cross compiling, we are a bit between a rock and a hard place with
the sample userspace programs:
- The target toolchain might not have libc support
- The host toolchain might be lacking recent kernel headers (therefore
  the need to do make headers_install)
- It's not clean whether the samples are meant to be ran on the build
  host or target.

There has been some work by Sam Ravnborg to introduce uapiprogs-y to for
sample userspace programs, but for now, please use hostprogs-y,
-Iusr/include and make each sample opt-in.

Michal

Re: [PATCH 13/14] kdbus: add walk-through user space example

[Daniel Mack]

[-- Attachment #1: Type: text/plain, Size: 1242 bytes --]

On 03/24/2015 07:22 PM, Michal Marek wrote:
> Dne 24.3.2015 v 18:37 Jiri Slaby napsal(a):

>> Oh, it's cut&paste, I see. This does not look correct though. The hack
>> inclusive. Host progs are intended to be run on the host where the
>> kernel is built. During the compilation or such (like x/menuconfig).
>> Quite misleading naming if you are used to the autotools one.
> 
> when cross compiling, we are a bit between a rock and a hard place with
> the sample userspace programs:
> - The target toolchain might not have libc support
> - The host toolchain might be lacking recent kernel headers (therefore
>   the need to do make headers_install)
> - It's not clean whether the samples are meant to be ran on the build
>   host or target.

Exactly. I just checked in a cross-compiled source tree, and none of the
compiled standalone executables from samples/ or Documentation/ are
actually built for the target platform. Only the samples which come as
kernel modules are.

> There has been some work by Sam Ravnborg to introduce uapiprogs-y to for
> sample userspace programs, but for now, please use hostprogs-y,
> -Iusr/include and make each sample opt-in.

Alright then. Does the attached patch fix your problem, Jiri?


Thanks,
Daniel


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-kdbus-sample-build-kdbus-workers-conditionally.patch --]
[-- Type: text/x-diff; name="0001-kdbus-sample-build-kdbus-workers-conditionally.patch", Size: 1468 bytes --]

From 56309ab0717720fb086ec3209f1b1fa719072f61 Mon Sep 17 00:00:00 2001
From: Daniel Mack <daniel@zonque.org>
Date: Tue, 24 Mar 2015 19:41:56 +0100
Subject: [PATCH] kdbus: sample: build kdbus-workers conditionally

Give the kdbus sample its own config switch and only build it if it's
explicitly switched on.

Reported-by: Jiri Slaby <jslaby@suse.cz>
Signed-off-by: Daniel Mack <daniel@zonque.org>
---
 samples/Kconfig        | 7 +++++++
 samples/kdbus/Makefile | 2 +-
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/samples/Kconfig b/samples/Kconfig
index 224ebb4..a4c6b2f 100644
--- a/samples/Kconfig
+++ b/samples/Kconfig
@@ -55,6 +55,13 @@ config SAMPLE_KDB
 	  Build an example of how to dynamically add the hello
 	  command to the kdb shell.
 
+config SAMPLE_KDBUS
+	bool "Build kdbus API example"
+	depends on KDBUS
+	help
+	  Build an example of how the kdbus API can be used from
+	  userspace.
+
 config SAMPLE_RPMSG_CLIENT
 	tristate "Build rpmsg client sample -- loadable modules only"
 	depends on RPMSG && m
diff --git a/samples/kdbus/Makefile b/samples/kdbus/Makefile
index d009025..9e40c68 100644
--- a/samples/kdbus/Makefile
+++ b/samples/kdbus/Makefile
@@ -1,7 +1,7 @@
 # kbuild trick to avoid linker error. Can be omitted if a module is built.
 obj- := dummy.o
 
-hostprogs-y += kdbus-workers
+hostprogs-$(CONFIG_SAMPLE_KDBUS) += kdbus-workers
 
 always := $(hostprogs-y)
 
-- 
2.3.3