linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5] selftest: size: Add size test for Linux kernel
@ 2014-12-03  3:36 Tim Bird
  2014-12-03  3:43 ` Michael Ellerman
  0 siblings, 1 reply; 9+ messages in thread
From: Tim Bird @ 2014-12-03  3:36 UTC (permalink / raw)
  To: Shuah Khan, linux-api; +Cc: Josh Triplett, linux-kernel, linux-embedded


This test shows the amount of memory used by the system.
Note that this is dependent on the user-space that is loaded
when this program runs.  Optimally, this program would be
run as the init program itself.

The program is optimized for size itself, to avoid conflating
its own execution with that of the system software.
The code is compiled statically, with no stdlibs. On my x86_64 system,
this results in a statically linked binary of less than 5K.


Signed-off-by: Tim Bird <tim.bird@sonymobile.com>
---
Changes from v5:
  - make most routines static
  - replace strip with gcc -s
  - remove explicit reference to _start
  - change --static to -static
  - remove explicit reference to LIBGCC
  - fix test description for ok and not ok paths, for test 1

Changes from v4:
  - add more human-readable output
  - put libgcc reference into a variable in Makefile

Changes from v3:
  - fix copyright string (again!)
  - use __builtin_strlen instead of my own strlen
  - replace main with _start

Changes from v2:
  - add return values to print routines
  - add .gitignore file

Changes from v1:
  - use more correct Copyright string in get_size.c

 tools/testing/selftests/Makefile        |   1 +
 tools/testing/selftests/size/.gitignore |   1 +
 tools/testing/selftests/size/Makefile   |  15 +++++
 tools/testing/selftests/size/get_size.c | 111 ++++++++++++++++++++++++++++++++
 4 files changed, 128 insertions(+)
 create mode 100644 tools/testing/selftests/size/.gitignore
 create mode 100644 tools/testing/selftests/size/Makefile
 create mode 100644 tools/testing/selftests/size/get_size.c

diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
index 45f145c..fa91aef 100644
--- a/tools/testing/selftests/Makefile
+++ b/tools/testing/selftests/Makefile
@@ -15,6 +15,7 @@ TARGETS += user
 TARGETS += sysctl
 TARGETS += firmware
 TARGETS += ftrace
+TARGETS += size
 
 TARGETS_HOTPLUG = cpu-hotplug
 TARGETS_HOTPLUG += memory-hotplug
diff --git a/tools/testing/selftests/size/.gitignore b/tools/testing/selftests/size/.gitignore
new file mode 100644
index 0000000..189b781
--- /dev/null
+++ b/tools/testing/selftests/size/.gitignore
@@ -0,0 +1 @@
+get_size
diff --git a/tools/testing/selftests/size/Makefile b/tools/testing/selftests/size/Makefile
new file mode 100644
index 0000000..47f8e9c
--- /dev/null
+++ b/tools/testing/selftests/size/Makefile
@@ -0,0 +1,15 @@
+#ifndef CC
+	CC = $(CROSS_COMPILE)gcc
+#endif
+
+all: get_size
+
+get_size: get_size.c
+	$(CC) -static -ffreestanding -nostartfiles \
+		-s get_size.c -o get_size
+
+run_tests: all
+	./get_size
+
+clean:
+	$(RM) get_size
diff --git a/tools/testing/selftests/size/get_size.c b/tools/testing/selftests/size/get_size.c
new file mode 100644
index 0000000..808e7c6
--- /dev/null
+++ b/tools/testing/selftests/size/get_size.c
@@ -0,0 +1,111 @@
+/*
+ * Copyright 2014 Sony Mobile Communications Inc.
+ *
+ * Licensed under the terms of the GNU GPL License version 2
+ *
+ * Selftest for runtime system size
+ *
+ * Prints the amount of RAM that the currently running system is using.
+ *
+ * This program tries to be as small as possible itself, to
+ * avoid perturbing the system memory utilization with its
+ * own execution.  It also attempts to have as few dependencies
+ * on kernel features as possible.
+ *
+ * It should be statically linked, with startup libs avoided.
+ * It uses no library calls, and only the following 3 syscalls:
+ *   sysinfo(), write(), and _exit()
+ *
+ * For output, it avoids printf (which in some C libraries
+ * has large external dependencies) by  implementing it's own
+ * number output and print routines, and using __builtin_strlen()
+ */
+
+#include <sys/sysinfo.h>
+#include <unistd.h>
+
+#define STDOUT_FILENO 1
+
+static int print(const char *s)
+{
+	return write(STDOUT_FILENO, s, __builtin_strlen(s));
+}
+
+
+/*
+ * num_to_str - put digits from num into *s, left to right
+ *   do this by dividing the number by powers of 10
+ *   the tricky part is to omit leading zeros
+ *   don't print zeros until we've started printing any numbers at all
+ */
+static void num_to_str(unsigned long num, char *s)
+{
+	unsigned long long temp, div;
+	int started;
+
+	temp = num;
+	div = 1000000000000000000LL;
+	started = 0;
+	while (div) {
+		if (temp/div || started) {
+			*s++ = (unsigned char)(temp/div + '0');
+			started = 1;
+		}
+		temp -= (temp/div)*div;
+		div /= 10;
+	}
+	*s = 0;
+}
+
+static int print_num(unsigned long num)
+{
+	char num_buf[30];
+
+	num_to_str(num, num_buf);
+	return print(num_buf);
+}
+
+static int print_k_value(const char *s, unsigned long num, unsigned long units)
+{
+	unsigned long long temp;
+	int ccode;
+
+	print(s);
+
+	temp = num;
+	temp = (temp * units)/1024;
+	num = temp;
+	ccode = print_num(num);
+	print("\n");
+	return ccode;
+}
+
+/* this program has no main(), as startup libraries are not used */
+void _start(void)
+{
+	int ccode;
+	struct sysinfo info;
+	unsigned long used;
+
+	print("Testing system size.\n");
+	print("1..1\n");
+
+	ccode = sysinfo(&info);
+	if (ccode < 0) {
+		print("not ok 1 get runtime memory use\n");
+		print("# could not get sysinfo\n");
+		_exit(ccode);
+	}
+	/* ignore cache complexities for now */
+	used = info.totalram - info.freeram - info.bufferram;
+	print_k_value("ok 1 get runtime memory use # size = ", used,
+		info.mem_unit);
+
+	print("# System runtime memory report (units in Kilobytes):\n");
+	print_k_value("#   Total:  ", info.totalram, info.mem_unit);
+	print_k_value("#   Free:   ", info.freeram, info.mem_unit);
+	print_k_value("#   Buffer: ", info.bufferram, info.mem_unit);
+	print_k_value("#   In use: ", used, info.mem_unit);
+
+	_exit(0);
+}
-- 
1.8.2.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v5] selftest: size: Add size test for Linux kernel
  2014-12-03  3:36 [PATCH v5] selftest: size: Add size test for Linux kernel Tim Bird
@ 2014-12-03  3:43 ` Michael Ellerman
  2014-12-03 13:01   ` Thomas Petazzoni
  2014-12-03 16:29   ` Tim Bird
  0 siblings, 2 replies; 9+ messages in thread
From: Michael Ellerman @ 2014-12-03  3:43 UTC (permalink / raw)
  To: Tim Bird
  Cc: Shuah Khan, linux-api, Josh Triplett, linux-kernel, linux-embedded

On Tue, 2014-12-02 at 19:36 -0800, Tim Bird wrote:
> This test shows the amount of memory used by the system.
> Note that this is dependent on the user-space that is loaded
> when this program runs.  Optimally, this program would be
> run as the init program itself.

Sorry to only chime in at v5.

> diff --git a/tools/testing/selftests/size/Makefile b/tools/testing/selftests/size/Makefile
> new file mode 100644
> index 0000000..47f8e9c
> --- /dev/null
> +++ b/tools/testing/selftests/size/Makefile
> @@ -0,0 +1,15 @@
> +#ifndef CC
> +	CC = $(CROSS_COMPILE)gcc
> +#endif

I think the following is preferable:

  CC := $(CROSS_COMPILE)$(CC)


It allows optionally setting a custom CC, as well as optionally CROSS_COMPILE.

The only thing it doesn't do is choose gcc explicitly, but you shouldn't really
do that anyway - unless you absolutely require gcc. Let the user choose their
compiler by choosing where cc points.

cheers




^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v5] selftest: size: Add size test for Linux kernel
  2014-12-03  3:43 ` Michael Ellerman
@ 2014-12-03 13:01   ` Thomas Petazzoni
  2014-12-03 16:13     ` Tim Bird
  2014-12-03 16:29   ` Tim Bird
  1 sibling, 1 reply; 9+ messages in thread
From: Thomas Petazzoni @ 2014-12-03 13:01 UTC (permalink / raw)
  To: Michael Ellerman
  Cc: Tim Bird, Shuah Khan, linux-api, Josh Triplett, linux-kernel,
	linux-embedded

Michael, Tim,

On Wed, 03 Dec 2014 14:43:11 +1100, Michael Ellerman wrote:

> > diff --git a/tools/testing/selftests/size/Makefile b/tools/testing/selftests/size/Makefile
> > new file mode 100644
> > index 0000000..47f8e9c
> > --- /dev/null
> > +++ b/tools/testing/selftests/size/Makefile
> > @@ -0,0 +1,15 @@
> > +#ifndef CC
> > +	CC = $(CROSS_COMPILE)gcc
> > +#endif
> 
> I think the following is preferable:
> 
>   CC := $(CROSS_COMPILE)$(CC)

It is even more necessary that #ifndef and #endif don't exist in make.
They are just comments, and therefore, ignored. Seems like Tim does too
much C :-)

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v5] selftest: size: Add size test for Linux kernel
  2014-12-03 13:01   ` Thomas Petazzoni
@ 2014-12-03 16:13     ` Tim Bird
  0 siblings, 0 replies; 9+ messages in thread
From: Tim Bird @ 2014-12-03 16:13 UTC (permalink / raw)
  To: Thomas Petazzoni, Michael Ellerman
  Cc: Shuah Khan, linux-api, Josh Triplett, linux-kernel, linux-embedded



On 12/03/2014 05:01 AM, Thomas Petazzoni wrote:
> Michael, Tim,
> 
> On Wed, 03 Dec 2014 14:43:11 +1100, Michael Ellerman wrote:
> 
>>> diff --git a/tools/testing/selftests/size/Makefile b/tools/testing/selftests/size/Makefile
>>> new file mode 100644
>>> index 0000000..47f8e9c
>>> --- /dev/null
>>> +++ b/tools/testing/selftests/size/Makefile
>>> @@ -0,0 +1,15 @@
>>> +#ifndef CC
>>> +	CC = $(CROSS_COMPILE)gcc
>>> +#endif
>>
>> I think the following is preferable:
>>
>>   CC := $(CROSS_COMPILE)$(CC)
> 
> It is even more necessary that #ifndef and #endif don't exist in make.
> They are just comments, and therefore, ignored. Seems like Tim does too
> much C :-)

OK - that's hilarious.  Saying 'Oops!' would be too casual for my degree of
embarrassment. :-)

Makefiles do have similar constructs.  Those should have been
ifeq ($(CC),)
	...
endif

This obviously got through via a failiure in testing - which is somewhat ironic.

Look for a v6 soon.  (Geez, when is the merge window coming.  I thought this trivial
program would get in pretty easily, but no... that's never the way.  Of course
it helps if the submitter is not an idiot.)
 -- Tim

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v5] selftest: size: Add size test for Linux kernel
  2014-12-03  3:43 ` Michael Ellerman
  2014-12-03 13:01   ` Thomas Petazzoni
@ 2014-12-03 16:29   ` Tim Bird
  2014-12-03 18:00     ` Geert Uytterhoeven
  2014-12-04  0:08     ` Michael Ellerman
  1 sibling, 2 replies; 9+ messages in thread
From: Tim Bird @ 2014-12-03 16:29 UTC (permalink / raw)
  To: Michael Ellerman
  Cc: Shuah Khan, linux-api, Josh Triplett, linux-kernel, linux-embedded



On 12/02/2014 07:43 PM, Michael Ellerman wrote:
> On Tue, 2014-12-02 at 19:36 -0800, Tim Bird wrote:
>> This test shows the amount of memory used by the system.
>> Note that this is dependent on the user-space that is loaded
>> when this program runs.  Optimally, this program would be
>> run as the init program itself.
> 
> Sorry to only chime in at v5.
> 
>> diff --git a/tools/testing/selftests/size/Makefile b/tools/testing/selftests/size/Makefile
>> new file mode 100644
>> index 0000000..47f8e9c
>> --- /dev/null
>> +++ b/tools/testing/selftests/size/Makefile
>> @@ -0,0 +1,15 @@
>> +#ifndef CC
>> +	CC = $(CROSS_COMPILE)gcc
>> +#endif
> 
> I think the following is preferable:
> 
>   CC := $(CROSS_COMPILE)$(CC)
> 
> 
> It allows optionally setting a custom CC, as well as optionally CROSS_COMPILE.

I'm not sure I follow this.

If CC is unset, you get only the CROSS_COMPILE prefix.
If CC is set to e.g. 'gcc', then you get a nicely formatted toolchain string.
But if CC already has the prefix applied, then this will result in
  having it duplicated, which surely won't work correctly.

In the long run, I would hope that a higher level Makefile or environment setting
will be setting the toolchain string appropriately (as well as handling build flags)
which is why I wanted to use an ifndef (which Thomas correctly pointed out is just
wrong).

Actually, after getting this tiny program accepted, my next task was working on a
proper fix for handling cross compilation in a more generic (not case-by-case) way.

CROSS_COMPILE prefix usage looks a bit uncoordinated in the tools directory, but most
tests seem to be favoring $(CROSS_COMPILE)gcc.

 $ cd tools ; mgrep CROSS
./vm/Makefile:CC = $(CROSS_COMPILE)gcc
./usb/Makefile:CC = $(CROSS_COMPILE)gcc
./testing/selftests/net/Makefile:CC = $(CROSS_COMPILE)gcc
./testing/selftests/vm/Makefile:CC = $(CROSS_COMPILE)gcc
./testing/selftests/efivarfs/Makefile:CC = $(CROSS_COMPILE)gcc
./testing/selftests/size/Makefile:	CC = $(CROSS_COMPILE)gcc
./testing/selftests/powerpc/Makefile:CC := $(CROSS_COMPILE)$(CC)
./hv/Makefile:CC = $(CROSS_COMPILE)gcc
./perf/config/feature-checks/Makefile:CC := $(CROSS_COMPILE)gcc -MD
./perf/config/feature-checks/Makefile:PKG_CONFIG := $(CROSS_COMPILE)pkg-config
./lib/api/Makefile:CC = $(CROSS_COMPILE)gcc
./lib/api/Makefile:AR = $(CROSS_COMPILE)ar
./lib/lockdep/Makefile:# Allow setting CC and AR, or setting CROSS_COMPILE as a prefix.
./lib/lockdep/Makefile:$(call allow-override,CC,$(CROSS_COMPILE)gcc)
./lib/lockdep/Makefile:$(call allow-override,AR,$(CROSS_COMPILE)ar)
./lib/lockdep/Makefile:TRACK_CFLAGS = $(subst ','\'',$(CFLAGS)):$(ARCH):$(CROSS_COMPILE)
./lib/traceevent/Makefile:# Allow setting CC and AR, or setting CROSS_COMPILE as a prefix.
./lib/traceevent/Makefile:$(call allow-override,CC,$(CROSS_COMPILE)gcc)
./lib/traceevent/Makefile:$(call allow-override,AR,$(CROSS_COMPILE)ar)
./lib/traceevent/Makefile:TRACK_CFLAGS = $(subst ','\'',$(CFLAGS)):$(ARCH):$(CROSS_COMPILE)
./cgroup/Makefile:CC = $(CROSS_COMPILE)gcc
./power/acpi/Makefile:CROSS = #/usr/i386-linux-uclibc/usr/bin/i386-uclibc-
./power/acpi/Makefile:CC = $(CROSS)gcc
./power/acpi/Makefile:LD = $(CROSS)gcc
./power/acpi/Makefile:STRIP = $(CROSS)strip
./power/cpupower/Makefile:CROSS = #/usr/i386-linux-uclibc/usr/bin/i386-uclibc-
./power/cpupower/Makefile:CC = $(CROSS)gcc
./power/cpupower/Makefile:LD = $(CROSS)gcc
./power/cpupower/Makefile:AR = $(CROSS)ar
./power/cpupower/Makefile:STRIP = $(CROSS)strip
./power/cpupower/Makefile:RANLIB = $(CROSS)ranlib
./power/cpupower/Makefile:export CROSS CC AR STRIP RANLIB CFLAGS LDFLAGS LIB_OBJS
./power/x86/turbostat/Makefile:CC		= $(CROSS_COMPILE)gcc

I agree it's desirable not to hardcode gcc, but we seem to be doing it all over
the place already.
 -- Tim


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v5] selftest: size: Add size test for Linux kernel
  2014-12-03 16:29   ` Tim Bird
@ 2014-12-03 18:00     ` Geert Uytterhoeven
  2014-12-03 18:25       ` Tim Bird
  2014-12-04  0:08     ` Michael Ellerman
  1 sibling, 1 reply; 9+ messages in thread
From: Geert Uytterhoeven @ 2014-12-03 18:00 UTC (permalink / raw)
  To: Tim Bird
  Cc: Michael Ellerman, Shuah Khan, linux-api, Josh Triplett,
	linux-kernel, linux-embedded

On Wed, Dec 3, 2014 at 5:29 PM, Tim Bird <tim.bird@sonymobile.com> wrote:
>>> diff --git a/tools/testing/selftests/size/Makefile b/tools/testing/selftests/size/Makefile
>>> new file mode 100644
>>> index 0000000..47f8e9c
>>> --- /dev/null
>>> +++ b/tools/testing/selftests/size/Makefile
>>> @@ -0,0 +1,15 @@
>>> +#ifndef CC
>>> +    CC = $(CROSS_COMPILE)gcc
>>> +#endif
>>
>> I think the following is preferable:
>>
>>   CC := $(CROSS_COMPILE)$(CC)
>>
>>
>> It allows optionally setting a custom CC, as well as optionally CROSS_COMPILE.
>
> I'm not sure I follow this.
>
> If CC is unset, you get only the CROSS_COMPILE prefix.
> If CC is set to e.g. 'gcc', then you get a nicely formatted toolchain string.
> But if CC already has the prefix applied, then this will result in
>   having it duplicated, which surely won't work correctly.
>
> In the long run, I would hope that a higher level Makefile or environment setting
> will be setting the toolchain string appropriately (as well as handling build flags)
> which is why I wanted to use an ifndef (which Thomas correctly pointed out is just
> wrong).
>
> Actually, after getting this tiny program accepted, my next task was working on a
> proper fix for handling cross compilation in a more generic (not case-by-case) way.
>
> CROSS_COMPILE prefix usage looks a bit uncoordinated in the tools directory, but most
> tests seem to be favoring $(CROSS_COMPILE)gcc.
>
>  $ cd tools ; mgrep CROSS

[...]

> I agree it's desirable not to hardcode gcc, but we seem to be doing it all over
> the place already.

Seems like it's time to start integrating the tests with the regular Kbuild
system, which handles cross-compilation fine...

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v5] selftest: size: Add size test for Linux kernel
  2014-12-03 18:00     ` Geert Uytterhoeven
@ 2014-12-03 18:25       ` Tim Bird
  0 siblings, 0 replies; 9+ messages in thread
From: Tim Bird @ 2014-12-03 18:25 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Michael Ellerman, Shuah Khan, linux-api, Josh Triplett,
	linux-kernel, linux-embedded, Thomas Petazzoni



On 12/03/2014 10:00 AM, Geert Uytterhoeven wrote:
> On Wed, Dec 3, 2014 at 5:29 PM, Tim Bird <tim.bird@sonymobile.com> wrote:
>>>> diff --git a/tools/testing/selftests/size/Makefile b/tools/testing/selftests/size/Makefile
>>>> new file mode 100644
>>>> index 0000000..47f8e9c
>>>> --- /dev/null
>>>> +++ b/tools/testing/selftests/size/Makefile
>>>> @@ -0,0 +1,15 @@
>>>> +#ifndef CC
>>>> +    CC = $(CROSS_COMPILE)gcc
>>>> +#endif
>>>
>>> I think the following is preferable:
>>>
>>>   CC := $(CROSS_COMPILE)$(CC)
>>>
>>>
>>> It allows optionally setting a custom CC, as well as optionally CROSS_COMPILE.
>>
>> I'm not sure I follow this.
>>
>> If CC is unset, you get only the CROSS_COMPILE prefix.
>> If CC is set to e.g. 'gcc', then you get a nicely formatted toolchain string.
>> But if CC already has the prefix applied, then this will result in
>>   having it duplicated, which surely won't work correctly.
>>
>> In the long run, I would hope that a higher level Makefile or environment setting
>> will be setting the toolchain string appropriately (as well as handling build flags)
>> which is why I wanted to use an ifndef (which Thomas correctly pointed out is just
>> wrong).
>>
>> Actually, after getting this tiny program accepted, my next task was working on a
>> proper fix for handling cross compilation in a more generic (not case-by-case) way.
>>
>> CROSS_COMPILE prefix usage looks a bit uncoordinated in the tools directory, but most
>> tests seem to be favoring $(CROSS_COMPILE)gcc.
>>
>>  $ cd tools ; mgrep CROSS
> 
> [...]
> 
>> I agree it's desirable not to hardcode gcc, but we seem to be doing it all over
>> the place already.
> 
> Seems like it's time to start integrating the tests with the regular Kbuild
> system, which handles cross-compilation fine...

Where possible, yes.  It would be nice to leverage CROSS_COMPILE and CFLAGS, or
a portion thereof, from the Kbuild system (as well a KBUILD_OUTPUT and friends).
It's on my to-do list, after getting my little C program accepted...
 -- Tim

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v5] selftest: size: Add size test for Linux kernel
  2014-12-03 16:29   ` Tim Bird
  2014-12-03 18:00     ` Geert Uytterhoeven
@ 2014-12-04  0:08     ` Michael Ellerman
  2014-12-04 16:39       ` Tim Bird
  1 sibling, 1 reply; 9+ messages in thread
From: Michael Ellerman @ 2014-12-04  0:08 UTC (permalink / raw)
  To: Tim Bird
  Cc: Shuah Khan, linux-api, Josh Triplett, linux-kernel, linux-embedded

On Wed, 2014-12-03 at 08:29 -0800, Tim Bird wrote:
> 
> On 12/02/2014 07:43 PM, Michael Ellerman wrote:
> > On Tue, 2014-12-02 at 19:36 -0800, Tim Bird wrote:
> >> This test shows the amount of memory used by the system.
> >> Note that this is dependent on the user-space that is loaded
> >> when this program runs.  Optimally, this program would be
> >> run as the init program itself.
> > 
> > Sorry to only chime in at v5.
> > 
> >> diff --git a/tools/testing/selftests/size/Makefile b/tools/testing/selftests/size/Makefile
> >> new file mode 100644
> >> index 0000000..47f8e9c
> >> --- /dev/null
> >> +++ b/tools/testing/selftests/size/Makefile
> >> @@ -0,0 +1,15 @@
> >> +#ifndef CC
> >> +	CC = $(CROSS_COMPILE)gcc
> >> +#endif
> > 
> > I think the following is preferable:
> > 
> >   CC := $(CROSS_COMPILE)$(CC)
> > 
> > 
> > It allows optionally setting a custom CC, as well as optionally CROSS_COMPILE.
> 
> I'm not sure I follow this.
> 
> If CC is unset, you get only the CROSS_COMPILE prefix.

CC is never unset. The default value is 'cc'.

> If CC is set to e.g. 'gcc', then you get a nicely formatted toolchain string.

Right.

> But if CC already has the prefix applied, then this will result in
>   having it duplicated, which surely won't work correctly.

That's just PEBKAC. Don't specify CROSS_COMPILE and also a fully specified CC.
Try it with the kernel Makefile and see how well it works.

> CROSS_COMPILE prefix usage looks a bit uncoordinated in the tools directory, but most
> tests seem to be favoring $(CROSS_COMPILE)gcc.

That doesn't make it right :)

> 
>  $ cd tools ; mgrep CROSS
...
> ./testing/selftests/powerpc/Makefile:CC := $(CROSS_COMPILE)$(CC)

You can run git blame on that one if you like ;)

> I agree it's desirable not to hardcode gcc, but we seem to be doing it all over
> the place already.

If everyone jumped off a bridge ... :)

cheers




^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v5] selftest: size: Add size test for Linux kernel
  2014-12-04  0:08     ` Michael Ellerman
@ 2014-12-04 16:39       ` Tim Bird
  0 siblings, 0 replies; 9+ messages in thread
From: Tim Bird @ 2014-12-04 16:39 UTC (permalink / raw)
  To: Michael Ellerman
  Cc: Shuah Khan, linux-api, Josh Triplett, linux-kernel,
	linux-embedded, Geert Uytterhoeven

On 12/03/2014 04:08 PM, Michael Ellerman wrote:
> On Wed, 2014-12-03 at 08:29 -0800, Tim Bird wrote:
>>
>> On 12/02/2014 07:43 PM, Michael Ellerman wrote:
>>> On Tue, 2014-12-02 at 19:36 -0800, Tim Bird wrote:
>>>> This test shows the amount of memory used by the system.
>>>> Note that this is dependent on the user-space that is loaded
>>>> when this program runs.  Optimally, this program would be
>>>> run as the init program itself.
>>>
>>> Sorry to only chime in at v5.
>>>
>>>> diff --git a/tools/testing/selftests/size/Makefile b/tools/testing/selftests/size/Makefile
>>>> new file mode 100644
>>>> index 0000000..47f8e9c
>>>> --- /dev/null
>>>> +++ b/tools/testing/selftests/size/Makefile
>>>> @@ -0,0 +1,15 @@
>>>> +#ifndef CC
>>>> +	CC = $(CROSS_COMPILE)gcc
>>>> +#endif
>>>
>>> I think the following is preferable:
>>>
>>>   CC := $(CROSS_COMPILE)$(CC)
>>>
>>>
>>> It allows optionally setting a custom CC, as well as optionally CROSS_COMPILE.
>>
>> I'm not sure I follow this.
>>
>> If CC is unset, you get only the CROSS_COMPILE prefix.
> 
> CC is never unset. The default value is 'cc'.

Yeah - I found that out by experimentation yesterday.  So my whole
idea for configuring it based on an unset CC was misguided.

> 
>> If CC is set to e.g. 'gcc', then you get a nicely formatted toolchain string.
> 
> Right.
> 
>> But if CC already has the prefix applied, then this will result in
>>   having it duplicated, which surely won't work correctly.
> 
> That's just PEBKAC. Don't specify CROSS_COMPILE and also a fully specified CC.
> Try it with the kernel Makefile and see how well it works.
> 
>> CROSS_COMPILE prefix usage looks a bit uncoordinated in the tools directory, but most
>> tests seem to be favoring $(CROSS_COMPILE)gcc.
> 
> That doesn't make it right :)

Agreed. I'd like to separate this issue from the rest of the patch.
Shuah has accepted the patch as it currently is, but I'd like
to start working on the best way to support cross compilation
throughout the kselftest suite.

Ideally we can have CC set properly in a higher-level Makefile,
and not reference CROSS_COMPILE at all in the sub-directory Makefiles.
This may involve copying how CROSS_COMPILE and CFLAGS work in
the rest of the kernel tree, or making some kselftest-specific
modifications.

 -- Tim

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2014-12-04 16:39 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-12-03  3:36 [PATCH v5] selftest: size: Add size test for Linux kernel Tim Bird
2014-12-03  3:43 ` Michael Ellerman
2014-12-03 13:01   ` Thomas Petazzoni
2014-12-03 16:13     ` Tim Bird
2014-12-03 16:29   ` Tim Bird
2014-12-03 18:00     ` Geert Uytterhoeven
2014-12-03 18:25       ` Tim Bird
2014-12-04  0:08     ` Michael Ellerman
2014-12-04 16:39       ` Tim Bird

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).