Re: [PATCH v4] selftest: size: Add size test for Linux kernel

On 11/26/2014 10:04 PM, Josh Triplett wrote:

On Wed, Nov 26, 2014 at 08:27:23PM -0800, Tim Bird wrote:

quoted

--- /dev/null
+++ b/tools/testing/selftests/size/Makefile

[...]

quoted

+LIBGCC=$(shell $(CC) -print-libgcc-file-name)
+
+get_size: get_size.c
+	$(CC) --static -ffreestanding -nostartfiles \
+		-Wl,--entry=_start get_size.c $(LIBGCC) \
+		-o get_size

You don't need -Wl,--entry=_start; that's the default.

OK - it works without this.  Thanks.

You shouldn't need to manually find libgcc, either; the compiler should
do that for you.  What goes wrong if you don't include that?  If you're
trying to link libgcc statically, try -static-libgcc.

Also, static is normally spelled -static, not --static.

Hmm.  Not sure where I got --static from, but it worked.
But if -static is the norm I'm fine changing to that.

Upon experimentation, I don't need the explicit libgcc or for that
matter the -static-libgcc either.

quoted

--- /dev/null
+++ b/tools/testing/selftests/size/get_size.c

[...]

quoted

+int print(const char *s)

This function, and all the others apart from _start, should be declared
static.

OK will do.

quoted

+void num_to_str(unsigned long num, char *s)

Likewise, static.

OK

quoted

+{
+	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;
+}

You'd probably end up with significantly smaller code (and no divisions,
and thus no corner cases on architectures that need a special function
to do unsigned long long division) if you print in hex.  You could also
drop the "no leading zeros" logic, and just *always* print a 64-bit
value as 16 hex digits.

I'd like to keep base-10 output.  As far as size is concerned, the
code is now at well under 2 pages (8k), which is much smaller than
any actually useful program.  The only noticeable change in size would
be if I got it under 4096, but I don't want to sacrifice too many
features to get there (as that's still only 1 page difference in
memory usage.)

BTW - using sstrip, I can get it to 4096 already, as is.

Unfortunately, 'sstrip -z', which gets it to 2061, makes
it not work.  I need to check out what the problem is there.
Also, the ELF file still has an unneeded note section.  I think
easier reductions, without sacrificing functionality, are available
by tweaking the ELF header (ie fixing sstrip, for those that
want to use it).
 

quoted

+int print_num(unsigned long num)

Likewise, static.

OK

quoted

+{
+	char num_buf[30];
+
+	num_to_str(num, num_buf);
+	return print(num_buf);
+}
+
+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;
+}

I'd suggest dropping this entirely, and just always printing the exact
values returned by sysinfo.  Drop the multiply, too, and just print
info.mem_unit as well.  It's easy to post-process the value in a more
capable environment.

I'd prefer to keep the output easily human-readable.
 

quoted

+/* 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 size runtime size\n");

Shouldn't the "not ok" here and the "ok" below have the same test
description?

Yes.  Thanks.  Good catch.
 

quoted

+		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

OK - look for a new version shortly (maybe not today, though).
 -- Tim