procfs: Add VmFlags field in smaps output

Hi guys, in a sake of c/r we need to fetch additional VMA characteristics, so I though would /proc/pid/smaps be appropriate place for it? If yes, I would like to know if the output format provided below looks reasonable.

Please review, thanks!
---

Quoted 7 lines · Cyrill Gorcunov

Hi guys, in a sake of c/r we need to fetch additional VMA characteristics, so I though would /proc/pid/smaps be appropriate place for it? If yes, I would like to know if the output format provided below looks reasonable.

Please review, thanks!
---

Also I've had a bit different output format, not sure which one is better.
---
From: Cyrill Gorcunov <[email protected]>
Subject: [RFC] procfs: Add VmFlags field in smaps output

When we do restore VMA area after checkpoint we would like to know if the area was locked or say it has mergeable attribute, but at moment the kernel does not provide such information, thus we can't figure out if we should call mlock/madvise on VMA restore.

This patch adds new VmFlags field to smaps output with vma->vm_flags encoded.

This field is CONFIG_CHECKPOINT_RESTORE dependent since at moment I don't know if someone else might need it.

Signed-off-by Cyrill Gorcunov <[email protected]>

CC Pavel Emelyanov <[email protected]>

CC Andrew Morton <[email protected]>

CC Peter Zijlstra <[email protected]>

---
 fs/proc/task_mmu.c |   21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

Index: linux-2.6.git/fs/proc/task_mmu.c
===================================================================

fs/proc/task_mmu.c +8 -0

@@ -480,6 +480,25 @@ static int smaps_pte_range(pmd_t *pmd, u
 	return 0;
 }
 
+#ifdef CONFIG_CHECKPOINT_RESTORE
+static void show_smap_vma_flags(struct seq_file *m, struct vm_area_struct *vma)
+{
+	seq_printf(m,
+		   "VmFlags:               %c%c%c%c%c%c%c%c%c\n",
+		   vma->vm_flags & VM_LOCKED ?		'l' : '-',
+		   vma->vm_flags & VM_GROWSDOWN ?	'g' : '-',
+		   vma->vm_flags & VM_RAND_READ ?	'r' : '-',

Quoted 12 lines

fs/proc/task_mmu.c +12 -0

+#ifdef CONFIG_CHECKPOINT_RESTORE
+static void show_smap_vma_flags(struct seq_file *m, struct vm_area_struct *vma)
+{
+	seq_printf(m,
+		   "VmFlags:               %c%c%c%c%c%c%c%c%c\n",
+		   vma->vm_flags & VM_LOCKED ?		'l' : '-',
+		   vma->vm_flags & VM_GROWSDOWN ?	'g' : '-',
+		   vma->vm_flags & VM_RAND_READ ?	'r' : '-',
+		   vma->vm_flags & VM_SEQ_READ ?	'q' : '-',
+		   vma->vm_flags & VM_DONTCOPY ?	'-' : 'c',
+		   vma->vm_flags & VM_MERGEABLE ?	'm' : '-',
+		   vma->vm_flags & VM_HUGEPAGE ?	'h' : '-',

fs/proc/task_mmu.c static int smaps_pte_range(pmd_t *pmd, u · line 500

Quoted 6 lines

fs/proc/task_mmu.c +6 -0

+		   vma->vm_flags & VM_NOHUGEPAGE ?	'-' : 'h',
+		   vma->vm_flags & VM_DONTDUMP ?	'-' : 'd');
+}
+#else
+static void show_smap_vma_flags(struct seq_file *m, struct vm_area_struct *vma) { }
+#endif

I guess this more terse output is better. It should be documented (in

Documentation/filesystems/proc.txt, I suppose) and we should add a code comment here reminding people to update that documentation when they change this function.

There are about 28 VM_foo flags and here you've semi-randomly chosen 9 of them for display. This seems rather ugly and we should expect that

people will change this code to display additional flags in the future.

And we should also expect some of the flags which _are_ displayed to vanish in the future as the code evolves.

This data is pretty dependent upon internal kernel implementation details, so it is something we normally try to avoid exporting to userspace.

We should design the interface with these future changes in mind. That means careful documentation and perhaps a format which discourages userspace programmers from assuming that there will always be nine fields.

A common way in which we do this future-proofing is to display the info in name:value tuples (eg, /proc/meminfo). So userspace parses for the "name" rather than looking into a fixed position in the /proc output.

So.... with this thought in mind, perhaps a better output format would be something like:

	VmFlags: LO:1 GR:0 RA:0 SE:1 ...

ie: a two-character "name" and a boolean "value". Something like that.

Also, it worries me a bit that this interface vanishes if CONFIG_CHECKPOINT_RESTORE=n. One can easily anticipate that non-c/r userspace programs will start to use this interface, and they will want it to be present in CONFIG_CHECKPOINT_RESTORE=n kernels.

Quoted 5 lines

A common way in which we do this future-proofing is to display the info in name:value tuples (eg, /proc/meminfo). So userspace parses for the "name" rather than looking into a fixed position in the /proc output.

Quoted 6 lines

So.... with this thought in mind, perhaps a better output format would be something like:

	VmFlags: LO:1 GR:0 RA:0 SE:1 ...

ie: a two-character "name" and a boolean "value". Something like that.

OK, Andrew, I'll try to come with something like that tomorrow, thanks!