2023-02-21 20:40:26

by Rob Landley

[permalink] [raw]
Subject: [PATCH 0/5] Patches used to build mkroot.

The ~300 line bash script in toybox that builds bootable Linux systems
for a dozen-ish targets can use a vanilla kernel, but the binaries I
ship are built from a kernel with these patches:

https://github.com/landley/toybox/blob/master/scripts/mkroot.sh
https://landley.net/toybox/downloads/binaries/mkroot/0.8.9/

I've posted each patch to the list already, but here they are together
and updated for 6.2:

1) let LLVM build normally without telling Linux on the command line
This way I can mix CROSS_COMPILE=$ARCH-unknown-linux-cross- without
having to care that some are llvm and some are gcc.

2) don't require an extra dependency to build x86-64 no other target needs.

3) Make CONFIG_DEVTMPFS_MOUNT work in initramfs. That way having
the kernel build archive up a directory into a cpio.gz as a normal
user doesn't leave me without a /dev/console and thus init running
with stdin/stderr/stdout closed.

4) Replace the only user of bc with c. (Another package dependency
with only one user.)

5) Fix rootfstype=tmpfs in initramfs. (A thinko I made in 2013 which
nobody else has bothered to fix for a decade now.)


2023-02-21 20:41:53

by Rob Landley

[permalink] [raw]
Subject: [PATCH 1/5] try generic compiler name "cc" before falling back to "gcc".

Distros like debian install the generic "cc" name for both gcc and clang,
and the plumbing already does CC_VERSION_TEXT to include Makefile.clang.

Previously: https://lkml.iu.edu/hypermail/linux/kernel/2202.0/01505.html
Signed-off-by: Rob Landley <[email protected]>
---
Makefile | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index 3f6628780eb2..0ac57ae3b45f 100644
--- a/Makefile
+++ b/Makefile
@@ -456,7 +456,7 @@ endif
HOSTCC = $(LLVM_PREFIX)clang$(LLVM_SUFFIX)
HOSTCXX = $(LLVM_PREFIX)clang++$(LLVM_SUFFIX)
else
-HOSTCC = gcc
+HOSTCC := $(shell cc --version >/dev/null 2>&1 && echo cc || echo gcc)
HOSTCXX = g++
endif
HOSTRUSTC = rustc
@@ -503,7 +503,8 @@ OBJDUMP = $(LLVM_PREFIX)llvm-objdump$(LLVM_SUFFIX)
READELF = $(LLVM_PREFIX)llvm-readelf$(LLVM_SUFFIX)
STRIP = $(LLVM_PREFIX)llvm-strip$(LLVM_SUFFIX)
else
-CC = $(CROSS_COMPILE)gcc
+CC := $(CROSS_COMPILE)$(shell $(CROSS_COMPILE)cc --version \
+ >/dev/null 2>&1 && echo cc || echo gcc)
LD = $(CROSS_COMPILE)ld
AR = $(CROSS_COMPILE)ar
NM = $(CROSS_COMPILE)nm

2023-02-21 20:43:07

by Rob Landley

[permalink] [raw]
Subject: [PATCH 2/5] X86-64 should not uniquely require a third ELF package to build.

x86-64 is the only architecture that can't compile without an extra
ELF library installed on the host. (The kernel already has multiple ELF
parse implementations built-in, so requiring another one is questionable
at best.) You can switch it back on in menuconfig if you want to, this
just stops it being mandatory.

See https://lkml.iu.edu/hypermail/linux/kernel/2110.3/00402.html
and https://lkml.iu.edu/hypermail/linux/kernel/2110.3/00278.html

Signed-off-by: Rob Landley <[email protected]>
---
arch/x86/Kconfig | 1 -
1 file changed, 1 deletion(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 3604074a878b..b63510d79baf 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -243,7 +243,6 @@ config X86
select HAVE_NOINSTR_HACK if HAVE_OBJTOOL
select HAVE_NMI
select HAVE_NOINSTR_VALIDATION if HAVE_OBJTOOL
- select HAVE_OBJTOOL if X86_64
select HAVE_OPTPROBES
select HAVE_PCSPKR_PLATFORM
select HAVE_PERF_EVENTS

2023-02-21 20:44:35

by Rob Landley

[permalink] [raw]
Subject: [PATCH 3/5] Wire up CONFIG_DEVTMPFS_MOUNT to initramfs.

The kernel has had CONFIG_DEVTMPFS_MOUNT for years, but it only applied to
fallback ROOT= not initramfs/initmpfs. As long as the config option exists,
it might as well work.

I use this for board bringup: populating a chdir and calling cpio as a
normal user often leaves /dev empty (because mknod requires root access),
meaning no /dev/console for init/main.c to open, meaning init runs without
stdin/stdout/stderr and has to mount devtmpfs and redirect the filehandles
blind with no error output if something goes wrong.

Signed-off-by: Rob Landley <[email protected]>
Previously: https://lkml.org/lkml/2017/9/13/651
---
init/main.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/init/main.c b/init/main.c
index e1c3911d7c70..eca7ba2c2764 100644
--- a/init/main.c
+++ b/init/main.c
@@ -1636,7 +1636,6 @@ static noinline void __init kernel_init_freeable(void)
kunit_run_all_tests();

wait_for_initramfs();
- console_on_rootfs();

/*
* check if there is an early userspace init. If yes, let it do all
@@ -1645,7 +1644,11 @@ static noinline void __init kernel_init_freeable(void)
if (init_eaccess(ramdisk_execute_command) != 0) {
ramdisk_execute_command = NULL;
prepare_namespace();
+ } else if (IS_ENABLED(CONFIG_DEVTMPFS_MOUNT)) {
+ init_mkdir("/dev", 0755);
+ devtmpfs_mount();
}
+ console_on_rootfs();

/*
* Ok, we have completed the initial bootup, and

2023-02-21 20:47:30

by Rob Landley

[permalink] [raw]
Subject: [PATCH 4/5] Replace timeconst.bc with mktimeconst.c

Update of my decade-old patch replacing timeconst.pl with mktimeconst.c,
still removing the only user of "bc" from the build.

All that's changed since the 2015 version at:
https://github.com/landley/aboriginal/blob/master/sources/patches/linux-noperl-timeconst.patch

Is one extra iteration of the loop for nanoseconds and different
makefile plumbing calling it. In theory this could calculate the values
at runtime in a small _init function and eliminate the header or even
allow HZ to change at runtime.

See https://lkml.iu.edu/hypermail/linux/kernel/2211.0/02589.html

Signed-off-by: Rob Landley <[email protected]>
---
Kbuild | 7 ++-
kernel/time/mktimeconst.c | 111 ++++++++++++++++++++++++++++++++++++
kernel/time/timeconst.bc | 117 --------------------------------------
3 files changed, 116 insertions(+), 119 deletions(-)
create mode 100644 kernel/time/mktimeconst.c
delete mode 100644 kernel/time/timeconst.bc

diff --git a/Kbuild b/Kbuild
index 464b34a08f51..8c12f6ef58c6 100644
--- a/Kbuild
+++ b/Kbuild
@@ -18,9 +18,12 @@ $(bounds-file): kernel/bounds.s FORCE

timeconst-file := include/generated/timeconst.h

-filechk_gentimeconst = echo $(CONFIG_HZ) | bc -q $<
+hostprogs += mktimeconst
+mktimeconst-objs = kernel/time/mktimeconst.o

-$(timeconst-file): kernel/time/timeconst.bc FORCE
+filechk_gentimeconst = $(obj)/mktimeconst $(CONFIG_HZ) -
+
+$(timeconst-file): $(obj)/mktimeconst FORCE
$(call filechk,gentimeconst)

# Generate asm-offsets.h
diff --git a/kernel/time/mktimeconst.c b/kernel/time/mktimeconst.c
new file mode 100644
index 000000000000..c4c0df289472
--- /dev/null
+++ b/kernel/time/mktimeconst.c
@@ -0,0 +1,113 @@
+// SPDX-License-Identifier: 0BSD
+// Copyright 2010-2023 Rob Landley <[email protected]>
+
+#include <inttypes.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+int main(int argc, char *argv[])
+{
+ uint64_t hz, periods[] = {1000, 1000000, 1000000000};
+ char *names[] = {"MSEC", "USEC", "NSEC"};
+ FILE *file;
+ int i, j;
+
+ if (argc != 3 || (hz = atol(argv[1])) < 1
+ || !(file = !strcmp(argv[2], "-") ? stdout : fopen(argv[2], "w"))) {
+ fprintf(stderr, "Usage: mktimeconst HZ FILENAME\n\n");
+ fprintf(stderr, "Generate a header file with constants to convert between\n");
+ fprintf(stderr, "decimal HZ timer ticks and milisecond or microsecond delays,\n");
+ fprintf(stderr, "using reciprocal multiplication to avoid 64 bit division.\n");
+ exit(1);
+ }
+
+ fprintf(file,
+ "/* Conversion constants for HZ == %"PRIu64" */\n\n"
+ "/* Automatically generated by kernel/time/mktimeconst.c */\n"
+ "/* This could be generated in __init code but isn't */\n"
+
+ "#ifndef __KERNEL_TIMECONST_H\n"
+ "#define __KERNEL_TIMECONST_H\n\n"
+ "#include <linux/param.h>\n"
+ "#include <linux/types.h>\n\n"
+ "#if HZ != %"PRIu64"\n"
+ "#error \"include/generated/timeconst.h has the wrong HZ value!\"\n"
+ "#endif\n\n", hz, hz);
+
+ /* Repeat for MSEC, USEC, and NSEC */
+
+ for (i = 0; i < 3; i++) {
+ uint64_t gcd, period;
+
+ /* Find greatest common denominator using Euclid's algorithm. */
+
+ gcd = hz;
+ period = periods[i];
+ while (period) {
+ uint64_t temp = gcd % period;
+
+ gcd = period;
+ period = temp;
+ }
+
+ /* Output both directions (HZ_TO_PERIOD and PERIOD_TO_HZ) */
+
+ for (j = 0; j < 2; j++) {
+ char name[16];
+ uint64_t from = j ? periods[i] : hz;
+ uint64_t to = j ? hz : periods[i];
+ uint64_t mul32 = 0, adj32 = 0, shift = 0;
+
+ sprintf(name, j ? "%s_TO_HZ" : "HZ_TO_%s", names[i]);
+
+ /* Figure out what shift value gives 32 significant
+ * bits of MUL32 data. (Worst case to=1 from=1000000
+ * uses 52 bits, to<<shift won't overflow 64 bit math.)
+ */
+
+ for (;;) {
+ mul32 = ((to << shift) + from - 1) / from;
+ if (mul32 >= (1UL<<31))
+ break;
+ shift++;
+ }
+
+ /* ADJ32 is just (((FROM/GCD)-1)<<SHIFT)/(FROM/GCD)
+ * but this can overflow 64 bit math (examples, HZ=24
+ * or HZ=122). Worst case scenario uses 32+20+20=72
+ * bits. Workaround: split off bottom 32 bits and
+ * reassemble after calculation (32+64=96 bits).
+ */
+
+ adj32 = from / gcd;
+
+ if (shift > 32) {
+ uint64_t upper, lower;
+
+ upper = (adj32 - 1) << (shift - 32);
+ lower = (upper % adj32) << 32;
+ adj32 = ((upper/adj32) << 32) + (lower/adj32);
+ } else
+ adj32 = ((adj32 - 1) << shift) / adj32;
+
+ /* Emit the constants into the header file. */
+
+ fprintf(file, "#define %s_MUL32\tU64_C(0x%"PRIx64")\n",
+ name, mul32);
+ fprintf(file, "#define %s_ADJ32\tU64_C(0x%"PRIx64")\n",
+ name, adj32);
+ fprintf(file, "#define %s_SHR32\t%"PRIu64"\n",
+ name, shift);
+ fprintf(file, "#define %s_NUM\t\tU64_C(%"PRIu64")\n",
+ name, to/gcd);
+ fprintf(file, "#define %s_DEN\t\tU64_C(%"PRIu64")\n\n",
+ name, from/gcd);
+ }
+ }
+ fprintf(file, "#endif /* __KERNEL_TIMECONST_H */\n");
+
+ /* Notice if the disk fills up. */
+
+ fflush(file);
+}
diff --git a/kernel/time/timeconst.bc b/kernel/time/timeconst.bc
deleted file mode 100644
index 7ed0e0fb5831..000000000000
--- a/kernel/time/timeconst.bc
+++ /dev/null
@@ -1,117 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-
-scale=0
-
-define gcd(a,b) {
- auto t;
- while (b) {
- t = b;
- b = a % b;
- a = t;
- }
- return a;
-}
-
-/* Division by reciprocal multiplication. */
-define fmul(b,n,d) {
- return (2^b*n+d-1)/d;
-}
-
-/* Adjustment factor when a ceiling value is used. Use as:
- (imul * n) + (fmulxx * n + fadjxx) >> xx) */
-define fadj(b,n,d) {
- auto v;
- d = d/gcd(n,d);
- v = 2^b*(d-1)/d;
- return v;
-}
-
-/* Compute the appropriate mul/adj values as well as a shift count,
- which brings the mul value into the range 2^b-1 <= x < 2^b. Such
- a shift value will be correct in the signed integer range and off
- by at most one in the upper half of the unsigned range. */
-define fmuls(b,n,d) {
- auto s, m;
- for (s = 0; 1; s++) {
- m = fmul(s,n,d);
- if (m >= 2^(b-1))
- return s;
- }
- return 0;
-}
-
-define timeconst(hz) {
- print "/* Automatically generated by kernel/time/timeconst.bc */\n"
- print "/* Time conversion constants for HZ == ", hz, " */\n"
- print "\n"
-
- print "#ifndef KERNEL_TIMECONST_H\n"
- print "#define KERNEL_TIMECONST_H\n\n"
-
- print "#include <linux/param.h>\n"
- print "#include <linux/types.h>\n\n"
-
- print "#if HZ != ", hz, "\n"
- print "#error \qinclude/generated/timeconst.h has the wrong HZ value!\q\n"
- print "#endif\n\n"
-
- if (hz < 2) {
- print "#error Totally bogus HZ value!\n"
- } else {
- s=fmuls(32,1000,hz)
- obase=16
- print "#define HZ_TO_MSEC_MUL32\tU64_C(0x", fmul(s,1000,hz), ")\n"
- print "#define HZ_TO_MSEC_ADJ32\tU64_C(0x", fadj(s,1000,hz), ")\n"
- obase=10
- print "#define HZ_TO_MSEC_SHR32\t", s, "\n"
-
- s=fmuls(32,hz,1000)
- obase=16
- print "#define MSEC_TO_HZ_MUL32\tU64_C(0x", fmul(s,hz,1000), ")\n"
- print "#define MSEC_TO_HZ_ADJ32\tU64_C(0x", fadj(s,hz,1000), ")\n"
- obase=10
- print "#define MSEC_TO_HZ_SHR32\t", s, "\n"
-
- obase=10
- cd=gcd(hz,1000)
- print "#define HZ_TO_MSEC_NUM\t\t", 1000/cd, "\n"
- print "#define HZ_TO_MSEC_DEN\t\t", hz/cd, "\n"
- print "#define MSEC_TO_HZ_NUM\t\t", hz/cd, "\n"
- print "#define MSEC_TO_HZ_DEN\t\t", 1000/cd, "\n"
- print "\n"
-
- s=fmuls(32,1000000,hz)
- obase=16
- print "#define HZ_TO_USEC_MUL32\tU64_C(0x", fmul(s,1000000,hz), ")\n"
- print "#define HZ_TO_USEC_ADJ32\tU64_C(0x", fadj(s,1000000,hz), ")\n"
- obase=10
- print "#define HZ_TO_USEC_SHR32\t", s, "\n"
-
- s=fmuls(32,hz,1000000)
- obase=16
- print "#define USEC_TO_HZ_MUL32\tU64_C(0x", fmul(s,hz,1000000), ")\n"
- print "#define USEC_TO_HZ_ADJ32\tU64_C(0x", fadj(s,hz,1000000), ")\n"
- obase=10
- print "#define USEC_TO_HZ_SHR32\t", s, "\n"
-
- obase=10
- cd=gcd(hz,1000000)
- print "#define HZ_TO_USEC_NUM\t\t", 1000000/cd, "\n"
- print "#define HZ_TO_USEC_DEN\t\t", hz/cd, "\n"
- print "#define USEC_TO_HZ_NUM\t\t", hz/cd, "\n"
- print "#define USEC_TO_HZ_DEN\t\t", 1000000/cd, "\n"
-
- cd=gcd(hz,1000000000)
- print "#define HZ_TO_NSEC_NUM\t\t", 1000000000/cd, "\n"
- print "#define HZ_TO_NSEC_DEN\t\t", hz/cd, "\n"
- print "#define NSEC_TO_HZ_NUM\t\t", hz/cd, "\n"
- print "#define NSEC_TO_HZ_DEN\t\t", 1000000000/cd, "\n"
- print "\n"
-
- print "#endif /* KERNEL_TIMECONST_H */\n"
- }
- halt
-}
-
-hz = read();
-timeconst(hz)

2023-02-21 20:50:58

by Rob Landley

[permalink] [raw]
Subject: [PATCH 5/5] fix rootfstype=tmpfs

Wire up rootfstype=tmpfs to force rootfs to be tmpfs even when you specify root=

Initramfs automatically uses tmpfs (if available) when you DON'T specify a
root= fallback root to mount over initramfs, but some people can't NOT do
that for some reason (old bootloaders), so let rootfstype=tmpfs override it.

My original code tried to do this 10 years ago but got the test wrong,
and nobody's corrected it since, so here you go...

Signed-off-by: Rob Landley <[email protected]>

See https://lkml.iu.edu/hypermail/linux/kernel/2207.3/06939.html
---
init/do_mounts.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/init/do_mounts.c b/init/do_mounts.c
index 811e94daf0a8..01d80fb828fd 100644
--- a/init/do_mounts.c
+++ b/init/do_mounts.c
@@ -665,7 +665,7 @@ struct file_system_type rootfs_fs_type = {

void __init init_rootfs(void)
{
- if (IS_ENABLED(CONFIG_TMPFS) && !saved_root_name[0] &&
- (!root_fs_names || strstr(root_fs_names, "tmpfs")))
+ if (IS_ENABLED(CONFIG_TMPFS) && (!root_fs_names ? !saved_root_name[0] :
+ !!strstr(root_fs_names, "tmpfs")))
is_tmpfs = true;
}

2023-02-21 23:12:33

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [PATCH 2/5] X86-64 should not uniquely require a third ELF package to build.

Rob!

On Tue, Feb 21 2023 at 14:56, Rob Landley wrote:

The subject line is not compliant to documentation. Please read and
follow Documentation/process/* especially submitting-patches.rst and
maintainer-tip.rst

> x86-64 is the only architecture that can't compile without an extra
> ELF library installed on the host. (The kernel already has multiple ELF
> parse implementations built-in, so requiring another one is questionable
> at best.)

How are ELF parsers in the kernel itself related to a build requirement
for a tool, which is part of the kernel build process?

Next time you ask for removal of perl, python or whatever the kernel
requires to build.

> You can switch it back on in menuconfig if you want to, this
> just stops it being mandatory.

How do you switch on CONFIG_HAVE_OBJTOOL in menuconfig?

config HAVE_OBJTOOL
bool

There is no knob.

> See https://lkml.iu.edu/hypermail/linux/kernel/2110.3/00402.html
> and https://lkml.iu.edu/hypermail/linux/kernel/2110.3/00278.html

Please use https://lore.kernel.org/lkml/ links.

> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index 3604074a878b..b63510d79baf 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -243,7 +243,6 @@ config X86
> select HAVE_NOINSTR_HACK if HAVE_OBJTOOL
> select HAVE_NMI
> select HAVE_NOINSTR_VALIDATION if HAVE_OBJTOOL
> - select HAVE_OBJTOOL if X86_64

This prevents runtime features, optimizations, mitigations and build
time validations rom being selected as you can see from your patch
context.

Just to be clear: objtool is mandatory for x86_64 builds.

git grep 'select HAVE_OBJTOOL' will tell you that your claim about
x86_64 being the only architecture is slightly wrong.

Thanks,

tglx

2023-02-21 23:53:39

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [PATCH 4/5] Replace timeconst.bc with mktimeconst.c

Rob!

On Tue, Feb 21 2023 at 15:00, Rob Landley wrote:

See my previous comment about Documentation. This time you even failed
to CC the maintainer of this code.

> Update of my decade-old patch replacing timeconst.pl with mktimeconst.c,
> still removing the only user of "bc" from the build.

How is 'decade-old patch' relevant changelog information?

> All that's changed since the 2015 version at:
> https://github.com/landley/aboriginal/blob/master/sources/patches/linux-noperl-timeconst.patch

That's neither interesting.

> Is one extra iteration of the loop for nanoseconds and different
> makefile plumbing calling it. In theory this could calculate the values
> at runtime in a small _init function and eliminate the header or even
> allow HZ to change at runtime.

In theory we can also build a compiler into the decompressor which
compiles and bootstraps the kernel itself, right?

> See https://lkml.iu.edu/hypermail/linux/kernel/2211.0/02589.html

I haven't seen a changelog in a while, which provides so much useless
information and lacks any content which justifies and documents the
proposed change.

> Kbuild | 7 ++-
> kernel/time/mktimeconst.c | 111 ++++++++++++++++++++++++++++++++++++
> kernel/time/timeconst.bc | 117 --------------------------------------

Clearly you provided evidence that the output of both tools is identical
and because you decided to reorder the defines it's not even verifyable
with diff.

But I don't even need diff to figure out that the results are not
identical:

# grep -c '#define' orig.h
25

# grep -c '#define' patched.h
31

Which means this adds 6 more unused defines to the already 8 unused
defines of today.

You clearly spent a lot of time to make this palatable to the people who
are responsible for this code.

That said, I'm not completely opposed to get rid of the bc dependency,
but if you want to sell this, then follow the documented process and
present it in a form which is acceptable.

Thanks,

tglx





2023-02-22 18:01:43

by Rob Landley

[permalink] [raw]
Subject: Re: [PATCH 2/5] X86-64 should not uniquely require a third ELF package to build.

On 2/21/23 17:12, Thomas Gleixner wrote:
> Rob!

Thomas!

> On Tue, Feb 21 2023 at 14:56, Rob Landley wrote:
>
> The subject line is not compliant to documentation. Please read and
> follow Documentation/process/* especially submitting-patches.rst and
> maintainer-tip.rst

I've read them multiple times over the years, but I'm not good at remembering
extensive bureaucracy over long periods. Let's see...

Are you complaining it doesn't have "RESEND"? These apply to 6.2, not 6.1, and a
couple things got tweaked. What's the "RESEND" threshold?

Or is this a "subsystem:" prefix thing where I have to guess what subsystem the
top level makefile or mktimeconst fall under? Is "init" a subsystem?

$ grep -i subsystem MAINTAINERS | wc
85 286 2216
$ grep -i subsystem MAINTAINERS | grep -i init
$

Apparently not. I would have thought get_maintainer.pl would emit this sort of
info if it's important, but I guess I'm remembering back when Linux had
hobbyists who worked on things that didn't require justifying the time on a
spreadsheet to their boss in a budget allocation meeting before modifying any
code...

>> x86-64 is the only architecture that can't compile without an extra
>> ELF library installed on the host. (The kernel already has multiple ELF
>> parse implementations built-in, so requiring another one is questionable
>> at best.)
>
> How are ELF parsers in the kernel itself related to a build requirement
> for a tool, which is part of the kernel build process?

The project already has multiple instances of code that traverses ELF data
structures. A definition of "code reuse" that pulls in an additional build
dependency which the project was not already using is just "code use" without a
"re".

My response was "this doesn't need to be done at all, it's just being used for
an optional stack unwinder, and even if you _want_ a stack unwinder there are
multiple other implementations without this dependency".

They just tangled up the kconfig plubmbing so even when nothing uses it, it
still tries to build the tool it won't run. If I'd tried to FIX the tool,
factoring out existing ELF code so it could be built on the host and used at
build time might have made sense. But the thing it's being used to do is not a
thing I need on the systems I'm building.

> Next time you ask for removal of perl, python or whatever the kernel
> requires to build.

Kernel doesn't require python to build, my perl removal series got merged 10
years ago, and other people have sent follow-up perl removal patches since.

>> You can switch it back on in menuconfig if you want to, this
>> just stops it being mandatory.
>
> How do you switch on CONFIG_HAVE_OBJTOOL in menuconfig?
>
> config HAVE_OBJTOOL
> bool
>
> There is no knob.

Sigh, I traced through all this stuff at one point. It's been a few releases
since then (and the symbol got renamed), and it looks like it's developed some
more dependencies off of it in kernel/trace/Kconfig.

Alright, how about this:

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 3604074a878b..70923305d596 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -243,7 +243,8 @@ config X86
select HAVE_NOINSTR_HACK if HAVE_OBJTOOL
select HAVE_NMI
select HAVE_NOINSTR_VALIDATION if HAVE_OBJTOOL
- select HAVE_OBJTOOL if X86_64
+ select HAVE_OBJTOOL if X86_64 && \
+ $(success,echo "#include <gelf.h>" | $(HOSTCC) -xc -o /dev/null -c -)
select HAVE_OPTPROBES
select HAVE_PCSPKR_PLATFORM
select HAVE_PERF_EVENTS

Actually TEST if the build environment has it, and set the symbol appropriately.

Would that with "test for optional build dependency" work? I'm aware people want
to use this plumbing for more thorough spectre/meltdown style mitigations, but
Linux has CONFIG_MULTIUSER which can remove support for non-root users. (I'm off
at the embedded end of things: we're weird.)

>> See https://lkml.iu.edu/hypermail/linux/kernel/2110.3/00402.html
>> and https://lkml.iu.edu/hypermail/linux/kernel/2110.3/00278.html
>
> Please use https://lore.kernel.org/lkml/ links.

Yeah, check_patch.pl complained about that too. Google's getting unreliable
enough it's not always easy to map between them, but I'll put it on the todo
list for the 6.3 repost of the series...

>> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
>> index 3604074a878b..b63510d79baf 100644
>> --- a/arch/x86/Kconfig
>> +++ b/arch/x86/Kconfig
>> @@ -243,7 +243,6 @@ config X86
>> select HAVE_NOINSTR_HACK if HAVE_OBJTOOL
>> select HAVE_NMI
>> select HAVE_NOINSTR_VALIDATION if HAVE_OBJTOOL
>> - select HAVE_OBJTOOL if X86_64
>
> This prevents runtime features, optimizations, mitigations and build
> time validations rom being selected as you can see from your patch
> context.
>
> Just to be clear: objtool is mandatory for x86_64 builds.

Because without this patch it's forced on, yes. Works fine without it in my testing?

The llvm guys built a whole compiler to get away from GPLv3 and
https://sourceware.org/elfutils is GPLv3, which is not _my_ motivation for
trying to avoid it, but I can see it being "of interest". (If the goal is to
push people away from x86-64 faster...)

No other architecture I tested had this requirement. I admit I'm only building
arm, arm64, x86, x86-64, m68k, mips, ppc32, ppc64, s390x, and superh kernels at
the moment. I'm only doing a userspace version of hexagon because they didn't
have qemu-system-hexagon working last I poked Taylor Simpson so I'd have nothing
to test the kernel on. Musl-libc's xtensa support is not just an out of tree
fork but _very_stale_ (the one I have, anyway). Musl doesn't support
alpha/arc/csky/itanic/mips-looongarch/nios2/parisc/sparc yet. Microblaze
binaries segfault in the ELF _start code for some reason (I should track down
why). I'm halfway to a working cortex-m qemu config and need to get back to that
(qemu SHOULD have a decent board, there's like 5 options now). Building or1k is
on the todo list...

> git grep 'select HAVE_OBJTOOL' will tell you that your claim about
> x86_64 being the only architecture is slightly wrong.

There's two hits, and the other is PPC32, which was building fine for me without
this patch last I checked?

Ah, commit e52ec98c5ab1 from 3 months ago. So 6.1 built fine without it but 6.2
also requires... no, hang on, I test built 6.2 with this patch series more than
once, and my patches don't modify the arch/powerpc directory? I still have the
binaries:

$ ./run-qemu.sh
...
Kernel memory protection not selected by kernel config.
Run /init as init process
Type exit when done.
# cat /proc/version
Linux version 6.2.0 (landley@driftwood) (powerpc-linux-musl-cc (GCC) 9.4.0, GNU
ld (GNU Binutils) 2.33.1) #1 Tue Feb 21 14:07:21 CST 2023
# cat /proc/cpuinfo
processor : 0
cpu : 740/750
clock : 266.000000MHz
revision : 3.1 (pvr 0008 0301)
bogomips : 33.20

timebase : 16603616
platform : PowerMac
model : Power Macintosh
machine : Power Macintosh
motherboard : AAPL,PowerMac G3 MacRISC
detected as : 49 (PowerMac G3 (Silk))
pmac flags : 00000000
pmac-generation : OldWorld
Memory : 256 MB
# file /bin/toybox
/bin/toybox: ELF executable, 32-bit MSB ppc, static, stripped


That's 6.2, G3 was 7xx series which is 32 bit... Yup, powerpc 32 bit built and
ran fine without this. I haven't got libelf-dev installed on my laptop, so it
CAN'T have required it,the build would break if it tried...

Which other architecture build breaks for you without libelf-dev installed?

> Thanks,
>
> tglx

Rob

2023-02-22 18:43:02

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [PATCH 2/5] X86-64 should not uniquely require a third ELF package to build.

Rob!

On Wed, Feb 22 2023 at 12:14, Rob Landley wrote:
> On 2/21/23 17:12, Thomas Gleixner wrote:
>> The subject line is not compliant to documentation. Please read and
>> follow Documentation/process/* especially submitting-patches.rst and
>> maintainer-tip.rst
>
> I've read them multiple times over the years, but I'm not good at remembering
> extensive bureaucracy over long periods. Let's see...
>
> Are you complaining it doesn't have "RESEND"? These apply to 6.2, not 6.1, and a
> couple things got tweaked. What's the "RESEND" threshold?
>
> Or is this a "subsystem:" prefix thing where I have to guess what subsystem the
> top level makefile or mktimeconst fall under? Is "init" a subsystem?
>
> $ grep -i subsystem MAINTAINERS | wc
> 85 286 2216
> $ grep -i subsystem MAINTAINERS | grep -i init
> $

What has a patch against arch/x86 to do with init? I gave you two files
which extensively describe what we ask people to do.

https://www.kernel.org/doc/html/latest/process/maintainer-tip.html#patch-subject

Do you need some more explicit links?

These rules have been set in place to make the maintainer work
scalable. If you ignore them, then you are wasting other peoples
time. The logical consequence is that those people will ignore you too.

> Apparently not. I would have thought get_maintainer.pl would emit this sort of
> info if it's important, but I guess I'm remembering back when Linux had
> hobbyists who worked on things that didn't require justifying the time on a
> spreadsheet to their boss in a budget allocation meeting before modifying any
> code...

Your unrelated and non-sensical rants are not getting you anywhere.

>>> x86-64 is the only architecture that can't compile without an extra
>>> ELF library installed on the host. (The kernel already has multiple ELF
>>> parse implementations built-in, so requiring another one is questionable
>>> at best.)
>>
>> How are ELF parsers in the kernel itself related to a build requirement
>> for a tool, which is part of the kernel build process?
>
> The project already has multiple instances of code that traverses ELF data
> structures. A definition of "code reuse" that pulls in an additional build
> dependency which the project was not already using is just "code use" without a
> "re".

Feel free to send patches against objtool to make use of those multiple
instances, which you gracefully omit to name.

> My response was "this doesn't need to be done at all, it's just being used for
> an optional stack unwinder, and even if you _want_ a stack unwinder there are
> multiple other implementations without this dependency".

Care to figure out what objtool really does and why it is required to
enable certain features? You might not care, but most people do for very
good reasons.

>> Just to be clear: objtool is mandatory for x86_64 builds.
>
> Because without this patch it's forced on, yes. Works fine without it
> in my testing?

Works for me does not cut it and you know that.

>> git grep 'select HAVE_OBJTOOL' will tell you that your claim about
>> x86_64 being the only architecture is slightly wrong.
>
> There's two hits, and the other is PPC32, which was building fine for me without
> this patch last I checked?

Select the right options on PPC32 or PPC64 and it requires objtool.

Just accept it. Your patch _is_ broken and it's not my problem to come
up with a solution which causes regressions for people who care and
depend on the features. It's just more work than removing a single line
in a Kconfig file and claiming that uncomprehensible word salad
qualifies as change log.

Please stop this right here and come back when you have something which
is worth to look at. I gave you enough pointers by now and I'm not going
to waste another second of my time on reading your rants.

Thanks,

tglx



2023-02-22 22:45:48

by Rob Landley

[permalink] [raw]
Subject: Re: [PATCH 4/5] Replace timeconst.bc with mktimeconst.c

On 2/21/23 17:53, Thomas Gleixner wrote:
> Rob!

Yup.

> On Tue, Feb 21 2023 at 15:00, Rob Landley wrote:
>
> See my previous comment about Documentation. This time you even failed
> to CC the maintainer of this code.

$ git show
commit c9c3395d5e3dcc6daee66c6908354d47bf98cb0c (HEAD, tag: v6.2)
Author: Linus Torvalds <[email protected]>
Date: Sun Feb 19 14:24:22 2023 -0800

Linux 6.2
...
$ scripts/get_maintainer.pl 0004-*.patch
Masahiro Yamada <[email protected]>
(commit_signer:3/3=100%,authored:3/3=100%,added_lines:61/61=100%,removed_lines:22/22=100%)
Nicolas Schier <[email protected]> (commit_signer:1/3=33%)
[email protected] (open list)

I cc'd who scripts/get_maintainer.pl told me to cc, when run in a 6.2 checkout.

>> Update of my decade-old patch replacing timeconst.pl with mktimeconst.c,
>> still removing the only user of "bc" from the build.
>
> How is 'decade-old patch' relevant changelog information?

I posted these patches on
https://landley.net/toybox/downloads/binaries/mkroot/0.8.9/linux-patches/ and
https://github.com/landley/linux/commits/mkroot-6.1 when I did the toybox
release where the mkroot binary system images were built from this stuff. The
descriptions mostly come from there. None of the people I interact with
regularly bothers to talk to linux-kernel anymore, they think you're too toxic
to ever contact. I'm weird for tolerating you.

Posting it here involved retesting all the targets against 6.2 and digging up a
thunderbird plugin to get it to stop wrapping lines (old one bit-rotted and is
no longer maintained) and silencing various check_patch.pl complaints about
curly bracket placement and so on. But not a lot of rewriting the descriptions.
It wouldn't help.

This particular patch is coming up on 15 years of ancestry
(https://lore.kernel.org/all/[email protected]/) so I really
don't expect linux-kernel to merge... anything anymore?

I'd have thought the "$ARCH-cross-cc toolchains just work whether they're gcc or
llvm without needing to tell make LLVM=1 but not GCC=1" patch was a no-brainer,
or the one that fixes an obvious bug in my own code from 10 years ago which
multiple people have poked me about and I finally bothered to do something
about, or the one where "hey, you have a CONFIG_DEVTMPFS_MOUNT and it doesn't
work for initramfs, that seems like a bug".

But that just shows I'm out of step with this community. I remember when posting
a simple patch would give someone the idea and they'd write their own as often
as merge mine because it was just easy to do. Hasn't been the case here in quite
a while.

>> All that's changed since the 2015 version at:
>> https://github.com/landley/aboriginal/blob/master/sources/patches/linux-noperl-timeconst.patch
>
> That's neither interesting.

"This area of the existing linux code is not regularly updated, replacing it
does not imply a large future maintenance burden."

>> Is one extra iteration of the loop for nanoseconds and different
>> makefile plumbing calling it. In theory this could calculate the values
>> at runtime in a small _init function and eliminate the header or even
>> allow HZ to change at runtime.
>
> In theory we can also build a compiler into the decompressor which
> compiles and bootstraps the kernel itself, right?

Fabrice Bellard did that in 2004:

https://bellard.org/tcc/tccboot.html

I maintained a fork of tinycc for ~3 years in hopes of extending it to build an
unmodified vanilla kernel, but was stretched too thin between too many projects:

https://landley.net/hg/tinycc

My proposed sequel project was gluing qemu's tcg (tiny code generator) to tcc's
front-end to get a maybe 150k line C compiler that could target every hardware
platform qemu supports:

https://landley.net/qcc/

But again, stretched too thin between too many other projects, didn't have the
spoons.

By the way, part of my motivation for all that was "countering trusting trust":

http://lists.landley.net/pipermail/toybox-landley.net/2020-July/011898.html

And part is minix and xv6 are restricted systems which explicitly reject
anything that would make them load bearing for real world usage (the patch
backlog on comp.os.minix from people who TRIED and couldn't get their patches
merged is how Linux went from 0.0.1 to 0.9.5 so fast).

It would be nice if people could study the base for real world usage in an
isolated reproducible context with minimal circular dependencies. (If you can't
reproduce it from first principles in a laboratory environment, what you're
doing isn't science.)

There was a time where Linux straddled that divide, but these days it seems more
likely that https://www.youtube.com/watch?v=Ce1pMlZO_mI or similar will "grow
legs" and become load bearing than that linux-kernel will get taught in high
schools.

>> See https://lkml.iu.edu/hypermail/linux/kernel/2211.0/02589.html
>
> I haven't seen a changelog in a while, which provides so much useless
> information and lacks any content which justifies and documents the
> proposed change.

You should get out more.

You guys didn't consider "reduce gratuitous build dependencies" to be inherently
useful back when it was perl removal either. That's why I maintain my own
patches. I expect I'll probably switch to a simpler kernel when you finally make
it so nobody can build the linux without C _and_ rust compilers, which doesn't
seem far off. But we'll see...

>> Kbuild | 7 ++-
>> kernel/time/mktimeconst.c | 111 ++++++++++++++++++++++++++++++++++++
>> kernel/time/timeconst.bc | 117 --------------------------------------
>
> Clearly you provided evidence that the output of both tools is identical

I checked that way back when, but producing exactly identical output didn't get
it merged.

This time I think I just read through the changelog on the bc tool and made the
minimal changes necessary to produce identical kernel binaries on all the
https://landley.net/toybox/downloads/binaries/mkroot/0.8.9/ targets but it was a
while ago now...

> and because you decided to reorder the defines it's not even verifyable
> with diff.
>
> But I don't even need diff to figure out that the results are not
> identical:
>
> # grep -c '#define' orig.h
> 25
>
> # grep -c '#define' patched.h
> 31
>
> Which means this adds 6 more unused defines to the already 8 unused
> defines of today.

It produces consistent output for each of the units. I wasn't trying to
determine why the existing code was producing unused defines, or why it
reordered the defines since I wrote the first version. That's social context I
don't have, I'm not a full-time member of the kernel clique.

> You clearly spent a lot of time to make this palatable to the people who
> are responsible for this code.

After 14 years? Not so much. Various people pick things up from linux-kernel...
and then email me privately. Sigh.

> That said, I'm not completely opposed to get rid of the bc dependency,
> but if you want to sell this, then follow the documented process and
> present it in a form which is acceptable.

I've been maintaining this particular patch on and off long enough it'll be
eligible to vote soon. Posting it to linux-kernel means other people with their
own dirty trees can pick it up, and where lawyers and/or a judge who want to
give me grief can be easily shown "I delivered it to the kernel community in the
community's official channel, not just once but multiple times, collated and run
through their get_maintainer.pl and check_patch.pl, and _they_ chose not to
merge it for their own reasons"...

Getting it merged, so I could stop, would be nice. So would winning the lottery.
I'm happy to make changes if you think they'll accomplish anything? But I've
done that before. Here's v4 of the DEVTMPFS_MOUNT patch, for example:

https://lkml.iu.edu/hypermail/linux/kernel/2005.1/09399.html

Rinse repeat, same old same old...

> Thanks,
>
> tglx

Rob

2023-02-23 05:32:32

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [PATCH 1/5] try generic compiler name "cc" before falling back to "gcc".

On Wed, Feb 22, 2023 at 5:41 AM Rob Landley <[email protected]> wrote:
>
> Distros like debian install the generic "cc" name for both gcc and clang,
> and the plumbing already does CC_VERSION_TEXT to include Makefile.clang.
>
> Previously: https://lkml.iu.edu/hypermail/linux/kernel/2202.0/01505.html
> Signed-off-by: Rob Landley <[email protected]>
> ---
> Makefile | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 3f6628780eb2..0ac57ae3b45f 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -456,7 +456,7 @@ endif
> HOSTCC = $(LLVM_PREFIX)clang$(LLVM_SUFFIX)
> HOSTCXX = $(LLVM_PREFIX)clang++$(LLVM_SUFFIX)
> else
> -HOSTCC = gcc
> +HOSTCC := $(shell cc --version >/dev/null 2>&1 && echo cc || echo gcc)



'cc' only makes sense for the host compiler,
which is configured by 'update-alternative'.

I tried it before, but LLVM folks preferred
using $(LLVM) to choose clang/gcc.






> HOSTCXX = g++
> endif
> HOSTRUSTC = rustc
> @@ -503,7 +503,8 @@ OBJDUMP = $(LLVM_PREFIX)llvm-objdump$(LLVM_SUFFIX)
> READELF = $(LLVM_PREFIX)llvm-readelf$(LLVM_SUFFIX)
> STRIP = $(LLVM_PREFIX)llvm-strip$(LLVM_SUFFIX)
> else
> -CC = $(CROSS_COMPILE)gcc
> +CC := $(CROSS_COMPILE)$(shell $(CROSS_COMPILE)cc --version \
> + >/dev/null 2>&1 && echo cc || echo gcc)

This hunk sets up GCC/binutils.
So, cc does not make sense. NACK.






> LD = $(CROSS_COMPILE)ld
> AR = $(CROSS_COMPILE)ar
> NM = $(CROSS_COMPILE)nm



--
Best Regards
Masahiro Yamada

2023-02-23 13:37:42

by Rob Landley

[permalink] [raw]
Subject: Re: [PATCH 1/5] try generic compiler name "cc" before falling back to "gcc".

On 2/22/23 23:31, Masahiro Yamada wrote:
> On Wed, Feb 22, 2023 at 5:41 AM Rob Landley <[email protected]> wrote:
>>
>> Distros like debian install the generic "cc" name for both gcc and clang,
>> and the plumbing already does CC_VERSION_TEXT to include Makefile.clang.
>>
>> Previously: https://lkml.iu.edu/hypermail/linux/kernel/2202.0/01505.html
>> Signed-off-by: Rob Landley <[email protected]>
>> ---
>> Makefile | 5 +++--
>> 1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/Makefile b/Makefile
>> index 3f6628780eb2..0ac57ae3b45f 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -456,7 +456,7 @@ endif
>> HOSTCC = $(LLVM_PREFIX)clang$(LLVM_SUFFIX)
>> HOSTCXX = $(LLVM_PREFIX)clang++$(LLVM_SUFFIX)
>> else
>> -HOSTCC = gcc
>> +HOSTCC := $(shell cc --version >/dev/null 2>&1 && echo cc || echo gcc)
>
> 'cc' only makes sense for the host compiler,

It was the generic posix name for "C compiler" until posix decided to put an
expiration date in the command name, which seems as widely honored as the
tar->pax switch or the removal of cpio.

The name "gcc" was like "gmake" and "gawk". (On macos homebrew there's "gsed".)

> which is configured by 'update-alternative'.

Hexagon only has llvm support, not gcc, so I had to add an llvm cross compiler
to my cross compiler set, prefixed hexagon-unknown-linux-musl-*, but the linux
kernel wants to call it as hexagon-unknown-linux-musl-gcc.

The kernel builds find with a "gcc" symlink to clang, but there _is_ a generic
name, which is widely installed.

> I tried it before, but LLVM folks preferred
> using $(LLVM) to choose clang/gcc.

So if we want generic behavior without having to specify, we should create a
"gcc" symlink to clang?

>> HOSTCXX = g++
>> endif
>> HOSTRUSTC = rustc
>> @@ -503,7 +503,8 @@ OBJDUMP = $(LLVM_PREFIX)llvm-objdump$(LLVM_SUFFIX)
>> READELF = $(LLVM_PREFIX)llvm-readelf$(LLVM_SUFFIX)
>> STRIP = $(LLVM_PREFIX)llvm-strip$(LLVM_SUFFIX)
>> else
>> -CC = $(CROSS_COMPILE)gcc
>> +CC := $(CROSS_COMPILE)$(shell $(CROSS_COMPILE)cc --version \
>> + >/dev/null 2>&1 && echo cc || echo gcc)
>
> This hunk sets up GCC/binutils.

This is the codepath that's taken when you don't explicitly specify "LLVM=1".
The test there is falling back to (appropriately prefixed) gcc when it can't
find a working (appropriately prefixed) cc.

> So, cc does not make sense. NACK.

Do you explicitly specify the "gold" linker vs the previous gnu one vs
lld.llvm.org on the kernel build command line?

Rob

2023-02-23 20:35:54

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH 0/5] Patches used to build mkroot.

On Tue, 21 Feb 2023 14:53:30 -0600 Rob Landley <[email protected]> wrote:

> The ~300 line bash script in toybox that builds bootable Linux systems
> for a dozen-ish targets can use a vanilla kernel, but the binaries I
> ship are built from a kernel with these patches:

It's nice to see a diffstat of the whole series to at least get an idea
of who might merge the series.

Who were you thinking might merge the series? I was only cc'ed on two
of them, I think.

There is no "Previously:" changelog tag. Please use plain old "Link:".

Also, "See https://..." after the "^---$" will get chopped off. It
might be helpful to includes this also as a Link: tag.


2023-02-24 00:27:57

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [PATCH 1/5] try generic compiler name "cc" before falling back to "gcc".

On Thu, Feb 23, 2023 at 10:36 PM Rob Landley <[email protected]> wrote:
>
> On 2/22/23 23:31, Masahiro Yamada wrote:
> > On Wed, Feb 22, 2023 at 5:41 AM Rob Landley <[email protected]> wrote:
> >>
> >> Distros like debian install the generic "cc" name for both gcc and clang,
> >> and the plumbing already does CC_VERSION_TEXT to include Makefile.clang.
> >>
> >> Previously: https://lkml.iu.edu/hypermail/linux/kernel/2202.0/01505.html
> >> Signed-off-by: Rob Landley <[email protected]>
> >> ---
> >> Makefile | 5 +++--
> >> 1 file changed, 3 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/Makefile b/Makefile
> >> index 3f6628780eb2..0ac57ae3b45f 100644
> >> --- a/Makefile
> >> +++ b/Makefile
> >> @@ -456,7 +456,7 @@ endif
> >> HOSTCC = $(LLVM_PREFIX)clang$(LLVM_SUFFIX)
> >> HOSTCXX = $(LLVM_PREFIX)clang++$(LLVM_SUFFIX)
> >> else
> >> -HOSTCC = gcc
> >> +HOSTCC := $(shell cc --version >/dev/null 2>&1 && echo cc || echo gcc)
> >
> > 'cc' only makes sense for the host compiler,
>
> It was the generic posix name for "C compiler" until posix decided to put an
> expiration date in the command name, which seems as widely honored as the
> tar->pax switch or the removal of cpio.
>
> The name "gcc" was like "gmake" and "gawk". (On macos homebrew there's "gsed".)
>
> > which is configured by 'update-alternative'.
>
> Hexagon only has llvm support, not gcc, so I had to add an llvm cross compiler
> to my cross compiler set, prefixed hexagon-unknown-linux-musl-*, but the linux
> kernel wants to call it as hexagon-unknown-linux-musl-gcc.
>
> The kernel builds find with a "gcc" symlink to clang, but there _is_ a generic
> name, which is widely installed.



In the discussion in the past [1], we decided to
go with LLVM=1 switch rather than 'cc'.
We do not need both.

[1]: https://lkml.org/lkml/2020/3/28/494








>
> > I tried it before, but LLVM folks preferred
> > using $(LLVM) to choose clang/gcc.
>
> So if we want generic behavior without having to specify, we should create a
> "gcc" symlink to clang?


If you mean "without LLVM=1 or HOSTCC=clang specified",
I think the answer is yes.






>
> >> HOSTCXX = g++
> >> endif
> >> HOSTRUSTC = rustc
> >> @@ -503,7 +503,8 @@ OBJDUMP = $(LLVM_PREFIX)llvm-objdump$(LLVM_SUFFIX)
> >> READELF = $(LLVM_PREFIX)llvm-readelf$(LLVM_SUFFIX)
> >> STRIP = $(LLVM_PREFIX)llvm-strip$(LLVM_SUFFIX)
> >> else
> >> -CC = $(CROSS_COMPILE)gcc
> >> +CC := $(CROSS_COMPILE)$(shell $(CROSS_COMPILE)cc --version \
> >> + >/dev/null 2>&1 && echo cc || echo gcc)
> >
> > This hunk sets up GCC/binutils.
>
> This is the codepath that's taken when you don't explicitly specify "LLVM=1".
> The test there is falling back to (appropriately prefixed) gcc when it can't
> find a working (appropriately prefixed) cc.


Unless LLVM=1 is specified, the toolchain defaults to GCC/binutils,
there is no need for such additional complexity.




>
> > So, cc does not make sense. NACK.
>
> Do you explicitly specify the "gold" linker vs the previous gnu one vs
> lld.llvm.org on the kernel build command line?


gold is unsupported.
See 75959d44f9dc8e44410667009724e4e238515502


Yes, to override only LD, LD=ld.lld should be given in the command line.





>
> Rob



--
Best Regards
Masahiro Yamada

2023-11-01 13:12:06

by Stefan Berger

[permalink] [raw]
Subject: Re: [PATCH 5/5] fix rootfstype=tmpfs



On 2/21/23 16:04, Rob Landley wrote:
> Wire up rootfstype=tmpfs to force rootfs to be tmpfs even when you specify root=
>
> Initramfs automatically uses tmpfs (if available) when you DON'T specify a
> root= fallback root to mount over initramfs, but some people can't NOT do
> that for some reason (old bootloaders), so let rootfstype=tmpfs override it.
>
> My original code tried to do this 10 years ago but got the test wrong,
> and nobody's corrected it since, so here you go...
>
> Signed-off-by: Rob Landley <[email protected]>

I would like to be able to have this for some work with OpenBMC and
ideally it would propagate to one of the recent kernels with a Fixes tag
like this?

Fixes: 6e19eded3684 ("initmpfs: use initramfs if rootfstype= or root=
specified")

Reviewed-by: Stefan Berger <[email protected]>
Tested-by: Stefan Berger <[email protected]>

>
> See https://lkml.iu.edu/hypermail/linux/kernel/2207.3/06939.html
> ---
> init/do_mounts.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/init/do_mounts.c b/init/do_mounts.c
> index 811e94daf0a8..01d80fb828fd 100644
> --- a/init/do_mounts.c
> +++ b/init/do_mounts.c
> @@ -665,7 +665,7 @@ struct file_system_type rootfs_fs_type = {
>
> void __init init_rootfs(void)
> {
> - if (IS_ENABLED(CONFIG_TMPFS) && !saved_root_name[0] &&
> - (!root_fs_names || strstr(root_fs_names, "tmpfs")))
> + if (IS_ENABLED(CONFIG_TMPFS) && (!root_fs_names ? !saved_root_name[0] :
> + !!strstr(root_fs_names, "tmpfs")))
> is_tmpfs = true;
> }