Skip to contentSkip to conversation

break up string_get_size()

RFC PATCH In review Dave Hansen 2 participants 5 messages Sep 30, 2011

Subsystems (6)
lore
Share & tools
lore
#0 lore raw

RFC PATCH 1/4 break up string_get_size()

I started to break up string_get_size() in to some pieces so that I could pick and choose which bits I wanted. I ended up not re-using this function. But, I think this still stands by itself since it makes the code much more self-explanatory.

---

 linux-2.6.git-dave/lib/string_helpers.c |   19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)
lib/string_helpers.c renamed from lib/string_helpers.c~break-up-string_get_size +14 -5
diff -puN lib/string_helpers.c~break-up-string_get_size lib/string_helpers.c
@@ -8,6 +8,18 @@
 #include <linux/module.h>
 #include <linux/string_helpers.h>
 
+static int calc_sf_digit_room(u64 size)
+{
+	u64 sf_cap;
+	int digits;
+
+	sf_cap = size;
+	for (digits = 0; sf_cap*10 < 1000; digits++)
+		sf_cap *= 10;
+
+	return digits;
+}
+
 /**
  * string_get_size - get the size in the specified units
  * @size:	The size to be converted
@@ -36,7 +48,7 @@ int string_get_size(u64 size, const enum
 		[STRING_UNITS_2] = 1024,
 	};
 	int i, j;
-	u64 remainder = 0, sf_cap;
+	u64 remainder = 0;
 	char tmp[8];
 
 	tmp[0] = '\0';
@@ -47,10 +59,7 @@ int string_get_size(u64 size, const enum
 			i++;
 		}
 
-		sf_cap = size;
-		for (j = 0; sf_cap*10 < 1000; j++)
-			sf_cap *= 10;
-
+		j = calc_sf_digit_room(size);
 		if (j) {
 			remainder *= 1000;
 			do_div(remainder, divisor[units]);

_

#1 lore raw

RFC PATCH 2/4 break out unit selection from string_get_size()

string_get_size() can really only print things in a single format. You're always stuck with a space, and it will always zero-pad the decimal places:

	4.00 KiB
	40.0 KiB
	400 KiB

Printing page sizes in decimal KiB does not make much sense since they are always nice powers of two. But, string_get_size() does have some nice code for selecting the right units and doing the division.

This breaks that nice code out so that we can reuse it. find_size_units() is a bit of a funky function since it has so many outputs. I don't think it's _too_ crazy though.

---

 linux-2.6.git-dave/include/linux/string_helpers.h |    3 
 linux-2.6.git-dave/lib/string_helpers.c           |   67 +++++++++++++++-------
 2 files changed, 50 insertions(+), 20 deletions(-)
include/linux/string_helpers.h renamed from include/linux/string_helpers.h~break-up-string_get_size-1 +3 -0
diff -puN include/linux/string_helpers.h~break-up-string_get_size-1 include/linux/string_helpers.h
@@ -10,6 +10,9 @@ enum string_size_units {
 	STRING_UNITS_2,		/* use binary powers of 2^10 */
 };
 
+u64 find_size_units(u64 *size, const enum string_size_units units,
+		const char **unit_str);
+
 int string_get_size(u64 size, enum string_size_units units,
 		    char *buf, int len);
 
lib/string_helpers.c renamed from lib/string_helpers.c~break-up-string_get_size-1 +47 -20

90 lines not on this page · raw message

_

#2 lore raw

Re: [RFC][PATCH 2/4] break out unit selection from string_get_size()

Quoted 13 lines · Dave Hansen

string_get_size() can really only print things in a single format. You're always stuck with a space, and it will always zero-pad the decimal places:

	4.00 KiB
	40.0 KiB
	400 KiB

Printing page sizes in decimal KiB does not make much sense since they are always nice powers of two. But, string_get_size() does have some nice code for selecting the right units and doing the division.

Quoted 5 lines

This breaks that nice code out so that we can reuse it. find_size_units() is a bit of a funky function since it has so many outputs. I don't think it's _too_ crazy though.

For powers of two, wouldn't it make a lot more sense to just do ilog2() to get the power of two and then the moral equivalent of:

	sprintf("%u %s", 1 << (pw2 % 10), units_str[pw2 / 10]);

	-hpa

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

#3 lore raw

RFC PATCH 3/4 add seq_print_size() function

In reply to Dave Hansen

In order to get nice, human-readable output, we are going to use MiB/KiB, etc... in numa_maps. Introduce a helper to do the conversion from a raw integer over to a string.

I thought about doing this as a new printk() format specifier. That would be interesting, but it's hard to argue with this since it's so short and sweet.

---

 linux-2.6.git-dave/fs/seq_file.c            |   15 +++++++++++++++
 linux-2.6.git-dave/include/linux/seq_file.h |    3 +++
 2 files changed, 18 insertions(+)
fs/seq_file.c renamed from fs/seq_file.c~add-seq_print_size +15 -0
diff -puN fs/seq_file.c~add-seq_print_size fs/seq_file.c
@@ -386,6 +386,21 @@ int seq_printf(struct seq_file *m, const
 }
 EXPORT_SYMBOL(seq_printf);
 
+/*
+ * Prints output with MiB/MB/KB/etc... suffixes
+ */
+int seq_print_size(struct seq_file *seq, u64 size,
+			const enum string_size_units units)
+{
+	int ret;
+	u64 remainder;
+	char *size_str;
+	remainder = find_size_units(&size, units, &size_str);
+	ret += seq_printf(seq, "%llu", size);
+	ret += seq_puts(seq, size_str);
+	return ret;
+}
+
 /**
  *	mangle_path -	mangle and copy path to buffer beginning
  *	@s: buffer start
include/linux/seq_file.h renamed from include/linux/seq_file.h~add-seq_print_size +3 -0
diff -puN include/linux/seq_file.h~add-seq_print_size include/linux/seq_file.h
@@ -3,6 +3,7 @@
 
 #include <linux/types.h>
 #include <linux/string.h>
+#include <linux/string_helpers.h>
 #include <linux/mutex.h>
 #include <linux/cpumask.h>
 #include <linux/nodemask.h>
@@ -83,6 +84,8 @@ int seq_escape(struct seq_file *, const 
 int seq_putc(struct seq_file *m, char c);
 int seq_puts(struct seq_file *m, const char *s);
 int seq_write(struct seq_file *seq, const void *data, size_t len);
+int seq_print_size(struct seq_file *seq, u64 size,
+			const enum string_size_units units);
 
 int seq_printf(struct seq_file *, const char *, ...)
 	__attribute__ ((format (printf,2,3)));

_

#4 lore raw

RFC PATCH 4/4 show page size in /proc/$pid/numa_maps

In reply to Dave Hansen

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

---

 linux-2.6.git-dave/fs/proc/task_mmu.c |    5 +++++
 1 file changed, 5 insertions(+)
fs/proc/task_mmu.c renamed from fs/proc/task_mmu.c~show-page-size +5 -0
diff -puN fs/proc/task_mmu.c~show-page-size fs/proc/task_mmu.c
@@ -1044,6 +1044,11 @@ static int show_numa_map(struct seq_file
 	if (!md->pages)
 		goto out;
 
+	/* Only interesting for hugetlbfs pages.
+	 * Transparent hugepages are still pagesize=4k */
+	seq_puts(m, " pagesize=");
+	seq_print_size(m, vma_kernel_pagesize(vma), STRING_UNITS_2);
+
 	if (md->anon)
 		seq_printf(m, " anon=%lu", md->anon);
 

_