[PATCH 0/3] mm,thp: fix sloppy text output

[PATCH 0/3] mm,thp: fix sloppy text output

[Hugh Dickins]

mm,thp: fix sloppy text output

Three independent trivial patches, fixing sloppy text output which has
annoyed me; but might risk surprising a parser, so any can be dropped.

1/3 mm,thp: no space after colon in Mem-Info fields
2/3 mm,thp: fix nodeN/meminfo output alignment
3/3 mm,thp: fix smaps THPeligible output alignment

 drivers/base/node.c                          | 4 ++--
 fs/proc/task_mmu.c                           | 2 +-
 mm/show_mem.c                                | 6 +++---
 tools/testing/selftests/proc/proc-empty-vm.c | 4 ++--
 4 files changed, 8 insertions(+), 8 deletions(-)

Hugh

[PATCH 1/3] mm,thp: no space after colon in Mem-Info fields

[Hugh Dickins]

The SysRq-m or OOM Mem-Info dmesg showed (long lines containing)
... shmem:NkB shmem_thp: NkB shmem_pmdmapped: NkB anon_thp: NkB ...

Delete the space after the colon after shmem_thp, shmem_pmdmapped,
anon_thp: as the shmem example shows, no other fields have a space
after the colon in this output.

Signed-off-by: Hugh Dickins <hughd@google.com>
---
 mm/show_mem.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/mm/show_mem.c b/mm/show_mem.c
index 09c7d036d49e..4b888b18bdde 100644
--- a/mm/show_mem.c
+++ b/mm/show_mem.c
@@ -251,9 +251,9 @@ static void show_free_areas(unsigned int filter, nodemask_t *nodemask, int max_z
 			" writeback:%lukB"
 			" shmem:%lukB"
 #ifdef CONFIG_TRANSPARENT_HUGEPAGE
-			" shmem_thp: %lukB"
-			" shmem_pmdmapped: %lukB"
-			" anon_thp: %lukB"
+			" shmem_thp:%lukB"
+			" shmem_pmdmapped:%lukB"
+			" anon_thp:%lukB"
 #endif
 			" writeback_tmp:%lukB"
 			" kernel_stack:%lukB"
-- 
2.35.3

Re: [PATCH 1/3] mm,thp: no space after colon in Mem-Info fields

[David Hildenbrand]

On 14.08.23 22:00, Hugh Dickins wrote:
> The SysRq-m or OOM Mem-Info dmesg showed (long lines containing)
> ... shmem:NkB shmem_thp: NkB shmem_pmdmapped: NkB anon_thp: NkB ...
> 
> Delete the space after the colon after shmem_thp, shmem_pmdmapped,
> anon_thp: as the shmem example shows, no other fields have a space
> after the colon in this output.
> 
> Signed-off-by: Hugh Dickins <hughd@google.com>
> ---
>   mm/show_mem.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/mm/show_mem.c b/mm/show_mem.c
> index 09c7d036d49e..4b888b18bdde 100644
> --- a/mm/show_mem.c
> +++ b/mm/show_mem.c
> @@ -251,9 +251,9 @@ static void show_free_areas(unsigned int filter, nodemask_t *nodemask, int max_z
>   			" writeback:%lukB"
>   			" shmem:%lukB"
>   #ifdef CONFIG_TRANSPARENT_HUGEPAGE
> -			" shmem_thp: %lukB"
> -			" shmem_pmdmapped: %lukB"
> -			" anon_thp: %lukB"
> +			" shmem_thp:%lukB"
> +			" shmem_pmdmapped:%lukB"
> +			" anon_thp:%lukB"
>   #endif
>   			" writeback_tmp:%lukB"
>   			" kernel_stack:%lukB"

Reviewed-by: David Hildenbrand <david@redhat.com>

-- 
Cheers,

David / dhildenb

[PATCH 2/3] mm,thp: fix nodeN/meminfo output alignment

[Hugh Dickins]

Add one more space to FileHugePages and FilePmdMapped, so the output
is aligned with other rows in /sys/devices/system/node/nodeN/meminfo.

Signed-off-by: Hugh Dickins <hughd@google.com>
---
 drivers/base/node.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/base/node.c b/drivers/base/node.c
index 9de524e56307..8e871ba9162f 100644
--- a/drivers/base/node.c
+++ b/drivers/base/node.c
@@ -446,8 +446,8 @@ static ssize_t node_read_meminfo(struct device *dev,
 			     "Node %d AnonHugePages:  %8lu kB\n"
 			     "Node %d ShmemHugePages: %8lu kB\n"
 			     "Node %d ShmemPmdMapped: %8lu kB\n"
-			     "Node %d FileHugePages: %8lu kB\n"
-			     "Node %d FilePmdMapped: %8lu kB\n"
+			     "Node %d FileHugePages:  %8lu kB\n"
+			     "Node %d FilePmdMapped:  %8lu kB\n"
 #endif
 #ifdef CONFIG_UNACCEPTED_MEMORY
 			     "Node %d Unaccepted:     %8lu kB\n"
-- 
2.35.3

Re: [PATCH 2/3] mm,thp: fix nodeN/meminfo output alignment

[David Hildenbrand]

On 14.08.23 22:01, Hugh Dickins wrote:
> Add one more space to FileHugePages and FilePmdMapped, so the output
> is aligned with other rows in /sys/devices/system/node/nodeN/meminfo.
> 
> Signed-off-by: Hugh Dickins <hughd@google.com>
> ---
>   drivers/base/node.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/base/node.c b/drivers/base/node.c
> index 9de524e56307..8e871ba9162f 100644
> --- a/drivers/base/node.c
> +++ b/drivers/base/node.c
> @@ -446,8 +446,8 @@ static ssize_t node_read_meminfo(struct device *dev,
>   			     "Node %d AnonHugePages:  %8lu kB\n"
>   			     "Node %d ShmemHugePages: %8lu kB\n"
>   			     "Node %d ShmemPmdMapped: %8lu kB\n"
> -			     "Node %d FileHugePages: %8lu kB\n"
> -			     "Node %d FilePmdMapped: %8lu kB\n"
> +			     "Node %d FileHugePages:  %8lu kB\n"
> +			     "Node %d FilePmdMapped:  %8lu kB\n"
>   #endif
>   #ifdef CONFIG_UNACCEPTED_MEMORY
>   			     "Node %d Unaccepted:     %8lu kB\n"

Reviewed-by: David Hildenbrand <david@redhat.com>

-- 
Cheers,

David / dhildenb

[PATCH 3/3] mm,thp: fix smaps THPeligible output alignment

[Hugh Dickins]

Extract from current /proc/self/smaps output:

Swap:                  0 kB
SwapPss:               0 kB
Locked:                0 kB
THPeligible:    0
ProtectionKey:         0

That's not the alignment shown in Documentation/filesystems/proc.rst:
it's an ugly artifact from missing out the %8 other fields are using;
but there's even one selftest which expects it to look that way.  Hoping
no other smaps parsers depend on THPeligible to look so ugly, fix these.

Signed-off-by: Hugh Dickins <hughd@google.com>
---
 fs/proc/task_mmu.c                           | 2 +-
 tools/testing/selftests/proc/proc-empty-vm.c | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
index 51315133cdc2..15ddf4653a19 100644
--- a/fs/proc/task_mmu.c
+++ b/fs/proc/task_mmu.c
@@ -855,7 +855,7 @@ static int show_smap(struct seq_file *m, void *v)
 
 	__show_smap(m, &mss, false);
 
-	seq_printf(m, "THPeligible:    %d\n",
+	seq_printf(m, "THPeligible:    %8u\n",
 		   hugepage_vma_check(vma, vma->vm_flags, true, false, true));
 
 	if (arch_pkeys_enabled())
diff --git a/tools/testing/selftests/proc/proc-empty-vm.c b/tools/testing/selftests/proc/proc-empty-vm.c
index dfbcb3ce2194..b16c13688b88 100644
--- a/tools/testing/selftests/proc/proc-empty-vm.c
+++ b/tools/testing/selftests/proc/proc-empty-vm.c
@@ -82,7 +82,7 @@ static const char proc_pid_smaps_vsyscall_1[] =
 "Swap:                  0 kB\n"
 "SwapPss:               0 kB\n"
 "Locked:                0 kB\n"
-"THPeligible:    0\n"
+"THPeligible:           0\n"
 /*
  * "ProtectionKey:" field is conditional. It is possible to check it as well,
  * but I don't have such machine.
@@ -112,7 +112,7 @@ static const char proc_pid_smaps_vsyscall_2[] =
 "Swap:                  0 kB\n"
 "SwapPss:               0 kB\n"
 "Locked:                0 kB\n"
-"THPeligible:    0\n"
+"THPeligible:           0\n"
 /*
  * "ProtectionKey:" field is conditional. It is possible to check it as well,
  * but I'm too tired.
-- 
2.35.3

Re: [PATCH 3/3] mm,thp: fix smaps THPeligible output alignment

[Alexey Dobriyan]

On Mon, Aug 14, 2023 at 01:02:08PM -0700, Hugh Dickins wrote:
> Swap:                  0 kB
> SwapPss:               0 kB
> Locked:                0 kB
> THPeligible:    0
> ProtectionKey:         0

> -	seq_printf(m, "THPeligible:    %d\n",
> +	seq_printf(m, "THPeligible:    %8u\n",
>  		   hugepage_vma_check(vma, vma->vm_flags, true, false, true));

Why format string change? It would only slow down printing.

I'd print with

	"%u", +hugepage_vma_check()

or just add whitespace.

Re: [PATCH 3/3] mm,thp: fix smaps THPeligible output alignment

[Hugh Dickins]

On Tue, 15 Aug 2023, Alexey Dobriyan wrote:
> On Mon, Aug 14, 2023 at 01:02:08PM -0700, Hugh Dickins wrote:
> > Swap:                  0 kB
> > SwapPss:               0 kB
> > Locked:                0 kB
> > THPeligible:    0
> > ProtectionKey:         0
> 
> > -	seq_printf(m, "THPeligible:    %d\n",
> > +	seq_printf(m, "THPeligible:    %8u\n",
> >  		   hugepage_vma_check(vma, vma->vm_flags, true, false, true));
> 
> Why format string change? It would only slow down printing.

To document the alignment, and to look like the ProtectionKey line below.

> 
> I'd print with
> 
> 	"%u", +hugepage_vma_check()

Sorry, I don't understand.

> 
> or just add whitespace.

My original patch did that, then I thought it better to document the
alignment and save those bytes of kernel.

Hugh