Hi, Willy
Here is last 3 patches for v6.6 from me.
It includes two generic patches from the tinyconfig part1 series and one
static related patch derived from Thomas' series.
Best regards,
Zhangjin
Zhangjin Wu (3):
selftests/nolibc: allow report with existing test log
selftests/nolibc: fix up O= option support
tools/nolibc: stackprotector.h: make __stack_chk_init static
tools/include/nolibc/crt.h | 2 +-
tools/include/nolibc/stackprotector.h | 5 ++---
tools/testing/selftests/nolibc/Makefile | 11 +++++++++--
3 files changed, 12 insertions(+), 6 deletions(-)
--
2.25.1
To avoid pollute the source code tree and avoid mrproper for every
architecture switch, the O= argument must be supported.
Both IMAGE and .config are from the building directory, let's use
objtree instead of srctree for them.
If no O= option specified, means building kernel in source code tree,
objtree should be srctree in such case.
Suggested-by: Willy Tarreau <[email protected]>
Link: https://lore.kernel.org/lkml/[email protected]/
Signed-off-by: Zhangjin Wu <[email protected]>
---
tools/testing/selftests/nolibc/Makefile | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/nolibc/Makefile b/tools/testing/selftests/nolibc/Makefile
index 51fef5e6a152..af590aee063a 100644
--- a/tools/testing/selftests/nolibc/Makefile
+++ b/tools/testing/selftests/nolibc/Makefile
@@ -9,6 +9,9 @@ ifeq ($(srctree),)
srctree := $(patsubst %/tools/testing/selftests/,%,$(dir $(CURDIR)))
endif
+# add objtree for O= argument, required by IMAGE and .config
+objtree ?= $(srctree)
+
ifeq ($(ARCH),)
include $(srctree)/scripts/subarch.include
ARCH = $(SUBARCH)
@@ -217,12 +220,12 @@ kernel: initramfs
# run the tests after building the kernel
run: kernel
- $(Q)qemu-system-$(QEMU_ARCH) -display none -no-reboot -kernel "$(srctree)/$(IMAGE)" -serial stdio $(QEMU_ARGS) > "$(CURDIR)/run.out"
+ $(Q)qemu-system-$(QEMU_ARCH) -display none -no-reboot -kernel "$(objtree)/$(IMAGE)" -serial stdio $(QEMU_ARGS) > "$(CURDIR)/run.out"
$(Q)$(REPORT) $(CURDIR)/run.out
# re-run the tests from an existing kernel
rerun:
- $(Q)qemu-system-$(QEMU_ARCH) -display none -no-reboot -kernel "$(srctree)/$(IMAGE)" -serial stdio $(QEMU_ARGS) > "$(CURDIR)/run.out"
+ $(Q)qemu-system-$(QEMU_ARCH) -display none -no-reboot -kernel "$(objtree)/$(IMAGE)" -serial stdio $(QEMU_ARGS) > "$(CURDIR)/run.out"
$(Q)$(REPORT) $(CURDIR)/run.out
# report with existing test log
--
2.25.1
On 2023-08-03 22:45:52+0800, Zhangjin Wu wrote:
> To avoid pollute the source code tree and avoid mrproper for every
> architecture switch, the O= argument must be supported.
>
> Both IMAGE and .config are from the building directory, let's use
> objtree instead of srctree for them.
>
> If no O= option specified, means building kernel in source code tree,
> objtree should be srctree in such case.
>
> Suggested-by: Willy Tarreau <[email protected]>
> Link: https://lore.kernel.org/lkml/[email protected]/
> Signed-off-by: Zhangjin Wu <[email protected]>
> ---
> tools/testing/selftests/nolibc/Makefile | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/tools/testing/selftests/nolibc/Makefile b/tools/testing/selftests/nolibc/Makefile
> index 51fef5e6a152..af590aee063a 100644
> --- a/tools/testing/selftests/nolibc/Makefile
> +++ b/tools/testing/selftests/nolibc/Makefile
> @@ -9,6 +9,9 @@ ifeq ($(srctree),)
> srctree := $(patsubst %/tools/testing/selftests/,%,$(dir $(CURDIR)))
> endif
>
> +# add objtree for O= argument, required by IMAGE and .config
> +objtree ?= $(srctree)
Isn't this already set by the included tools/scripts/Makefile.include?
Also I'm not entirely if O= works as intended currently.
When using O=foo in the $LINUX/tools/testing/selftests/nolibc directory
the build instead is happening in $LINUX/foo. But the Makefile first
validates that $LINUX/tools/testing/selftests/nolibc/foo exists.
It seems we need to pass $(COMMAND_O) to the recursive calls to $(MAKE),
too?
> +
> ifeq ($(ARCH),)
> include $(srctree)/scripts/subarch.include
> ARCH = $(SUBARCH)
> @@ -217,12 +220,12 @@ kernel: initramfs
>
> # run the tests after building the kernel
> run: kernel
> - $(Q)qemu-system-$(QEMU_ARCH) -display none -no-reboot -kernel "$(srctree)/$(IMAGE)" -serial stdio $(QEMU_ARGS) > "$(CURDIR)/run.out"
> + $(Q)qemu-system-$(QEMU_ARCH) -display none -no-reboot -kernel "$(objtree)/$(IMAGE)" -serial stdio $(QEMU_ARGS) > "$(CURDIR)/run.out"
> $(Q)$(REPORT) $(CURDIR)/run.out
>
> # re-run the tests from an existing kernel
> rerun:
> - $(Q)qemu-system-$(QEMU_ARCH) -display none -no-reboot -kernel "$(srctree)/$(IMAGE)" -serial stdio $(QEMU_ARGS) > "$(CURDIR)/run.out"
> + $(Q)qemu-system-$(QEMU_ARCH) -display none -no-reboot -kernel "$(objtree)/$(IMAGE)" -serial stdio $(QEMU_ARGS) > "$(CURDIR)/run.out"
> $(Q)$(REPORT) $(CURDIR)/run.out
>
> # report with existing test log
> --
> 2.25.1
>
Hi, Thomas
> On 2023-08-03 22:45:52+0800, Zhangjin Wu wrote:
> > To avoid pollute the source code tree and avoid mrproper for every
> > architecture switch, the O= argument must be supported.
> >
> > Both IMAGE and .config are from the building directory, let's use
> > objtree instead of srctree for them.
> >
> > If no O= option specified, means building kernel in source code tree,
> > objtree should be srctree in such case.
> >
> > Suggested-by: Willy Tarreau <[email protected]>
> > Link: https://lore.kernel.org/lkml/[email protected]/
> > Signed-off-by: Zhangjin Wu <[email protected]>
> > ---
> > tools/testing/selftests/nolibc/Makefile | 7 +++++--
> > 1 file changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/tools/testing/selftests/nolibc/Makefile b/tools/testing/selftests/nolibc/Makefile
> > index 51fef5e6a152..af590aee063a 100644
> > --- a/tools/testing/selftests/nolibc/Makefile
> > +++ b/tools/testing/selftests/nolibc/Makefile
> > @@ -9,6 +9,9 @@ ifeq ($(srctree),)
> > srctree := $(patsubst %/tools/testing/selftests/,%,$(dir $(CURDIR)))
> > endif
> >
> > +# add objtree for O= argument, required by IMAGE and .config
> > +objtree ?= $(srctree)
>
> Isn't this already set by the included tools/scripts/Makefile.include?
>
Good question, but it is empty if no O= specified, checked it several
times before ;-)
> Also I'm not entirely if O= works as intended currently.
> When using O=foo in the $LINUX/tools/testing/selftests/nolibc directory
It does work, I have used it to test all of the run targets of the
tinyconfig patches like this:
$ make run O=kernel-$arch RUN_OUT=run.$arch.out ...
Everything about the kernel will be built in $(O).
Just rechecked the O variable in top-level Makefile, selftests/nolibc Makefile
and tools/nolibc Makefile, all of them get the right O value from command line.
From my Makefile experience, an option from command line will be passed
to every sub Makefile via $(-*-command-variables-*-), it has the highest
priority then the others, except when we use 'override' keyword
internally.
> the build instead is happening in $LINUX/foo. But the Makefile first
> validates that $LINUX/tools/testing/selftests/nolibc/foo exists.
>
Sorry, I didn't get your meaning above?
Do you mean this line:
srctree := $(patsubst %/tools/testing/selftests/,%,$(dir $(CURDIR)))
by removing tools/testing/selftests/ of curdir's dir (means no nolibc itself),
srctree above is just the top-level kernel source code tree.
> It seems we need to pass $(COMMAND_O) to the recursive calls to $(MAKE),
> too?
>
So, no need to pass O or COMMAND_O.
Thanks,
Zhangjin
> > +
> > ifeq ($(ARCH),)
> > include $(srctree)/scripts/subarch.include
> > ARCH = $(SUBARCH)
> > @@ -217,12 +220,12 @@ kernel: initramfs
> >
> > # run the tests after building the kernel
> > run: kernel
> > - $(Q)qemu-system-$(QEMU_ARCH) -display none -no-reboot -kernel "$(srctree)/$(IMAGE)" -serial stdio $(QEMU_ARGS) > "$(CURDIR)/run.out"
> > + $(Q)qemu-system-$(QEMU_ARCH) -display none -no-reboot -kernel "$(objtree)/$(IMAGE)" -serial stdio $(QEMU_ARGS) > "$(CURDIR)/run.out"
> > $(Q)$(REPORT) $(CURDIR)/run.out
> >
> > # re-run the tests from an existing kernel
> > rerun:
> > - $(Q)qemu-system-$(QEMU_ARCH) -display none -no-reboot -kernel "$(srctree)/$(IMAGE)" -serial stdio $(QEMU_ARGS) > "$(CURDIR)/run.out"
> > + $(Q)qemu-system-$(QEMU_ARCH) -display none -no-reboot -kernel "$(objtree)/$(IMAGE)" -serial stdio $(QEMU_ARGS) > "$(CURDIR)/run.out"
> > $(Q)$(REPORT) $(CURDIR)/run.out
> >
> > # report with existing test log
> > --
> > 2.25.1
> >
On 2023-08-04 15:43:42+0800, Zhangjin Wu wrote:
> Hi, Thomas
>
> > On 2023-08-03 22:45:52+0800, Zhangjin Wu wrote:
> > > To avoid pollute the source code tree and avoid mrproper for every
> > > architecture switch, the O= argument must be supported.
> > >
> > > Both IMAGE and .config are from the building directory, let's use
> > > objtree instead of srctree for them.
> > >
> > > If no O= option specified, means building kernel in source code tree,
> > > objtree should be srctree in such case.
> > >
> > > Suggested-by: Willy Tarreau <[email protected]>
> > > Link: https://lore.kernel.org/lkml/[email protected]/
> > > Signed-off-by: Zhangjin Wu <[email protected]>
> > > ---
> > > tools/testing/selftests/nolibc/Makefile | 7 +++++--
> > > 1 file changed, 5 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/tools/testing/selftests/nolibc/Makefile b/tools/testing/selftests/nolibc/Makefile
> > > index 51fef5e6a152..af590aee063a 100644
> > > --- a/tools/testing/selftests/nolibc/Makefile
> > > +++ b/tools/testing/selftests/nolibc/Makefile
> > > @@ -9,6 +9,9 @@ ifeq ($(srctree),)
> > > srctree := $(patsubst %/tools/testing/selftests/,%,$(dir $(CURDIR)))
> > > endif
> > >
> > > +# add objtree for O= argument, required by IMAGE and .config
> > > +objtree ?= $(srctree)
> >
> > Isn't this already set by the included tools/scripts/Makefile.include?
> >
>
> Good question, but it is empty if no O= specified, checked it several
> times before ;-)
For me it is not empty when I am in tools/testing/selftests/nolibc/.
> > Also I'm not entirely if O= works as intended currently.
> > When using O=foo in the $LINUX/tools/testing/selftests/nolibc directory
>
> It does work, I have used it to test all of the run targets of the
> tinyconfig patches like this:
>
> $ make run O=kernel-$arch RUN_OUT=run.$arch.out ...
>
> Everything about the kernel will be built in $(O).
It does not work for me:
$ cd $HOME/src/linux/tools/testing/selftests/nolibc
$ make O=out kernel
../../../scripts/Makefile.include:4: *** O=out does not exist. Stop.
$ mkdir out
$ make O=out kernel
... Stuff happens
... all the output is in $HOME/src/linux/out not in
tools/testing/selftests/nolibc/out where I would expect it.
The new out directory for which an error was reported before is
completely empty.
> Just rechecked the O variable in top-level Makefile, selftests/nolibc Makefile
> and tools/nolibc Makefile, all of them get the right O value from command line.
But the one from the command line is a relative path. And it seems to be
resolved from the kernel source tree instead of from where make is
executed.
> From my Makefile experience, an option from command line will be passed
> to every sub Makefile via $(-*-command-variables-*-), it has the highest
> priority then the others, except when we use 'override' keyword
> internally.
>
> > the build instead is happening in $LINUX/foo. But the Makefile first
> > validates that $LINUX/tools/testing/selftests/nolibc/foo exists.
> >
>
> Sorry, I didn't get your meaning above?
>
> Do you mean this line:
>
> srctree := $(patsubst %/tools/testing/selftests/,%,$(dir $(CURDIR)))
>
> by removing tools/testing/selftests/ of curdir's dir (means no nolibc itself),
> srctree above is just the top-level kernel source code tree.
No, srctree should be fine.
> > It seems we need to pass $(COMMAND_O) to the recursive calls to $(MAKE),
> > too?
> >
>
> So, no need to pass O or COMMAND_O.
As above, it doesn't work for me.
If I add COMMAND_O to all the $(MAKE) commands that are executed
explicitly, how it is also done in Makefile.include itself, things start
to work as I would expect:
All the kernel build output is in out/ in the current directory.
> Thanks,
> Zhangjin
>
> > > +
> > > ifeq ($(ARCH),)
> > > include $(srctree)/scripts/subarch.include
> > > ARCH = $(SUBARCH)
> > > @@ -217,12 +220,12 @@ kernel: initramfs
> > >
> > > # run the tests after building the kernel
> > > run: kernel
> > > - $(Q)qemu-system-$(QEMU_ARCH) -display none -no-reboot -kernel "$(srctree)/$(IMAGE)" -serial stdio $(QEMU_ARGS) > "$(CURDIR)/run.out"
> > > + $(Q)qemu-system-$(QEMU_ARCH) -display none -no-reboot -kernel "$(objtree)/$(IMAGE)" -serial stdio $(QEMU_ARGS) > "$(CURDIR)/run.out"
> > > $(Q)$(REPORT) $(CURDIR)/run.out
> > >
> > > # re-run the tests from an existing kernel
> > > rerun:
> > > - $(Q)qemu-system-$(QEMU_ARCH) -display none -no-reboot -kernel "$(srctree)/$(IMAGE)" -serial stdio $(QEMU_ARGS) > "$(CURDIR)/run.out"
> > > + $(Q)qemu-system-$(QEMU_ARCH) -display none -no-reboot -kernel "$(objtree)/$(IMAGE)" -serial stdio $(QEMU_ARGS) > "$(CURDIR)/run.out"
> > > $(Q)$(REPORT) $(CURDIR)/run.out
> > >
> > > # report with existing test log
> > > --
> > > 2.25.1
> > >
> On 2023-08-04 15:43:42+0800, Zhangjin Wu wrote:
> > Hi, Thomas
> >
> > > On 2023-08-03 22:45:52+0800, Zhangjin Wu wrote:
> > > > To avoid pollute the source code tree and avoid mrproper for every
> > > > architecture switch, the O= argument must be supported.
> > > >
> > > > Both IMAGE and .config are from the building directory, let's use
> > > > objtree instead of srctree for them.
> > > >
> > > > If no O= option specified, means building kernel in source code tree,
> > > > objtree should be srctree in such case.
> > > >
> > > > Suggested-by: Willy Tarreau <[email protected]>
> > > > Link: https://lore.kernel.org/lkml/[email protected]/
> > > > Signed-off-by: Zhangjin Wu <[email protected]>
> > > > ---
> > > > tools/testing/selftests/nolibc/Makefile | 7 +++++--
> > > > 1 file changed, 5 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/tools/testing/selftests/nolibc/Makefile b/tools/testing/selftests/nolibc/Makefile
> > > > index 51fef5e6a152..af590aee063a 100644
> > > > --- a/tools/testing/selftests/nolibc/Makefile
> > > > +++ b/tools/testing/selftests/nolibc/Makefile
> > > > @@ -9,6 +9,9 @@ ifeq ($(srctree),)
> > > > srctree := $(patsubst %/tools/testing/selftests/,%,$(dir $(CURDIR)))
> > > > endif
> > > >
> > > > +# add objtree for O= argument, required by IMAGE and .config
> > > > +objtree ?= $(srctree)
> > >
> > > Isn't this already set by the included tools/scripts/Makefile.include?
> > >
> >
> > Good question, but it is empty if no O= specified, checked it several
> > times before ;-)
>
> For me it is not empty when I am in tools/testing/selftests/nolibc/.
>
Interesting, here is the code I added to check the value:
diff --git a/tools/testing/selftests/nolibc/Makefile b/tools/testing/selftests/nolibc/Makefile
index 22f1e1d73fa8..1ae19e896e24 100644
--- a/tools/testing/selftests/nolibc/Makefile
+++ b/tools/testing/selftests/nolibc/Makefile
@@ -12,6 +12,8 @@ include $(srctree)/scripts/subarch.include
ARCH = $(SUBARCH)
endif
+$(error objtree=$(objtree), srctree=$(srctree))
+
Whenever I do defconfig or run,
$ make help
Makefile:15: *** objtree=, srctree=/labs/linux-lab/src/linux-stable. Stop.
It is only not empty when we pass O explicitly:
$ mkdir out
$ make help O=out
Makefile:15: *** objtree=out, srctree=/labs/linux-lab/src/linux-stable. Stop.
$ make help O=$PWD/out
Makefile:15: *** objtree=/labs/linux-lab/src/linux-stable/tools/testing/selftests/nolibc/out, srctree=/labs/linux-lab/src/linux-stable. Stop.
> > > Also I'm not entirely if O= works as intended currently.
> > > When using O=foo in the $LINUX/tools/testing/selftests/nolibc directory
> >
> > It does work, I have used it to test all of the run targets of the
> > tinyconfig patches like this:
> >
> > $ make run O=kernel-$arch RUN_OUT=run.$arch.out ...
> >
> > Everything about the kernel will be built in $(O).
>
> It does not work for me:
>
> $ cd $HOME/src/linux/tools/testing/selftests/nolibc
> $ make O=out kernel
> ../../../scripts/Makefile.include:4: *** O=out does not exist. Stop.
> $ mkdir out
> $ make O=out kernel
> ... Stuff happens
> ... all the output is in $HOME/src/linux/out not in
> tools/testing/selftests/nolibc/out where I would expect it.
> The new out directory for which an error was reported before is
> completely empty.
>
Oh, yeah, my fault, I have always used the absolute path, but pasted a relative
one above, this is the one I really used for every test:
$ make run O=$PWD/kernel-$arch RUN_OUT=run.$arch.out ...
> > Just rechecked the O variable in top-level Makefile, selftests/nolibc Makefile
> > and tools/nolibc Makefile, all of them get the right O value from command line.
>
> But the one from the command line is a relative path. And it seems to be
> resolved from the kernel source tree instead of from where make is
> executed.
>
That's exactly:
$ make defconfig O=out
Makefile:15: objtree=out, srctree=/labs/linux-lab/src/linux-stable
Makefile:114: O=/labs/linux-lab/src/linux-stable/tools/testing/selftests/nolibc/out
The COMMAND_O suggested by you have prefixed $(PWD) for us, it is the right way
we should apply. Seems I have passed O=something to all kernel targets before,
but that is a little ugly, and when the O=$PWD/out always work for me, I even
forgot to check the simpler O=out again.
And it also works perfectly with $PWD:
$ make defconfig O=$PWD/out
Makefile:15: objtree=/labs/linux-lab/src/linux-stable/tools/testing/selftests/nolibc/out, srctree=/labs/linux-lab/src/linux-stable
Makefile:114: O=/labs/linux-lab/src/linux-stable/tools/testing/selftests/nolibc/out
> > From my Makefile experience, an option from command line will be passed
> > to every sub Makefile via $(-*-command-variables-*-), it has the highest
> > priority then the others, except when we use 'override' keyword
> > internally.
> >
> > > the build instead is happening in $LINUX/foo. But the Makefile first
> > > validates that $LINUX/tools/testing/selftests/nolibc/foo exists.
> > >
> >
> > Sorry, I didn't get your meaning above?
> >
> > Do you mean this line:
> >
> > srctree := $(patsubst %/tools/testing/selftests/,%,$(dir $(CURDIR)))
> >
> > by removing tools/testing/selftests/ of curdir's dir (means no nolibc itself),
> > srctree above is just the top-level kernel source code tree.
>
> No, srctree should be fine.
>
> > > It seems we need to pass $(COMMAND_O) to the recursive calls to $(MAKE),
> > > too?
> > >
> >
> > So, no need to pass O or COMMAND_O.
>
> As above, it doesn't work for me.
>
> If I add COMMAND_O to all the $(MAKE) commands that are executed
> explicitly, how it is also done in Makefile.include itself, things start
> to work as I would expect:
> All the kernel build output is in out/ in the current directory.
>
Ok, let's use it. Thanks very much.
After align the empty objtree value with you, will renew this patch.
Best regards,
Zhangjin
> On 2023-08-04 23:52:18+0800, Zhangjin Wu wrote:
> > > On 2023-08-04 15:43:42+0800, Zhangjin Wu wrote:
> > > > Hi, Thomas
> > > >
> > > > > On 2023-08-03 22:45:52+0800, Zhangjin Wu wrote:
> > > > > > To avoid pollute the source code tree and avoid mrproper for every
> > > > > > architecture switch, the O= argument must be supported.
> > > > > >
> > > > > > Both IMAGE and .config are from the building directory, let's use
> > > > > > objtree instead of srctree for them.
> > > > > >
> > > > > > If no O= option specified, means building kernel in source code tree,
> > > > > > objtree should be srctree in such case.
> > > > > >
> > > > > > Suggested-by: Willy Tarreau <[email protected]>
> > > > > > Link: https://lore.kernel.org/lkml/[email protected]/
> > > > > > Signed-off-by: Zhangjin Wu <[email protected]>
> > > > > > ---
> > > > > > tools/testing/selftests/nolibc/Makefile | 7 +++++--
> > > > > > 1 file changed, 5 insertions(+), 2 deletions(-)
> > > > > >
> > > > > > diff --git a/tools/testing/selftests/nolibc/Makefile b/tools/testing/selftests/nolibc/Makefile
> > > > > > index 51fef5e6a152..af590aee063a 100644
> > > > > > --- a/tools/testing/selftests/nolibc/Makefile
> > > > > > +++ b/tools/testing/selftests/nolibc/Makefile
> > > > > > @@ -9,6 +9,9 @@ ifeq ($(srctree),)
> > > > > > srctree := $(patsubst %/tools/testing/selftests/,%,$(dir $(CURDIR)))
> > > > > > endif
> > > > > >
> > > > > > +# add objtree for O= argument, required by IMAGE and .config
> > > > > > +objtree ?= $(srctree)
> > > > >
> > > > > Isn't this already set by the included tools/scripts/Makefile.include?
> > > > >
> > > >
> > > > Good question, but it is empty if no O= specified, checked it several
> > > > times before ;-)
> > >
> > > For me it is not empty when I am in tools/testing/selftests/nolibc/.
> > >
> >
> > Interesting, here is the code I added to check the value:
> >
> > diff --git a/tools/testing/selftests/nolibc/Makefile b/tools/testing/selftests/nolibc/Makefile
> > index 22f1e1d73fa8..1ae19e896e24 100644
> > --- a/tools/testing/selftests/nolibc/Makefile
> > +++ b/tools/testing/selftests/nolibc/Makefile
> > @@ -12,6 +12,8 @@ include $(srctree)/scripts/subarch.include
> > ARCH = $(SUBARCH)
> > endif
> >
> > +$(error objtree=$(objtree), srctree=$(srctree))
> > +
> >
> > Whenever I do defconfig or run,
> >
> > $ make help
> > Makefile:15: *** objtree=, srctree=/labs/linux-lab/src/linux-stable. Stop.
> >
> > It is only not empty when we pass O explicitly:
> >
> > $ mkdir out
> > $ make help O=out
> > Makefile:15: *** objtree=out, srctree=/labs/linux-lab/src/linux-stable. Stop.
> > $ make help O=$PWD/out
> > Makefile:15: *** objtree=/labs/linux-lab/src/linux-stable/tools/testing/selftests/nolibc/out, srctree=/labs/linux-lab/src/linux-stable. Stop.
>
> Welp, now it's the same for me.
> I guess I messed it up before, maybe I forgot to remove your changes
> while testing?
>
> Anyways instead of having to manually do stuff with $(objtree) we could
> also use $(OUTPUT)$(IMAGE) to always get the correct image.
>
Do you mean here?
# kernel image names by architecture
IMAGE_i386 = arch/x86/boot/bzImage
IMAGE_x86 = arch/x86/boot/bzImage
IMAGE_arm64 = arch/arm64/boot/Image
IMAGE_arm = arch/arm/boot/zImage
IMAGE_mips = vmlinuz
IMAGE_riscv = arch/riscv/boot/Image
IMAGE = $(IMAGE_$(ARCH))
IMAGE_NAME = $(notdir $(IMAGE))
It does save another KERNEL_IMAGE macro in my future patch ;-)
But without O=, OUTPUT is also empty like objtree and when empty, it is
assigned as $(CURDIR), not $(srctree) as we expected for IMAGE and .config. To
be cleaner, objtree should also be used:
- IMAGE = $(IMAGE_$(ARCH))
+ IMAGE = $(objtree)/$(IMAGE_$(ARCH))
Is this what you want?
Thanks!
Zhangjin
> > [..]
> >
> > After align the empty objtree value with you, will renew this patch.
>
> Thanks!
> Thomas
On 2023-08-04 23:52:18+0800, Zhangjin Wu wrote:
> > On 2023-08-04 15:43:42+0800, Zhangjin Wu wrote:
> > > Hi, Thomas
> > >
> > > > On 2023-08-03 22:45:52+0800, Zhangjin Wu wrote:
> > > > > To avoid pollute the source code tree and avoid mrproper for every
> > > > > architecture switch, the O= argument must be supported.
> > > > >
> > > > > Both IMAGE and .config are from the building directory, let's use
> > > > > objtree instead of srctree for them.
> > > > >
> > > > > If no O= option specified, means building kernel in source code tree,
> > > > > objtree should be srctree in such case.
> > > > >
> > > > > Suggested-by: Willy Tarreau <[email protected]>
> > > > > Link: https://lore.kernel.org/lkml/[email protected]/
> > > > > Signed-off-by: Zhangjin Wu <[email protected]>
> > > > > ---
> > > > > tools/testing/selftests/nolibc/Makefile | 7 +++++--
> > > > > 1 file changed, 5 insertions(+), 2 deletions(-)
> > > > >
> > > > > diff --git a/tools/testing/selftests/nolibc/Makefile b/tools/testing/selftests/nolibc/Makefile
> > > > > index 51fef5e6a152..af590aee063a 100644
> > > > > --- a/tools/testing/selftests/nolibc/Makefile
> > > > > +++ b/tools/testing/selftests/nolibc/Makefile
> > > > > @@ -9,6 +9,9 @@ ifeq ($(srctree),)
> > > > > srctree := $(patsubst %/tools/testing/selftests/,%,$(dir $(CURDIR)))
> > > > > endif
> > > > >
> > > > > +# add objtree for O= argument, required by IMAGE and .config
> > > > > +objtree ?= $(srctree)
> > > >
> > > > Isn't this already set by the included tools/scripts/Makefile.include?
> > > >
> > >
> > > Good question, but it is empty if no O= specified, checked it several
> > > times before ;-)
> >
> > For me it is not empty when I am in tools/testing/selftests/nolibc/.
> >
>
> Interesting, here is the code I added to check the value:
>
> diff --git a/tools/testing/selftests/nolibc/Makefile b/tools/testing/selftests/nolibc/Makefile
> index 22f1e1d73fa8..1ae19e896e24 100644
> --- a/tools/testing/selftests/nolibc/Makefile
> +++ b/tools/testing/selftests/nolibc/Makefile
> @@ -12,6 +12,8 @@ include $(srctree)/scripts/subarch.include
> ARCH = $(SUBARCH)
> endif
>
> +$(error objtree=$(objtree), srctree=$(srctree))
> +
>
> Whenever I do defconfig or run,
>
> $ make help
> Makefile:15: *** objtree=, srctree=/labs/linux-lab/src/linux-stable. Stop.
>
> It is only not empty when we pass O explicitly:
>
> $ mkdir out
> $ make help O=out
> Makefile:15: *** objtree=out, srctree=/labs/linux-lab/src/linux-stable. Stop.
> $ make help O=$PWD/out
> Makefile:15: *** objtree=/labs/linux-lab/src/linux-stable/tools/testing/selftests/nolibc/out, srctree=/labs/linux-lab/src/linux-stable. Stop.
Welp, now it's the same for me.
I guess I messed it up before, maybe I forgot to remove your changes
while testing?
Anyways instead of having to manually do stuff with $(objtree) we could
also use $(OUTPUT)$(IMAGE) to always get the correct image.
> [..]
>
> After align the empty objtree value with you, will renew this patch.
Thanks!
Thomas
On 2023-08-05 00:29:10+0800, Zhangjin Wu wrote:
> > On 2023-08-04 23:52:18+0800, Zhangjin Wu wrote:
> > > > On 2023-08-04 15:43:42+0800, Zhangjin Wu wrote:
> > > > > Hi, Thomas
> > > > >
> > > > > > On 2023-08-03 22:45:52+0800, Zhangjin Wu wrote:
> > > > > > > To avoid pollute the source code tree and avoid mrproper for every
> > > > > > > architecture switch, the O= argument must be supported.
> > > > > > >
> > > > > > > Both IMAGE and .config are from the building directory, let's use
> > > > > > > objtree instead of srctree for them.
> > > > > > >
> > > > > > > If no O= option specified, means building kernel in source code tree,
> > > > > > > objtree should be srctree in such case.
> > > > > > >
> > > > > > > Suggested-by: Willy Tarreau <[email protected]>
> > > > > > > Link: https://lore.kernel.org/lkml/[email protected]/
> > > > > > > Signed-off-by: Zhangjin Wu <[email protected]>
> > > > > > > ---
> > > > > > > tools/testing/selftests/nolibc/Makefile | 7 +++++--
> > > > > > > 1 file changed, 5 insertions(+), 2 deletions(-)
> > > > > > >
> > > > > > > diff --git a/tools/testing/selftests/nolibc/Makefile b/tools/testing/selftests/nolibc/Makefile
> > > > > > > index 51fef5e6a152..af590aee063a 100644
> > > > > > > --- a/tools/testing/selftests/nolibc/Makefile
> > > > > > > +++ b/tools/testing/selftests/nolibc/Makefile
> > > > > > > @@ -9,6 +9,9 @@ ifeq ($(srctree),)
> > > > > > > srctree := $(patsubst %/tools/testing/selftests/,%,$(dir $(CURDIR)))
> > > > > > > endif
> > > > > > >
> > > > > > > +# add objtree for O= argument, required by IMAGE and .config
> > > > > > > +objtree ?= $(srctree)
> > > > > >
> > > > > > Isn't this already set by the included tools/scripts/Makefile.include?
> > > > > >
> > > > >
> > > > > Good question, but it is empty if no O= specified, checked it several
> > > > > times before ;-)
> > > >
> > > > For me it is not empty when I am in tools/testing/selftests/nolibc/.
> > > >
> > >
> > > Interesting, here is the code I added to check the value:
> > >
> > > diff --git a/tools/testing/selftests/nolibc/Makefile b/tools/testing/selftests/nolibc/Makefile
> > > index 22f1e1d73fa8..1ae19e896e24 100644
> > > --- a/tools/testing/selftests/nolibc/Makefile
> > > +++ b/tools/testing/selftests/nolibc/Makefile
> > > @@ -12,6 +12,8 @@ include $(srctree)/scripts/subarch.include
> > > ARCH = $(SUBARCH)
> > > endif
> > >
> > > +$(error objtree=$(objtree), srctree=$(srctree))
> > > +
> > >
> > > Whenever I do defconfig or run,
> > >
> > > $ make help
> > > Makefile:15: *** objtree=, srctree=/labs/linux-lab/src/linux-stable. Stop.
> > >
> > > It is only not empty when we pass O explicitly:
> > >
> > > $ mkdir out
> > > $ make help O=out
> > > Makefile:15: *** objtree=out, srctree=/labs/linux-lab/src/linux-stable. Stop.
> > > $ make help O=$PWD/out
> > > Makefile:15: *** objtree=/labs/linux-lab/src/linux-stable/tools/testing/selftests/nolibc/out, srctree=/labs/linux-lab/src/linux-stable. Stop.
> >
> > Welp, now it's the same for me.
> > I guess I messed it up before, maybe I forgot to remove your changes
> > while testing?
> >
> > Anyways instead of having to manually do stuff with $(objtree) we could
> > also use $(OUTPUT)$(IMAGE) to always get the correct image.
> >
>
> Do you mean here?
>
> # kernel image names by architecture
> IMAGE_i386 = arch/x86/boot/bzImage
> IMAGE_x86 = arch/x86/boot/bzImage
> IMAGE_arm64 = arch/arm64/boot/Image
> IMAGE_arm = arch/arm/boot/zImage
> IMAGE_mips = vmlinuz
> IMAGE_riscv = arch/riscv/boot/Image
> IMAGE = $(IMAGE_$(ARCH))
> IMAGE_NAME = $(notdir $(IMAGE))
>
> It does save another KERNEL_IMAGE macro in my future patch ;-)
>
> But without O=, OUTPUT is also empty like objtree and when empty, it is
> assigned as $(CURDIR), not $(srctree) as we expected for IMAGE and .config. To
> be cleaner, objtree should also be used:
>
> - IMAGE = $(IMAGE_$(ARCH))
> + IMAGE = $(objtree)/$(IMAGE_$(ARCH))
>
> Is this what you want?
More like:
- $(Q)qemu-system-$(QEMU_ARCH) -display none -no-reboot -kernel "$(srctree)/$(IMAGE)" -serial stdio $(QEMU_ARGS) > "$(CURDIR)/run.out"
+ $(Q)qemu-system-$(QEMU_ARCH) -display none -no-reboot -kernel "$(OUTPUT)$(IMAGE)" -serial stdio $(QEMU_ARGS) > "$(CURDIR)/run.out"
My assumption was that it's weird that we need to define such variables
ourselves.
Using an empty $(OUTPUT) would have been fine if make is run from the
root of the kernel tree. But that is not the case.
It still feels weird but I can't think of a nicer way, and it's not
a big issue. So let's keep that part the same.
Or maybe Willy has a better idea.
On Sat, Aug 05, 2023 at 02:40:23AM +0800, Zhangjin Wu wrote:
> > On Fri, Aug 04, 2023 at 07:51:50PM +0200, Thomas Wei?schuh wrote:
> > > On 2023-08-05 00:29:10+0800, Zhangjin Wu wrote:
> [...]
> > > > Do you mean here?
> > > >
> > > > # kernel image names by architecture
> > > > IMAGE_i386 = arch/x86/boot/bzImage
> > > > IMAGE_x86 = arch/x86/boot/bzImage
> > > > IMAGE_arm64 = arch/arm64/boot/Image
> > > > IMAGE_arm = arch/arm/boot/zImage
> > > > IMAGE_mips = vmlinuz
> > > > IMAGE_riscv = arch/riscv/boot/Image
> > > > IMAGE = $(IMAGE_$(ARCH))
> > > > IMAGE_NAME = $(notdir $(IMAGE))
> > > >
> > > > It does save another KERNEL_IMAGE macro in my future patch ;-)
> > > >
> > > > But without O=, OUTPUT is also empty like objtree and when empty, it is
> > > > assigned as $(CURDIR), not $(srctree) as we expected for IMAGE and .config. To
> > > > be cleaner, objtree should also be used:
> > > >
> > > > - IMAGE = $(IMAGE_$(ARCH))
> > > > + IMAGE = $(objtree)/$(IMAGE_$(ARCH))
> > > >
> > > > Is this what you want?
> > >
> > > More like:
> > >
> > > - $(Q)qemu-system-$(QEMU_ARCH) -display none -no-reboot -kernel "$(srctree)/$(IMAGE)" -serial stdio $(QEMU_ARGS) > "$(CURDIR)/run.out"
> > > + $(Q)qemu-system-$(QEMU_ARCH) -display none -no-reboot -kernel "$(OUTPUT)$(IMAGE)" -serial stdio $(QEMU_ARGS) > "$(CURDIR)/run.out"
> > >
> > > My assumption was that it's weird that we need to define such variables
> > > ourselves.
> > >
> > > Using an empty $(OUTPUT) would have been fine if make is run from the
> > > root of the kernel tree. But that is not the case.
> > >
> > > It still feels weird but I can't think of a nicer way, and it's not
> > > a big issue. So let's keep that part the same.
> > >
> > > Or maybe Willy has a better idea.
> >
> > I've just glanced over the discussion, but I'm wondering, why not
> > "$(objtree)/$(IMAGE)" instead ?
> >
>
> We used "$(objtree)/$(IMAGE)" originally, I thought of Thomas asked me to
> further add "$(objtree)" in the first place of IMAGE,
>
> - IMAGE = $(IMAGE_$(ARCH))
> + IMAGE = $(objtree)/$(IMAGE_$(ARCH))
>
> And then, we can use "IMAGE" directly in the qemu run & rerun targets:
>
> # run the tests after building the kernel
> run: kernel
> - $(Q)qemu-system-$(QEMU_ARCH) -display none -no-reboot -kernel "$(objtree)/$(IMAGE)" -serial stdio $(QEMU_ARGS) > "$(RUN_OUT)"
> + $(Q)qemu-system-$(QEMU_ARCH) -display none -no-reboot -kernel "$(IMAGE)" -serial stdio $(QEMU_ARGS) > "$(RUN_OUT)"
> $(Q)$(REPORT) "$(RUN_OUT)"
>
> # re-run the tests from an existing kernel
> rerun:
> - $(Q)qemu-system-$(QEMU_ARCH) -display none -no-reboot -kernel "$(objtree)/$(IMAGE)" -serial stdio $(QEMU_ARGS) > "$(RUN_OUT)"
> + $(Q)qemu-system-$(QEMU_ARCH) -display none -no-reboot -kernel "$(IMAGE)" -serial stdio $(QEMU_ARGS) > "$(RUN_OUT)"
> $(Q)$(REPORT) "$(RUN_OUT)"
>
> Which one do you prefer? will renew this series soon.
It's not a matter of preference but which solution really works. In short
it should work fine and consistently when launched from topdir and from
the nolibc-test dir. I personally don't know the effect of O= when run
from a subdir (is it relative to the current dir or topdir?).
> And even further, I thought of puting everything to $(OUTPUT), but the
> change is very ugly and not good for v6.6, If you like, I will send that
> patch only as a discuss stuff.
Yeah I'd rather avoid ugly things for 6.6 now.
Thanks,
Willy
> On Fri, Aug 04, 2023 at 07:51:50PM +0200, Thomas Wei?schuh wrote:
> > On 2023-08-05 00:29:10+0800, Zhangjin Wu wrote:
[...]
> > > Do you mean here?
> > >
> > > # kernel image names by architecture
> > > IMAGE_i386 = arch/x86/boot/bzImage
> > > IMAGE_x86 = arch/x86/boot/bzImage
> > > IMAGE_arm64 = arch/arm64/boot/Image
> > > IMAGE_arm = arch/arm/boot/zImage
> > > IMAGE_mips = vmlinuz
> > > IMAGE_riscv = arch/riscv/boot/Image
> > > IMAGE = $(IMAGE_$(ARCH))
> > > IMAGE_NAME = $(notdir $(IMAGE))
> > >
> > > It does save another KERNEL_IMAGE macro in my future patch ;-)
> > >
> > > But without O=, OUTPUT is also empty like objtree and when empty, it is
> > > assigned as $(CURDIR), not $(srctree) as we expected for IMAGE and .config. To
> > > be cleaner, objtree should also be used:
> > >
> > > - IMAGE = $(IMAGE_$(ARCH))
> > > + IMAGE = $(objtree)/$(IMAGE_$(ARCH))
> > >
> > > Is this what you want?
> >
> > More like:
> >
> > - $(Q)qemu-system-$(QEMU_ARCH) -display none -no-reboot -kernel "$(srctree)/$(IMAGE)" -serial stdio $(QEMU_ARGS) > "$(CURDIR)/run.out"
> > + $(Q)qemu-system-$(QEMU_ARCH) -display none -no-reboot -kernel "$(OUTPUT)$(IMAGE)" -serial stdio $(QEMU_ARGS) > "$(CURDIR)/run.out"
> >
> > My assumption was that it's weird that we need to define such variables
> > ourselves.
> >
> > Using an empty $(OUTPUT) would have been fine if make is run from the
> > root of the kernel tree. But that is not the case.
> >
> > It still feels weird but I can't think of a nicer way, and it's not
> > a big issue. So let's keep that part the same.
> >
> > Or maybe Willy has a better idea.
>
> I've just glanced over the discussion, but I'm wondering, why not
> "$(objtree)/$(IMAGE)" instead ?
>
We used "$(objtree)/$(IMAGE)" originally, I thought of Thomas asked me to
further add "$(objtree)" in the first place of IMAGE,
- IMAGE = $(IMAGE_$(ARCH))
+ IMAGE = $(objtree)/$(IMAGE_$(ARCH))
And then, we can use "IMAGE" directly in the qemu run & rerun targets:
# run the tests after building the kernel
run: kernel
- $(Q)qemu-system-$(QEMU_ARCH) -display none -no-reboot -kernel "$(objtree)/$(IMAGE)" -serial stdio $(QEMU_ARGS) > "$(RUN_OUT)"
+ $(Q)qemu-system-$(QEMU_ARCH) -display none -no-reboot -kernel "$(IMAGE)" -serial stdio $(QEMU_ARGS) > "$(RUN_OUT)"
$(Q)$(REPORT) "$(RUN_OUT)"
# re-run the tests from an existing kernel
rerun:
- $(Q)qemu-system-$(QEMU_ARCH) -display none -no-reboot -kernel "$(objtree)/$(IMAGE)" -serial stdio $(QEMU_ARGS) > "$(RUN_OUT)"
+ $(Q)qemu-system-$(QEMU_ARCH) -display none -no-reboot -kernel "$(IMAGE)" -serial stdio $(QEMU_ARGS) > "$(RUN_OUT)"
$(Q)$(REPORT) "$(RUN_OUT)"
Which one do you prefer? will renew this series soon.
And even further, I thought of puting everything to $(OUTPUT), but the
change is very ugly and not good for v6.6, If you like, I will send that
patch only as a discuss stuff.
Thanks,
Zhangjin
> Willy
On Fri, Aug 04, 2023 at 07:51:50PM +0200, Thomas Wei?schuh wrote:
> On 2023-08-05 00:29:10+0800, Zhangjin Wu wrote:
> > > On 2023-08-04 23:52:18+0800, Zhangjin Wu wrote:
> > > > > On 2023-08-04 15:43:42+0800, Zhangjin Wu wrote:
> > > > > > Hi, Thomas
> > > > > >
> > > > > > > On 2023-08-03 22:45:52+0800, Zhangjin Wu wrote:
> > > > > > > > To avoid pollute the source code tree and avoid mrproper for every
> > > > > > > > architecture switch, the O= argument must be supported.
> > > > > > > >
> > > > > > > > Both IMAGE and .config are from the building directory, let's use
> > > > > > > > objtree instead of srctree for them.
> > > > > > > >
> > > > > > > > If no O= option specified, means building kernel in source code tree,
> > > > > > > > objtree should be srctree in such case.
> > > > > > > >
> > > > > > > > Suggested-by: Willy Tarreau <[email protected]>
> > > > > > > > Link: https://lore.kernel.org/lkml/[email protected]/
> > > > > > > > Signed-off-by: Zhangjin Wu <[email protected]>
> > > > > > > > ---
> > > > > > > > tools/testing/selftests/nolibc/Makefile | 7 +++++--
> > > > > > > > 1 file changed, 5 insertions(+), 2 deletions(-)
> > > > > > > >
> > > > > > > > diff --git a/tools/testing/selftests/nolibc/Makefile b/tools/testing/selftests/nolibc/Makefile
> > > > > > > > index 51fef5e6a152..af590aee063a 100644
> > > > > > > > --- a/tools/testing/selftests/nolibc/Makefile
> > > > > > > > +++ b/tools/testing/selftests/nolibc/Makefile
> > > > > > > > @@ -9,6 +9,9 @@ ifeq ($(srctree),)
> > > > > > > > srctree := $(patsubst %/tools/testing/selftests/,%,$(dir $(CURDIR)))
> > > > > > > > endif
> > > > > > > >
> > > > > > > > +# add objtree for O= argument, required by IMAGE and .config
> > > > > > > > +objtree ?= $(srctree)
> > > > > > >
> > > > > > > Isn't this already set by the included tools/scripts/Makefile.include?
> > > > > > >
> > > > > >
> > > > > > Good question, but it is empty if no O= specified, checked it several
> > > > > > times before ;-)
> > > > >
> > > > > For me it is not empty when I am in tools/testing/selftests/nolibc/.
> > > > >
> > > >
> > > > Interesting, here is the code I added to check the value:
> > > >
> > > > diff --git a/tools/testing/selftests/nolibc/Makefile b/tools/testing/selftests/nolibc/Makefile
> > > > index 22f1e1d73fa8..1ae19e896e24 100644
> > > > --- a/tools/testing/selftests/nolibc/Makefile
> > > > +++ b/tools/testing/selftests/nolibc/Makefile
> > > > @@ -12,6 +12,8 @@ include $(srctree)/scripts/subarch.include
> > > > ARCH = $(SUBARCH)
> > > > endif
> > > >
> > > > +$(error objtree=$(objtree), srctree=$(srctree))
> > > > +
> > > >
> > > > Whenever I do defconfig or run,
> > > >
> > > > $ make help
> > > > Makefile:15: *** objtree=, srctree=/labs/linux-lab/src/linux-stable. Stop.
> > > >
> > > > It is only not empty when we pass O explicitly:
> > > >
> > > > $ mkdir out
> > > > $ make help O=out
> > > > Makefile:15: *** objtree=out, srctree=/labs/linux-lab/src/linux-stable. Stop.
> > > > $ make help O=$PWD/out
> > > > Makefile:15: *** objtree=/labs/linux-lab/src/linux-stable/tools/testing/selftests/nolibc/out, srctree=/labs/linux-lab/src/linux-stable. Stop.
> > >
> > > Welp, now it's the same for me.
> > > I guess I messed it up before, maybe I forgot to remove your changes
> > > while testing?
> > >
> > > Anyways instead of having to manually do stuff with $(objtree) we could
> > > also use $(OUTPUT)$(IMAGE) to always get the correct image.
> > >
> >
> > Do you mean here?
> >
> > # kernel image names by architecture
> > IMAGE_i386 = arch/x86/boot/bzImage
> > IMAGE_x86 = arch/x86/boot/bzImage
> > IMAGE_arm64 = arch/arm64/boot/Image
> > IMAGE_arm = arch/arm/boot/zImage
> > IMAGE_mips = vmlinuz
> > IMAGE_riscv = arch/riscv/boot/Image
> > IMAGE = $(IMAGE_$(ARCH))
> > IMAGE_NAME = $(notdir $(IMAGE))
> >
> > It does save another KERNEL_IMAGE macro in my future patch ;-)
> >
> > But without O=, OUTPUT is also empty like objtree and when empty, it is
> > assigned as $(CURDIR), not $(srctree) as we expected for IMAGE and .config. To
> > be cleaner, objtree should also be used:
> >
> > - IMAGE = $(IMAGE_$(ARCH))
> > + IMAGE = $(objtree)/$(IMAGE_$(ARCH))
> >
> > Is this what you want?
>
> More like:
>
> - $(Q)qemu-system-$(QEMU_ARCH) -display none -no-reboot -kernel "$(srctree)/$(IMAGE)" -serial stdio $(QEMU_ARGS) > "$(CURDIR)/run.out"
> + $(Q)qemu-system-$(QEMU_ARCH) -display none -no-reboot -kernel "$(OUTPUT)$(IMAGE)" -serial stdio $(QEMU_ARGS) > "$(CURDIR)/run.out"
>
> My assumption was that it's weird that we need to define such variables
> ourselves.
>
> Using an empty $(OUTPUT) would have been fine if make is run from the
> root of the kernel tree. But that is not the case.
>
> It still feels weird but I can't think of a nicer way, and it's not
> a big issue. So let's keep that part the same.
>
> Or maybe Willy has a better idea.
I've just glanced over the discussion, but I'm wondering, why not
"$(objtree)/$(IMAGE)" instead ?
Willy
> On Sat, Aug 05, 2023 at 02:40:23AM +0800, Zhangjin Wu wrote:
> > > On Fri, Aug 04, 2023 at 07:51:50PM +0200, Thomas Wei?schuh wrote:
> > > > On 2023-08-05 00:29:10+0800, Zhangjin Wu wrote:
> > [...]
> > > > > Do you mean here?
> > > > >
> > > > > # kernel image names by architecture
> > > > > IMAGE_i386 = arch/x86/boot/bzImage
> > > > > IMAGE_x86 = arch/x86/boot/bzImage
> > > > > IMAGE_arm64 = arch/arm64/boot/Image
> > > > > IMAGE_arm = arch/arm/boot/zImage
> > > > > IMAGE_mips = vmlinuz
> > > > > IMAGE_riscv = arch/riscv/boot/Image
> > > > > IMAGE = $(IMAGE_$(ARCH))
> > > > > IMAGE_NAME = $(notdir $(IMAGE))
> > > > >
> > > > > It does save another KERNEL_IMAGE macro in my future patch ;-)
> > > > >
> > > > > But without O=, OUTPUT is also empty like objtree and when empty, it is
> > > > > assigned as $(CURDIR), not $(srctree) as we expected for IMAGE and .config. To
> > > > > be cleaner, objtree should also be used:
> > > > >
> > > > > - IMAGE = $(IMAGE_$(ARCH))
> > > > > + IMAGE = $(objtree)/$(IMAGE_$(ARCH))
> > > > >
> > > > > Is this what you want?
> > > >
> > > > More like:
> > > >
> > > > - $(Q)qemu-system-$(QEMU_ARCH) -display none -no-reboot -kernel "$(srctree)/$(IMAGE)" -serial stdio $(QEMU_ARGS) > "$(CURDIR)/run.out"
> > > > + $(Q)qemu-system-$(QEMU_ARCH) -display none -no-reboot -kernel "$(OUTPUT)$(IMAGE)" -serial stdio $(QEMU_ARGS) > "$(CURDIR)/run.out"
> > > >
> > > > My assumption was that it's weird that we need to define such variables
> > > > ourselves.
> > > >
> > > > Using an empty $(OUTPUT) would have been fine if make is run from the
> > > > root of the kernel tree. But that is not the case.
> > > >
> > > > It still feels weird but I can't think of a nicer way, and it's not
> > > > a big issue. So let's keep that part the same.
> > > >
> > > > Or maybe Willy has a better idea.
> > >
> > > I've just glanced over the discussion, but I'm wondering, why not
> > > "$(objtree)/$(IMAGE)" instead ?
> > >
> >
> > We used "$(objtree)/$(IMAGE)" originally, I thought of Thomas asked me to
> > further add "$(objtree)" in the first place of IMAGE,
> >
> > - IMAGE = $(IMAGE_$(ARCH))
> > + IMAGE = $(objtree)/$(IMAGE_$(ARCH))
> >
> > And then, we can use "IMAGE" directly in the qemu run & rerun targets:
> >
> > # run the tests after building the kernel
> > run: kernel
> > - $(Q)qemu-system-$(QEMU_ARCH) -display none -no-reboot -kernel "$(objtree)/$(IMAGE)" -serial stdio $(QEMU_ARGS) > "$(RUN_OUT)"
> > + $(Q)qemu-system-$(QEMU_ARCH) -display none -no-reboot -kernel "$(IMAGE)" -serial stdio $(QEMU_ARGS) > "$(RUN_OUT)"
> > $(Q)$(REPORT) "$(RUN_OUT)"
> >
> > # re-run the tests from an existing kernel
> > rerun:
> > - $(Q)qemu-system-$(QEMU_ARCH) -display none -no-reboot -kernel "$(objtree)/$(IMAGE)" -serial stdio $(QEMU_ARGS) > "$(RUN_OUT)"
> > + $(Q)qemu-system-$(QEMU_ARCH) -display none -no-reboot -kernel "$(IMAGE)" -serial stdio $(QEMU_ARGS) > "$(RUN_OUT)"
> > $(Q)$(REPORT) "$(RUN_OUT)"
> >
> > Which one do you prefer? will renew this series soon.
>
> It's not a matter of preference but which solution really works. In short
> it should work fine and consistently when launched from topdir and from
> the nolibc-test dir. I personally don't know the effect of O= when run
> from a subdir (is it relative to the current dir or topdir?).
>
As suggested by Thomas, $(COMMAND_O) will be used to pass the absolute
$(O) for every sub $(MAKE), O= will work like this:
// subdir
$ pwd
/labs/linux-lab/src/linux-stable/tools/testing/selftests/nolibc
$ mkdir -p out
$ make run-user O=out -C /labs/linux-lab/src/linux-stable/tools/testing/selftests/nolibc/ -j4 | grep status
Makefile:122: srctree=/labs/linux-lab/src/linux-stable
Makefile:123: objtree: out
Makefile:124: COMMAND_O: O=/labs/linux-lab/src/linux-stable/tools/testing/selftests/nolibc/out
Makefile:125: OUTPUT=/labs/linux-lab/src/linux-stable/tools/testing/selftests/nolibc/out/
166 test(s): 158 passed, 8 skipped, 0 failed => status: warning
// topdir
$ pwd
/labs/linux-lab/src/linux-stable
$ mkdir -p out
$ make run-user O=out -C /labs/linux-lab/src/linux-stable/tools/testing/selftests/nolibc/ -j4 | grep status
Makefile:122: srctree=/labs/linux-lab/src/linux-stable
Makefile:123: objtree: out
Makefile:124: COMMAND_O: O=/labs/linux-lab/src/linux-stable/out
Makefile:125: OUTPUT=/labs/linux-lab/src/linux-stable/out/
166 test(s): 158 passed, 8 skipped, 0 failed => status: warning
// outside
$ pwd
/labs/linux-lab/build
$ mkdir -p out
$ make run-user O=out -C /labs/linux-lab/src/linux-stable/tools/testing/selftests/nolibc/ -j4 | grep status
Makefile:122: srctree=/labs/linux-lab/src/linux-stable
Makefile:123: objtree: out
Makefile:124: COMMAND_O: O=/labs/linux-lab/build/out
Makefile:125: OUTPUT=/labs/linux-lab/build/out/
166 test(s): 158 passed, 8 skipped, 0 failed => status: warning
The change:
$ grep COMMAND_O -ur tools/testing/selftests/nolibc/Makefile
$(Q)$(MAKE) -C ../../../include/nolibc $(COMMAND_O) ARCH=$(ARCH) OUTPUT=$(OUTPUT)sysroot/ headers_standalone
$(Q)$(MAKE) -C $(srctree) $(COMMAND_O) ARCH=$(ARCH) CC=$(CC) CROSS_COMPILE=$(CROSS_COMPILE) mrproper $(DEFCONFIG) prepare
$(Q)$(MAKE) -C $(srctree) $(COMMAND_O) ARCH=$(ARCH) CC=$(CC) CROSS_COMPILE=$(CROSS_COMPILE) $(IMAGE_NAME) CONFIG_INITRAMFS_SOURCE=$(CURDIR)/initramfs
> > And even further, I thought of puting everything to $(OUTPUT), but the
> > change is very ugly and not good for v6.6, If you like, I will send that
> > patch only as a discuss stuff.
>
> Yeah I'd rather avoid ugly things for 6.6 now.
>
Ok, since you also suggested let's develop in outside directory, it is better
to reserve the left parts as-is ;-)
> Thanks,
> Willy