diff mbox series

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

Message ID cfb81f7a-f448-5bc2-b0e1-8136fcd1dd8c@google.com (mailing list archive)
State New
Headers show
Series mm,thp: fix sloppy text output | expand

Commit Message

Hugh Dickins Aug. 14, 2023, 8:02 p.m. UTC
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(-)

Comments

Alexey Dobriyan Aug. 15, 2023, 2:27 p.m. UTC | #1
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.
Hugh Dickins Aug. 15, 2023, 4:57 p.m. UTC | #2
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
diff mbox series

Patch

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.