Re: [PATCH v5] selftest: size: Add size test for Linux kernel
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 000..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 -- To unsubscribe from this list: send the line "unsubscribe linux-embedded" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5] selftest: size: Add size test for Linux kernel
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 000..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 -- To unsubscribe from this list: send the line "unsubscribe linux-embedded" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5] selftest: size: Add size test for Linux kernel
On 12/03/2014 10:00 AM, Geert Uytterhoeven wrote: > On Wed, Dec 3, 2014 at 5:29 PM, Tim Bird wrote: diff --git a/tools/testing/selftests/size/Makefile b/tools/testing/selftests/size/Makefile new file mode 100644 index 000..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 -- To unsubscribe from this list: send the line "unsubscribe linux-embedded" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5] selftest: size: Add size test for Linux kernel
On Wed, Dec 3, 2014 at 5:29 PM, Tim Bird wrote:
>>> diff --git a/tools/testing/selftests/size/Makefile
>>> b/tools/testing/selftests/size/Makefile
>>> new file mode 100644
>>> index 000..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 -- ge...@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
--
To unsubscribe from this list: send the line "unsubscribe linux-embedded" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5] selftest: size: Add size test for Linux kernel
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 000..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 -- To unsubscribe from this list: send the line "unsubscribe linux-embedded" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5] selftest: size: Add size test for Linux kernel
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 000..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 -- To unsubscribe from this list: send the line "unsubscribe linux-embedded" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5] selftest: size: Add size test for Linux kernel
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 000..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 -- To unsubscribe from this list: send the line "unsubscribe linux-embedded" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5] selftest: size: Add size test for Linux kernel
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 000..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 -- To unsubscribe from this list: send the line "unsubscribe linux-embedded" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
