replace string_get_size() arrays

The output of /proc/$pid/numa_maps is in terms of number of pages like anon=22 or dirty=54. Here's some output:

7f4680000000 default file=/hugetlb/bigfile anon=50 dirty=50 N0=50 7f7659600000 default file=/anon_hugepage\040(deleted) anon=50 dirty=50 N0=50 7fff8d425000 default stack anon=50 dirty=50 N0=50

Looks like we have a stack and a couple of anonymous hugetlbfs areas page which both use the same amount of memory. They don't.

The 'bigfile' uses 1GB pages and takes up ~50GB of space. The anon_hugepage uses 2MB pages and takes up ~100MB of space while the stack uses normal 4k pages. You can go over to smaps to figure out what the page size _really_ is with KernelPageSize or MMUPageSize. But, I think this is a pretty nasty and counterintuitive interface as it stands.

The following patch adds a pagesize= field. Note that this only shows the kernel's notion of page size. For transparent hugepages, it still shows the base page size. Here's some real output. Note the anon_hugepage in there.

# cat /proc/`pidof memknobs`/numa_maps
00400000 default file=/root/memknobs pagesize=4KiB dirty=3 active=2 N0=3
00602000 default file=/root/memknobs pagesize=4KiB anon=1 dirty=1 N0=1
00603000 default file=/root/memknobs pagesize=4KiB anon=1 dirty=1 N0=1
00604000 default heap pagesize=4KiB anon=6 dirty=6 N0=6
7f6766216000 default file=/lib/libc-2.9.so pagesize=4KiB mapped=98 mapmax=25 active=97 N0=98
7f676637e000 default file=/lib/libc-2.9.so
7f676657e000 default file=/lib/libc-2.9.so pagesize=4KiB anon=4 dirty=4 N0=4
7f6766582000 default file=/lib/libc-2.9.so pagesize=4KiB anon=1 dirty=1 N0=1
7f6766583000 default pagesize=4KiB anon=3 dirty=3 N0=3

7f6766588000 default file=/lib/ld-2.9.so pagesize=4KiB mapped=25 mapmax=24 N0=25
7f676679d000 default pagesize=4KiB anon=2 dirty=2 N0=2
7f67667a3000 default pagesize=4KiB anon=4 dirty=4 N0=4
7f67667a7000 default file=/lib/ld-2.9.so pagesize=4KiB anon=1 dirty=1 N0=1
7f67667a8000 default file=/lib/ld-2.9.so pagesize=4KiB anon=1 dirty=1 N0=1
7f6766800000 default file=/anon_hugepage\040(deleted) pagesize=2MiB anon=10 dirty=10 N0=10
7fff5b948000 default stack pagesize=4KiB anon=2 dirty=2 N0=2
7fff5b96d000 default

Signed-off-by Dave Haneen <[email protected]>

---

 linux-2.6.git-dave/fs/proc/task_mmu.c |    5 +++++
 1 file changed, 5 insertions(+)

diff -puN fs/proc/task_mmu.c~show-page-size fs/proc/task_mmu.c

fs/proc/task_mmu.c renamed from fs/proc/task_mmu.c~show-page-size +3 -0

+	seq_puts(m, " pagesize=");
+	seq_print_pow2(m, vma_kernel_pagesize(vma));
+
 	if (md->anon)
 		seq_printf(m, " anon=%lu", md->anon);
 

This would be great if all the /proc/pid/numa_maps consumers were human, but unfortuantely that's not the case.

I understand that this patchset was probably the result of me asking for the pagesize= to be specified in each line and using pagesize=4K and pagesize=2M as examples, but that exact usage is probably not what we want.

As long as there are scripts that go through and read this information (we have some internally), expressing them with differing units just makes it more difficult to parse. I'd rather them just be the byte count.

That way, 1G pages would just show pagesize=1073741824. I don't think that's too long and is much easier to parse systematically.

Quoted 12 lines

This would be great if all the /proc/pid/numa_maps consumers were human, but unfortuantely that's not the case.

I understand that this patchset was probably the result of me asking for the pagesize= to be specified in each line and using pagesize=4K and pagesize=2M as examples, but that exact usage is probably not what we want.

As long as there are scripts that go through and read this information (we have some internally), expressing them with differing units just makes it more difficult to parse. I'd rather them just be the byte count.

Quoted 3 lines

That way, 1G pages would just show pagesize=1073741824. I don't think that's too long and is much easier to parse systematically.

Hmm... Thats sounds strange.

Are you saying you cant change your scripts [But you'll have to anyway to parse pagesize=] ?

I routinely use "cat /proc/xxx/numa_maps", and am stuck when a kernel displays nothing (it happened on some debian released kernels)

Seeing pagesize=1GB is slightly better for human, and not that hard to parse for a program.

By the way, "pagesize=4KiB" are just noise if you ask me, thats the default PAGE_SIZE. This also breaks old scripts :)

Why on earth do we want to convert a byte value into a string so a script can convert it the other way around? Do you have a hard time parsing 4096, 2097152, and 1073741824 to be 4K, 2M, and 1G respectively?

Quoted 3 lines

Why on earth do we want to convert a byte value into a string so a script can convert it the other way around? Do you have a hard time parsing 4096, 2097152, and 1073741824 to be 4K, 2M, and 1G respectively?

Yes I do. I dont have in my head all possible 2^X values, but K, M, G, T : thats ok (less neurons needed)

You focus on current x86_64 hardware.

Some arches have lot of different choices. (powerpc has 64K, 16M, 16GB pages)

In 10 years, you'll have pagesize=549755813888, or maybe pagesize=8589934592

I pretty much prefer pagesize=512GB and pagesize=8TB

This is consistent with usual conventions and practice.

By the way, "pagesize=4KiB" are just noise if you ask me, thats the default PAGE_SIZE. This also breaks old scripts :)

Quoted 2 lines · Eric Dumazet

By the way, "pagesize=4KiB" are just noise if you ask me, thats the default PAGE_SIZE. This also breaks old scripts :)

How does it break old scripts?

Quoted 2 lines

How does it break old scripts?

Old scripts just parse numa_maps, and on typical machines where hugepages are not used, they dont have to care about page size. They assume pages are 4KB.

Adding a new word (pagesize=...) might break them, but personally I dont care.

That way, 1G pages would just show pagesize=1073741824. I don't think that's too long and is much easier to parse systematically.

Instead of explicitly storing the entire string for each possible units, just store the thing that varies: the first character.

Instead of explicitly storing the entire string for each possible units, just store the thing that varies: the first character.

Quoted 3 lines · Dave Hansen

Instead of explicitly storing the entire string for each possible units, just store the thing that varies: the first character.

trivia

Quoted 7 lines

diff -puN lib/string_helpers.c~string_get_size-pow2 lib/string_helpers.c

lib/string_helpers.c renamed from lib/string_helpers.c~string_get_size-pow2 +0 -0

@@ -8,6 +8,23 @@
 #include <linux/module.h>
 #include <linux/string_helpers.h>
 

lib/string_helpers.c line 11

Quoted 1 line

lib/string_helpers.c renamed from lib/string_helpers.c~string_get_size-pow2 +1 -0

+static const char byte_units[] = "_KMGTPEZY";

u64 could be up to ~1.8**19 decimal zetta and yotta are not possible or necessary. u128 maybe someday, but then other changes would be necessary too.

lib/string_helpers.c line 19

Quoted 7 lines

lib/string_helpers.c renamed from lib/string_helpers.c~string_get_size-pow2 +7 -0

+static char *__units_str(enum string_size_units unit, char *buf, int index)
+{
+	int place = 0;
+
+	/* index=0 is plain 'B' with no other unit */
+	if (index) {
+		buf[place++] = byte_units[index];

index is unbounded (doesn't matter currently, it will for u128)

lib/string_helpers.c line 41

Quoted 3 lines

lib/string_helpers.c renamed from lib/string_helpers.c~string_get_size-pow2 +0 -0

@@ -23,26 +40,19 @@
 int string_get_size(u64 size, const enum string_size_units units,
 		    char *buf, int len)

[]

lib/string_helpers.c line 46

Quoted 4 lines

lib/string_helpers.c renamed from lib/string_helpers.c~string_get_size-pow2 +0 -0

 	const unsigned int divisor[] = {
 		[STRING_UNITS_10] = 1000,
 		[STRING_UNITS_2] = 1024,
 	};

static const or it might be better to use

	unsigned int divisor = (string_size_units == STRING_UNITS_2) ? 1024 : 1000;

as that would make the code clearer in a couple of uses of divisor[] later.

lib/string_helpers.c int string_get_size(u64 size, const enum · line 73

Quoted 4 lines

lib/string_helpers.c renamed from lib/string_helpers.c~string_get_size-pow2 +0 -0

@@ -61,7 +71,7 @@ int string_get_size(u64 size, const enum
 	}
 
 	snprintf(buf, len, "%lld%s %s", (unsigned long long)size,

%llu

Quoted 5 lines · Joe Perches

Quoted 3 lines · Dave Hansen

Instead of explicitly storing the entire string for each possible units, just store the thing that varies: the first character.

trivia

I'm not sure what you mean by that.

Quoted 7 lines

Quoted 7 lines

diff -puN lib/string_helpers.c~string_get_size-pow2 lib/string_helpers.c

lib/string_helpers.c renamed from lib/string_helpers.c~string_get_size-pow2 +0 -0

@@ -8,6 +8,23 @@
 #include <linux/module.h>
 #include <linux/string_helpers.h>
 

Quoted 6 lines

lib/string_helpers.c line 11

Quoted 1 line

lib/string_helpers.c renamed from lib/string_helpers.c~string_get_size-pow2 +1 -0

+static const char byte_units[] = "_KMGTPEZY";

u64 could be up to ~1.8**19 decimal zetta and yotta are not possible or necessary. u128 maybe someday, but then other changes would be necessary too.

Right, but we're only handling u64.

Quoted 1 line

Right, but we're only handling u64.

So the declaration should be:

        static const char byte_units[] = " KMGTPE";

Quoted 3 lines · Joe Perches

Quoted 1 line

Right, but we're only handling u64.

So the declaration should be:

        static const char byte_units[] = " KMGTPE";

I guess that's worth a comment. But that first character doesn't get used. There were two alternatives:

	static const char byte_units[] = "_KMGTPE";

or something along the lines of:

(quoted diff) +4 -0

+	static const char byte_units[] = "KMGTPE";
...
+	index--;
+       /* index=-1 is plain 'B' with no other unit */
+       if (index >= 0) {

We don't ever _actually_ look at the space (or underscore). I figured the _ was nicer since it would be _obvious_ if it ever got printed out somehow.

Instead of explicitly storing the entire string for each possible units, just store the thing that varies: the first character.

We have to special-case the 'B' unit (index==0).

This shaves about 100 bytes off of my .o file.

Signed-off-by Dave Hansen <[email protected]>

---

 linux-2.6.git-dave/lib/string_helpers.c |   30 ++++++++++++++++++++----------
 1 file changed, 20 insertions(+), 10 deletions(-)

diff -puN lib/string_helpers.c~string_get_size-pow2 lib/string_helpers.c

lib/string_helpers.c renamed from lib/string_helpers.c~string_get_size-pow2 +20 -10

@@ -8,6 +8,23 @@
 #include <linux/module.h>
 #include <linux/string_helpers.h>
 
+static const char byte_units[] = "_KMGTPEZY";
+
+static char *__units_str(enum string_size_units unit, char *buf, int index)
+{
+	int place = 0;
+
+	/* index=0 is plain 'B' with no other unit */
+	if (index) {
+		buf[place++] = byte_units[index];
+		if (unit == STRING_UNITS_2)
+			buf[place++] = 'i';
+	}
+	buf[place++] = 'B';
+	buf[place++] = '\0';
+	return buf;
+}
+
 /**
  * string_get_size - get the size in the specified units
  * @size:	The size to be converted

@@ -23,26 +40,19 @@
 int string_get_size(u64 size, const enum string_size_units units,
 		    char *buf, int len)
 {
-	const char *units_10[] = { "B", "kB", "MB", "GB", "TB", "PB",
-				   "EB", "ZB", "YB", NULL};
-	const char *units_2[] = {"B", "KiB", "MiB", "GiB", "TiB", "PiB",
-				 "EiB", "ZiB", "YiB", NULL };
-	const char **units_str[] = {
-		[STRING_UNITS_10] =  units_10,
-		[STRING_UNITS_2] = units_2,
-	};
 	const unsigned int divisor[] = {
 		[STRING_UNITS_10] = 1000,
 		[STRING_UNITS_2] = 1024,
 	};
 	int i, j;
 	u64 remainder = 0, sf_cap;
+	char unit_buf[4];
 	char tmp[8];
 
 	tmp[0] = '\0';
 	i = 0;
 	if (size >= divisor[units]) {
-		while (size >= divisor[units] && units_str[units][i]) {
+		while (size >= divisor[units] && (i < strlen(byte_units))) {
 			remainder = do_div(size, divisor[units]);
 			i++;
 		}

@@ -61,7 +71,7 @@ int string_get_size(u64 size, const enum
 	}
 
 	snprintf(buf, len, "%lld%s %s", (unsigned long long)size,
-		 tmp, units_str[units][i]);
+		 tmp, __units_str(units, unit_buf, i));
 
 	return 0;
 }