public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [RFC] proc: meminfo: Replace kB with KiB in output
@ 2016-04-10 15:23 Alexandru Juncu
  2016-04-10 15:23 ` [PATCH] " Alexandru Juncu
  0 siblings, 1 reply; 6+ messages in thread
From: Alexandru Juncu @ 2016-04-10 15:23 UTC (permalink / raw)
  To: linux-kernel, den, mhocko, redkoi, rkagan, hannes, alexj



Hi!

I was helping somebody write some documentation about meminfo and
I started explaining that memory values are mostly multiples of
1024 and that binary prefixes are preferred. The output of
/proc/meminfo is hardcoded to be in 'kB' which should mean 1000
bytes (one kilobytes). But judging by the definition of the K()
macro it is based on page size which is a multiple of 1024.
So the values are actually kibibytes (KiB). Therefore we got
stuck with writing the documentation because there is a
discrepancy between the output and the code.

If I am mistaken, please correct me and ignore the rest of
the message.

I know there is a big debate about using binary prefixes versus
using the de facto definitions based on powers of 10. The
technically correct usage in this case would be KiB but I am
thinking that nobody changed this because it might break things.

Therefore I am writing this email with the main purpose of
finding out what is the reason what the output is as it is.
Is it because changing it would break userspace tools that
read /proc/meminfo ? Or is it because the technical correctness
of the binary prefix side in the debate lost? Or just because
nobody bothered fixing this?

If the last reason is the answer I would like to take the
first step in addressing it with this RFC.

Thank you!

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

* [PATCH] proc: meminfo: Replace kB with KiB in output
  2016-04-10 15:23 [RFC] proc: meminfo: Replace kB with KiB in output Alexandru Juncu
@ 2016-04-10 15:23 ` Alexandru Juncu
  2016-04-11  6:12   ` Michal Hocko
  2016-04-11  7:53   ` Andy Shevchenko
  0 siblings, 2 replies; 6+ messages in thread
From: Alexandru Juncu @ 2016-04-10 15:23 UTC (permalink / raw)
  To: linux-kernel, den, mhocko, redkoi, rkagan, hannes, alexj

Current values are based on multiples of 1024 (powers of 2)
which means that the values in meminfo are not kilobytes
(1000 bytes) but kibibytes (1024 bytes). The correct
prefix for that would be 'Ki' so the output should be 'KiB'.

Signed-off-by: Alexandru Juncu <alexj@linux.com>
---
 fs/proc/meminfo.c | 90 +++++++++++++++++++++++++++----------------------------
 1 file changed, 45 insertions(+), 45 deletions(-)

diff --git a/fs/proc/meminfo.c b/fs/proc/meminfo.c
index 8372046..5f0015e 100644
--- a/fs/proc/meminfo.c
+++ b/fs/proc/meminfo.c
@@ -33,7 +33,7 @@ static int meminfo_proc_show(struct seq_file *m, void *v)
 	int lru;
 
 /*
- * display in kilobytes.
+ * display in kibibytes.
  */
 #define K(x) ((x) << (PAGE_SHIFT - 10))
 	si_meminfo(&i);
@@ -54,61 +54,61 @@ static int meminfo_proc_show(struct seq_file *m, void *v)
 	 * Tagged format, for easy grepping and expansion.
 	 */
 	seq_printf(m,
-		"MemTotal:       %8lu kB\n"
-		"MemFree:        %8lu kB\n"
-		"MemAvailable:   %8lu kB\n"
-		"Buffers:        %8lu kB\n"
-		"Cached:         %8lu kB\n"
-		"SwapCached:     %8lu kB\n"
-		"Active:         %8lu kB\n"
-		"Inactive:       %8lu kB\n"
-		"Active(anon):   %8lu kB\n"
-		"Inactive(anon): %8lu kB\n"
-		"Active(file):   %8lu kB\n"
-		"Inactive(file): %8lu kB\n"
-		"Unevictable:    %8lu kB\n"
-		"Mlocked:        %8lu kB\n"
+		"MemTotal:       %8lu KiB\n"
+		"MemFree:        %8lu KiB\n"
+		"MemAvailable:   %8lu KiB\n"
+		"Buffers:        %8lu KiB\n"
+		"Cached:         %8lu KiB\n"
+		"SwapCached:     %8lu KiB\n"
+		"Active:         %8lu KiB\n"
+		"Inactive:       %8lu KiB\n"
+		"Active(anon):   %8lu KiB\n"
+		"Inactive(anon): %8lu KiB\n"
+		"Active(file):   %8lu KiB\n"
+		"Inactive(file): %8lu KiB\n"
+		"Unevictable:    %8lu KiB\n"
+		"Mlocked:        %8lu KiB\n"
 #ifdef CONFIG_HIGHMEM
-		"HighTotal:      %8lu kB\n"
-		"HighFree:       %8lu kB\n"
-		"LowTotal:       %8lu kB\n"
-		"LowFree:        %8lu kB\n"
+		"HighTotal:      %8lu KiB\n"
+		"HighFree:       %8lu KiB\n"
+		"LowTotal:       %8lu KiB\n"
+		"LowFree:        %8lu KiB\n"
 #endif
 #ifndef CONFIG_MMU
-		"MmapCopy:       %8lu kB\n"
+		"MmapCopy:       %8lu KiB\n"
 #endif
-		"SwapTotal:      %8lu kB\n"
-		"SwapFree:       %8lu kB\n"
-		"Dirty:          %8lu kB\n"
-		"Writeback:      %8lu kB\n"
-		"AnonPages:      %8lu kB\n"
-		"Mapped:         %8lu kB\n"
-		"Shmem:          %8lu kB\n"
-		"Slab:           %8lu kB\n"
-		"SReclaimable:   %8lu kB\n"
-		"SUnreclaim:     %8lu kB\n"
-		"KernelStack:    %8lu kB\n"
-		"PageTables:     %8lu kB\n"
+		"SwapTotal:      %8lu KiB\n"
+		"SwapFree:       %8lu KiB\n"
+		"Dirty:          %8lu KiB\n"
+		"Writeback:      %8lu KiB\n"
+		"AnonPages:      %8lu KiB\n"
+		"Mapped:         %8lu KiB\n"
+		"Shmem:          %8lu KiB\n"
+		"Slab:           %8lu KiB\n"
+		"SReclaimable:   %8lu KiB\n"
+		"SUnreclaim:     %8lu KiB\n"
+		"KernelStack:    %8lu KiB\n"
+		"PageTables:     %8lu KiB\n"
 #ifdef CONFIG_QUICKLIST
-		"Quicklists:     %8lu kB\n"
+		"Quicklists:     %8lu KiB\n"
 #endif
-		"NFS_Unstable:   %8lu kB\n"
-		"Bounce:         %8lu kB\n"
-		"WritebackTmp:   %8lu kB\n"
-		"CommitLimit:    %8lu kB\n"
-		"Committed_AS:   %8lu kB\n"
-		"VmallocTotal:   %8lu kB\n"
-		"VmallocUsed:    %8lu kB\n"
-		"VmallocChunk:   %8lu kB\n"
+		"NFS_Unstable:   %8lu KiB\n"
+		"Bounce:         %8lu KiB\n"
+		"WritebackTmp:   %8lu KiB\n"
+		"CommitLimit:    %8lu KiB\n"
+		"Committed_AS:   %8lu KiB\n"
+		"VmallocTotal:   %8lu KiB\n"
+		"VmallocUsed:    %8lu KiB\n"
+		"VmallocChunk:   %8lu KiB\n"
 #ifdef CONFIG_MEMORY_FAILURE
-		"HardwareCorrupted: %5lu kB\n"
+		"HardwareCorrupted: %5lu KiB\n"
 #endif
 #ifdef CONFIG_TRANSPARENT_HUGEPAGE
-		"AnonHugePages:  %8lu kB\n"
+		"AnonHugePages:  %8lu KiB\n"
 #endif
 #ifdef CONFIG_CMA
-		"CmaTotal:       %8lu kB\n"
-		"CmaFree:        %8lu kB\n"
+		"CmaTotal:       %8lu KiB\n"
+		"CmaFree:        %8lu KiB\n"
 #endif
 		,
 		K(i.totalram),
-- 
2.5.5

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

* Re: [PATCH] proc: meminfo: Replace kB with KiB in output
  2016-04-10 15:23 ` [PATCH] " Alexandru Juncu
@ 2016-04-11  6:12   ` Michal Hocko
  2016-04-11  6:35     ` Alexandru Juncu
  2016-04-11  7:53   ` Andy Shevchenko
  1 sibling, 1 reply; 6+ messages in thread
From: Michal Hocko @ 2016-04-11  6:12 UTC (permalink / raw)
  To: Alexandru Juncu; +Cc: linux-kernel, den, redkoi, rkagan, hannes

On Sun 10-04-16 17:23:11, Alexandru Juncu wrote:
> Current values are based on multiples of 1024 (powers of 2)
> which means that the values in meminfo are not kilobytes
> (1000 bytes) but kibibytes (1024 bytes). The correct
> prefix for that would be 'Ki' so the output should be 'KiB'.

Does the difference actually matters so much that we should change a
user visible format of a file? Some users might not ready to changes
(say sombody did sed 's@.*:[[:space:]]*\([0-9]*\) kB@\1@' to get values.
This change would break it which is something we try to prevent as much
as possible.

So I do not think this all is worth the potential troubles. There are
probably other places where we present kB while we in fact think kiB.

> Signed-off-by: Alexandru Juncu <alexj@linux.com>
> ---
>  fs/proc/meminfo.c | 90 +++++++++++++++++++++++++++----------------------------
>  1 file changed, 45 insertions(+), 45 deletions(-)
> 
> diff --git a/fs/proc/meminfo.c b/fs/proc/meminfo.c
> index 8372046..5f0015e 100644
> --- a/fs/proc/meminfo.c
> +++ b/fs/proc/meminfo.c
> @@ -33,7 +33,7 @@ static int meminfo_proc_show(struct seq_file *m, void *v)
>  	int lru;
>  
>  /*
> - * display in kilobytes.
> + * display in kibibytes.
>   */
>  #define K(x) ((x) << (PAGE_SHIFT - 10))
>  	si_meminfo(&i);
> @@ -54,61 +54,61 @@ static int meminfo_proc_show(struct seq_file *m, void *v)
>  	 * Tagged format, for easy grepping and expansion.
>  	 */
>  	seq_printf(m,
> -		"MemTotal:       %8lu kB\n"
> -		"MemFree:        %8lu kB\n"
> -		"MemAvailable:   %8lu kB\n"
> -		"Buffers:        %8lu kB\n"
> -		"Cached:         %8lu kB\n"
> -		"SwapCached:     %8lu kB\n"
> -		"Active:         %8lu kB\n"
> -		"Inactive:       %8lu kB\n"
> -		"Active(anon):   %8lu kB\n"
> -		"Inactive(anon): %8lu kB\n"
> -		"Active(file):   %8lu kB\n"
> -		"Inactive(file): %8lu kB\n"
> -		"Unevictable:    %8lu kB\n"
> -		"Mlocked:        %8lu kB\n"
> +		"MemTotal:       %8lu KiB\n"
> +		"MemFree:        %8lu KiB\n"
> +		"MemAvailable:   %8lu KiB\n"
> +		"Buffers:        %8lu KiB\n"
> +		"Cached:         %8lu KiB\n"
> +		"SwapCached:     %8lu KiB\n"
> +		"Active:         %8lu KiB\n"
> +		"Inactive:       %8lu KiB\n"
> +		"Active(anon):   %8lu KiB\n"
> +		"Inactive(anon): %8lu KiB\n"
> +		"Active(file):   %8lu KiB\n"
> +		"Inactive(file): %8lu KiB\n"
> +		"Unevictable:    %8lu KiB\n"
> +		"Mlocked:        %8lu KiB\n"
>  #ifdef CONFIG_HIGHMEM
> -		"HighTotal:      %8lu kB\n"
> -		"HighFree:       %8lu kB\n"
> -		"LowTotal:       %8lu kB\n"
> -		"LowFree:        %8lu kB\n"
> +		"HighTotal:      %8lu KiB\n"
> +		"HighFree:       %8lu KiB\n"
> +		"LowTotal:       %8lu KiB\n"
> +		"LowFree:        %8lu KiB\n"
>  #endif
>  #ifndef CONFIG_MMU
> -		"MmapCopy:       %8lu kB\n"
> +		"MmapCopy:       %8lu KiB\n"
>  #endif
> -		"SwapTotal:      %8lu kB\n"
> -		"SwapFree:       %8lu kB\n"
> -		"Dirty:          %8lu kB\n"
> -		"Writeback:      %8lu kB\n"
> -		"AnonPages:      %8lu kB\n"
> -		"Mapped:         %8lu kB\n"
> -		"Shmem:          %8lu kB\n"
> -		"Slab:           %8lu kB\n"
> -		"SReclaimable:   %8lu kB\n"
> -		"SUnreclaim:     %8lu kB\n"
> -		"KernelStack:    %8lu kB\n"
> -		"PageTables:     %8lu kB\n"
> +		"SwapTotal:      %8lu KiB\n"
> +		"SwapFree:       %8lu KiB\n"
> +		"Dirty:          %8lu KiB\n"
> +		"Writeback:      %8lu KiB\n"
> +		"AnonPages:      %8lu KiB\n"
> +		"Mapped:         %8lu KiB\n"
> +		"Shmem:          %8lu KiB\n"
> +		"Slab:           %8lu KiB\n"
> +		"SReclaimable:   %8lu KiB\n"
> +		"SUnreclaim:     %8lu KiB\n"
> +		"KernelStack:    %8lu KiB\n"
> +		"PageTables:     %8lu KiB\n"
>  #ifdef CONFIG_QUICKLIST
> -		"Quicklists:     %8lu kB\n"
> +		"Quicklists:     %8lu KiB\n"
>  #endif
> -		"NFS_Unstable:   %8lu kB\n"
> -		"Bounce:         %8lu kB\n"
> -		"WritebackTmp:   %8lu kB\n"
> -		"CommitLimit:    %8lu kB\n"
> -		"Committed_AS:   %8lu kB\n"
> -		"VmallocTotal:   %8lu kB\n"
> -		"VmallocUsed:    %8lu kB\n"
> -		"VmallocChunk:   %8lu kB\n"
> +		"NFS_Unstable:   %8lu KiB\n"
> +		"Bounce:         %8lu KiB\n"
> +		"WritebackTmp:   %8lu KiB\n"
> +		"CommitLimit:    %8lu KiB\n"
> +		"Committed_AS:   %8lu KiB\n"
> +		"VmallocTotal:   %8lu KiB\n"
> +		"VmallocUsed:    %8lu KiB\n"
> +		"VmallocChunk:   %8lu KiB\n"
>  #ifdef CONFIG_MEMORY_FAILURE
> -		"HardwareCorrupted: %5lu kB\n"
> +		"HardwareCorrupted: %5lu KiB\n"
>  #endif
>  #ifdef CONFIG_TRANSPARENT_HUGEPAGE
> -		"AnonHugePages:  %8lu kB\n"
> +		"AnonHugePages:  %8lu KiB\n"
>  #endif
>  #ifdef CONFIG_CMA
> -		"CmaTotal:       %8lu kB\n"
> -		"CmaFree:        %8lu kB\n"
> +		"CmaTotal:       %8lu KiB\n"
> +		"CmaFree:        %8lu KiB\n"
>  #endif
>  		,
>  		K(i.totalram),
> -- 
> 2.5.5
> 

-- 
Michal Hocko
SUSE Labs

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

* Re: [PATCH] proc: meminfo: Replace kB with KiB in output
  2016-04-11  6:12   ` Michal Hocko
@ 2016-04-11  6:35     ` Alexandru Juncu
  2016-04-11  6:48       ` Michal Hocko
  0 siblings, 1 reply; 6+ messages in thread
From: Alexandru Juncu @ 2016-04-11  6:35 UTC (permalink / raw)
  To: Michal Hocko
  Cc: linux-kernel@vger.kernel.org, den, redkoi, rkagan,
	Johannes Weiner

On 11 April 2016 at 08:12, Michal Hocko <mhocko@kernel.org> wrote:
> On Sun 10-04-16 17:23:11, Alexandru Juncu wrote:
>> Current values are based on multiples of 1024 (powers of 2)
>> which means that the values in meminfo are not kilobytes
>> (1000 bytes) but kibibytes (1024 bytes). The correct
>> prefix for that would be 'Ki' so the output should be 'KiB'.
>
> Does the difference actually matters so much that we should change a
> user visible format of a file? Some users might not ready to changes
> (say sombody did sed 's@.*:[[:space:]]*\([0-9]*\) kB@\1@' to get values.
> This change would break it which is something we try to prevent as much
> as possible.
>
> So I do not think this all is worth the potential troubles. There are
> probably other places where we present kB while we in fact think kiB.

Isn't it at least worth clearly specifying this in the documentation
(maybe also comment in code)?

>
>> Signed-off-by: Alexandru Juncu <alexj@linux.com>
>> ---
>>  fs/proc/meminfo.c | 90 +++++++++++++++++++++++++++----------------------------
>>  1 file changed, 45 insertions(+), 45 deletions(-)
>>
>> diff --git a/fs/proc/meminfo.c b/fs/proc/meminfo.c
>> index 8372046..5f0015e 100644
>> --- a/fs/proc/meminfo.c
>> +++ b/fs/proc/meminfo.c
>> @@ -33,7 +33,7 @@ static int meminfo_proc_show(struct seq_file *m, void *v)
>>       int lru;
>>
>>  /*
>> - * display in kilobytes.
>> + * display in kibibytes.
>>   */
>>  #define K(x) ((x) << (PAGE_SHIFT - 10))
>>       si_meminfo(&i);
>> @@ -54,61 +54,61 @@ static int meminfo_proc_show(struct seq_file *m, void *v)
>>        * Tagged format, for easy grepping and expansion.
>>        */
>>       seq_printf(m,
>> -             "MemTotal:       %8lu kB\n"
>> -             "MemFree:        %8lu kB\n"
>> -             "MemAvailable:   %8lu kB\n"
>> -             "Buffers:        %8lu kB\n"
>> -             "Cached:         %8lu kB\n"
>> -             "SwapCached:     %8lu kB\n"
>> -             "Active:         %8lu kB\n"
>> -             "Inactive:       %8lu kB\n"
>> -             "Active(anon):   %8lu kB\n"
>> -             "Inactive(anon): %8lu kB\n"
>> -             "Active(file):   %8lu kB\n"
>> -             "Inactive(file): %8lu kB\n"
>> -             "Unevictable:    %8lu kB\n"
>> -             "Mlocked:        %8lu kB\n"
>> +             "MemTotal:       %8lu KiB\n"
>> +             "MemFree:        %8lu KiB\n"
>> +             "MemAvailable:   %8lu KiB\n"
>> +             "Buffers:        %8lu KiB\n"
>> +             "Cached:         %8lu KiB\n"
>> +             "SwapCached:     %8lu KiB\n"
>> +             "Active:         %8lu KiB\n"
>> +             "Inactive:       %8lu KiB\n"
>> +             "Active(anon):   %8lu KiB\n"
>> +             "Inactive(anon): %8lu KiB\n"
>> +             "Active(file):   %8lu KiB\n"
>> +             "Inactive(file): %8lu KiB\n"
>> +             "Unevictable:    %8lu KiB\n"
>> +             "Mlocked:        %8lu KiB\n"
>>  #ifdef CONFIG_HIGHMEM
>> -             "HighTotal:      %8lu kB\n"
>> -             "HighFree:       %8lu kB\n"
>> -             "LowTotal:       %8lu kB\n"
>> -             "LowFree:        %8lu kB\n"
>> +             "HighTotal:      %8lu KiB\n"
>> +             "HighFree:       %8lu KiB\n"
>> +             "LowTotal:       %8lu KiB\n"
>> +             "LowFree:        %8lu KiB\n"
>>  #endif
>>  #ifndef CONFIG_MMU
>> -             "MmapCopy:       %8lu kB\n"
>> +             "MmapCopy:       %8lu KiB\n"
>>  #endif
>> -             "SwapTotal:      %8lu kB\n"
>> -             "SwapFree:       %8lu kB\n"
>> -             "Dirty:          %8lu kB\n"
>> -             "Writeback:      %8lu kB\n"
>> -             "AnonPages:      %8lu kB\n"
>> -             "Mapped:         %8lu kB\n"
>> -             "Shmem:          %8lu kB\n"
>> -             "Slab:           %8lu kB\n"
>> -             "SReclaimable:   %8lu kB\n"
>> -             "SUnreclaim:     %8lu kB\n"
>> -             "KernelStack:    %8lu kB\n"
>> -             "PageTables:     %8lu kB\n"
>> +             "SwapTotal:      %8lu KiB\n"
>> +             "SwapFree:       %8lu KiB\n"
>> +             "Dirty:          %8lu KiB\n"
>> +             "Writeback:      %8lu KiB\n"
>> +             "AnonPages:      %8lu KiB\n"
>> +             "Mapped:         %8lu KiB\n"
>> +             "Shmem:          %8lu KiB\n"
>> +             "Slab:           %8lu KiB\n"
>> +             "SReclaimable:   %8lu KiB\n"
>> +             "SUnreclaim:     %8lu KiB\n"
>> +             "KernelStack:    %8lu KiB\n"
>> +             "PageTables:     %8lu KiB\n"
>>  #ifdef CONFIG_QUICKLIST
>> -             "Quicklists:     %8lu kB\n"
>> +             "Quicklists:     %8lu KiB\n"
>>  #endif
>> -             "NFS_Unstable:   %8lu kB\n"
>> -             "Bounce:         %8lu kB\n"
>> -             "WritebackTmp:   %8lu kB\n"
>> -             "CommitLimit:    %8lu kB\n"
>> -             "Committed_AS:   %8lu kB\n"
>> -             "VmallocTotal:   %8lu kB\n"
>> -             "VmallocUsed:    %8lu kB\n"
>> -             "VmallocChunk:   %8lu kB\n"
>> +             "NFS_Unstable:   %8lu KiB\n"
>> +             "Bounce:         %8lu KiB\n"
>> +             "WritebackTmp:   %8lu KiB\n"
>> +             "CommitLimit:    %8lu KiB\n"
>> +             "Committed_AS:   %8lu KiB\n"
>> +             "VmallocTotal:   %8lu KiB\n"
>> +             "VmallocUsed:    %8lu KiB\n"
>> +             "VmallocChunk:   %8lu KiB\n"
>>  #ifdef CONFIG_MEMORY_FAILURE
>> -             "HardwareCorrupted: %5lu kB\n"
>> +             "HardwareCorrupted: %5lu KiB\n"
>>  #endif
>>  #ifdef CONFIG_TRANSPARENT_HUGEPAGE
>> -             "AnonHugePages:  %8lu kB\n"
>> +             "AnonHugePages:  %8lu KiB\n"
>>  #endif
>>  #ifdef CONFIG_CMA
>> -             "CmaTotal:       %8lu kB\n"
>> -             "CmaFree:        %8lu kB\n"
>> +             "CmaTotal:       %8lu KiB\n"
>> +             "CmaFree:        %8lu KiB\n"
>>  #endif
>>               ,
>>               K(i.totalram),
>> --
>> 2.5.5
>>
>
> --
> Michal Hocko
> SUSE Labs

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

* Re: [PATCH] proc: meminfo: Replace kB with KiB in output
  2016-04-11  6:35     ` Alexandru Juncu
@ 2016-04-11  6:48       ` Michal Hocko
  0 siblings, 0 replies; 6+ messages in thread
From: Michal Hocko @ 2016-04-11  6:48 UTC (permalink / raw)
  To: Alexandru Juncu
  Cc: linux-kernel@vger.kernel.org, den, redkoi, rkagan,
	Johannes Weiner

On Mon 11-04-16 08:35:00, Alexandru Juncu wrote:
> On 11 April 2016 at 08:12, Michal Hocko <mhocko@kernel.org> wrote:
> > On Sun 10-04-16 17:23:11, Alexandru Juncu wrote:
> >> Current values are based on multiples of 1024 (powers of 2)
> >> which means that the values in meminfo are not kilobytes
> >> (1000 bytes) but kibibytes (1024 bytes). The correct
> >> prefix for that would be 'Ki' so the output should be 'KiB'.
> >
> > Does the difference actually matters so much that we should change a
> > user visible format of a file? Some users might not ready to changes
> > (say sombody did sed 's@.*:[[:space:]]*\([0-9]*\) kB@\1@' to get values.
> > This change would break it which is something we try to prevent as much
> > as possible.
> >
> > So I do not think this all is worth the potential troubles. There are
> > probably other places where we present kB while we in fact think kiB.
> 
> Isn't it at least worth clearly specifying this in the documentation
> (maybe also comment in code)?

Sure, improving our documentation is highly appreciated of course. 

Thanks!
-- 
Michal Hocko
SUSE Labs

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

* Re: [PATCH] proc: meminfo: Replace kB with KiB in output
  2016-04-10 15:23 ` [PATCH] " Alexandru Juncu
  2016-04-11  6:12   ` Michal Hocko
@ 2016-04-11  7:53   ` Andy Shevchenko
  1 sibling, 0 replies; 6+ messages in thread
From: Andy Shevchenko @ 2016-04-11  7:53 UTC (permalink / raw)
  To: Alexandru Juncu
  Cc: linux-kernel@vger.kernel.org, den, mhocko, redkoi, rkagan, hannes

On Sun, Apr 10, 2016 at 6:23 PM, Alexandru Juncu <alexj@linux.com> wrote:
> Current values are based on multiples of 1024 (powers of 2)
> which means that the values in meminfo are not kilobytes
> (1000 bytes) but kibibytes (1024 bytes). The correct
> prefix for that would be 'Ki' so the output should be 'KiB'.

I'm pretty sure you will get NAK for that.
Obvious reason — procfs is ABI of the kernel. "We won't break userspace."

>
> Signed-off-by: Alexandru Juncu <alexj@linux.com>
> ---
>  fs/proc/meminfo.c | 90 +++++++++++++++++++++++++++----------------------------
>  1 file changed, 45 insertions(+), 45 deletions(-)
>
> diff --git a/fs/proc/meminfo.c b/fs/proc/meminfo.c
> index 8372046..5f0015e 100644
> --- a/fs/proc/meminfo.c
> +++ b/fs/proc/meminfo.c
> @@ -33,7 +33,7 @@ static int meminfo_proc_show(struct seq_file *m, void *v)
>         int lru;
>
>  /*
> - * display in kilobytes.
> + * display in kibibytes.
>   */
>  #define K(x) ((x) << (PAGE_SHIFT - 10))
>         si_meminfo(&i);
> @@ -54,61 +54,61 @@ static int meminfo_proc_show(struct seq_file *m, void *v)
>          * Tagged format, for easy grepping and expansion.
>          */
>         seq_printf(m,
> -               "MemTotal:       %8lu kB\n"
> -               "MemFree:        %8lu kB\n"
> -               "MemAvailable:   %8lu kB\n"
> -               "Buffers:        %8lu kB\n"
> -               "Cached:         %8lu kB\n"
> -               "SwapCached:     %8lu kB\n"
> -               "Active:         %8lu kB\n"
> -               "Inactive:       %8lu kB\n"
> -               "Active(anon):   %8lu kB\n"
> -               "Inactive(anon): %8lu kB\n"
> -               "Active(file):   %8lu kB\n"
> -               "Inactive(file): %8lu kB\n"
> -               "Unevictable:    %8lu kB\n"
> -               "Mlocked:        %8lu kB\n"
> +               "MemTotal:       %8lu KiB\n"
> +               "MemFree:        %8lu KiB\n"
> +               "MemAvailable:   %8lu KiB\n"
> +               "Buffers:        %8lu KiB\n"
> +               "Cached:         %8lu KiB\n"
> +               "SwapCached:     %8lu KiB\n"
> +               "Active:         %8lu KiB\n"
> +               "Inactive:       %8lu KiB\n"
> +               "Active(anon):   %8lu KiB\n"
> +               "Inactive(anon): %8lu KiB\n"
> +               "Active(file):   %8lu KiB\n"
> +               "Inactive(file): %8lu KiB\n"
> +               "Unevictable:    %8lu KiB\n"
> +               "Mlocked:        %8lu KiB\n"
>  #ifdef CONFIG_HIGHMEM
> -               "HighTotal:      %8lu kB\n"
> -               "HighFree:       %8lu kB\n"
> -               "LowTotal:       %8lu kB\n"
> -               "LowFree:        %8lu kB\n"
> +               "HighTotal:      %8lu KiB\n"
> +               "HighFree:       %8lu KiB\n"
> +               "LowTotal:       %8lu KiB\n"
> +               "LowFree:        %8lu KiB\n"
>  #endif
>  #ifndef CONFIG_MMU
> -               "MmapCopy:       %8lu kB\n"
> +               "MmapCopy:       %8lu KiB\n"
>  #endif
> -               "SwapTotal:      %8lu kB\n"
> -               "SwapFree:       %8lu kB\n"
> -               "Dirty:          %8lu kB\n"
> -               "Writeback:      %8lu kB\n"
> -               "AnonPages:      %8lu kB\n"
> -               "Mapped:         %8lu kB\n"
> -               "Shmem:          %8lu kB\n"
> -               "Slab:           %8lu kB\n"
> -               "SReclaimable:   %8lu kB\n"
> -               "SUnreclaim:     %8lu kB\n"
> -               "KernelStack:    %8lu kB\n"
> -               "PageTables:     %8lu kB\n"
> +               "SwapTotal:      %8lu KiB\n"
> +               "SwapFree:       %8lu KiB\n"
> +               "Dirty:          %8lu KiB\n"
> +               "Writeback:      %8lu KiB\n"
> +               "AnonPages:      %8lu KiB\n"
> +               "Mapped:         %8lu KiB\n"
> +               "Shmem:          %8lu KiB\n"
> +               "Slab:           %8lu KiB\n"
> +               "SReclaimable:   %8lu KiB\n"
> +               "SUnreclaim:     %8lu KiB\n"
> +               "KernelStack:    %8lu KiB\n"
> +               "PageTables:     %8lu KiB\n"
>  #ifdef CONFIG_QUICKLIST
> -               "Quicklists:     %8lu kB\n"
> +               "Quicklists:     %8lu KiB\n"
>  #endif
> -               "NFS_Unstable:   %8lu kB\n"
> -               "Bounce:         %8lu kB\n"
> -               "WritebackTmp:   %8lu kB\n"
> -               "CommitLimit:    %8lu kB\n"
> -               "Committed_AS:   %8lu kB\n"
> -               "VmallocTotal:   %8lu kB\n"
> -               "VmallocUsed:    %8lu kB\n"
> -               "VmallocChunk:   %8lu kB\n"
> +               "NFS_Unstable:   %8lu KiB\n"
> +               "Bounce:         %8lu KiB\n"
> +               "WritebackTmp:   %8lu KiB\n"
> +               "CommitLimit:    %8lu KiB\n"
> +               "Committed_AS:   %8lu KiB\n"
> +               "VmallocTotal:   %8lu KiB\n"
> +               "VmallocUsed:    %8lu KiB\n"
> +               "VmallocChunk:   %8lu KiB\n"
>  #ifdef CONFIG_MEMORY_FAILURE
> -               "HardwareCorrupted: %5lu kB\n"
> +               "HardwareCorrupted: %5lu KiB\n"
>  #endif
>  #ifdef CONFIG_TRANSPARENT_HUGEPAGE
> -               "AnonHugePages:  %8lu kB\n"
> +               "AnonHugePages:  %8lu KiB\n"
>  #endif
>  #ifdef CONFIG_CMA
> -               "CmaTotal:       %8lu kB\n"
> -               "CmaFree:        %8lu kB\n"
> +               "CmaTotal:       %8lu KiB\n"
> +               "CmaFree:        %8lu KiB\n"
>  #endif
>                 ,
>                 K(i.totalram),
> --
> 2.5.5
>



-- 
With Best Regards,
Andy Shevchenko

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

end of thread, other threads:[~2016-04-11  7:53 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-04-10 15:23 [RFC] proc: meminfo: Replace kB with KiB in output Alexandru Juncu
2016-04-10 15:23 ` [PATCH] " Alexandru Juncu
2016-04-11  6:12   ` Michal Hocko
2016-04-11  6:35     ` Alexandru Juncu
2016-04-11  6:48       ` Michal Hocko
2016-04-11  7:53   ` Andy Shevchenko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox