Introducing VmFlags field into smaps output

Hi guys, during c/r sessions we've found that there is no way at the moment to fetch some VMA associated flags, such as mlock() and madvise(), thus the patches in this series intorduce new field into "smaps" output called VmFlags where all flags associated with the particular VMA is shown in two letter mnemonic.

Strictly speaking for c/r we only need mlock/madvise bits but it has been said that providing just a few flags looks somehow inconsistent. So all flags are here now.

Please review. Comments and complains are quite welcome!

Quoted 6 lines · On Mon, 22 Oct 2012 23:14:52 +0400 Cyrill Gorcunov <[email protected]> wrote:

Hi guys, during c/r sessions we've found that there is no way at the moment to fetch some VMA associated flags, such as mlock() and madvise(), thus the patches in this series intorduce new field into "smaps" output called VmFlags where all flags associated with the particular VMA is shown in two letter mnemonic.

Quoted 5 lines

Strictly speaking for c/r we only need mlock/madvise bits but it has been said that providing just a few flags looks somehow inconsistent. So all flags are here now.

Please review. Comments and complains are quite welcome!

Sigh, it's still a pretty nasty-looking interface. Better ideas are welcomed.

I joined the patches into one, massaged the changelog a bit and added a couple more paragraphs explaining why we're doing this to ourselves.

Quoted 6 lines · On Mon, 22 Oct 2012 23:14:52 +0400 Cyrill Gorcunov <[email protected]> wrote:

Hi guys, during c/r sessions we've found that there is no way at the moment to fetch some VMA associated flags, such as mlock() and madvise(), thus the patches in this series intorduce new field into "smaps" output called VmFlags where all flags associated with the particular VMA is shown in two letter mnemonic.

Quoted 5 lines

Strictly speaking for c/r we only need mlock/madvise bits but it has been said that providing just a few flags looks somehow inconsistent. So all flags are here now.

Please review. Comments and complains are quite welcome!

Sigh, it's still a pretty nasty-looking interface. Better ideas are welcomed.

I joined the patches into one, massaged the changelog a bit and added a couple more paragraphs explaining why we're doing this to ourselves.

Quoted 9 lines · Andrew Morton

Quoted 6 lines · On Mon, 22 Oct 2012 23:14:52 +0400 Cyrill Gorcunov <[email protected]> wrote:

Hi guys, during c/r sessions we've found that there is no way at the moment to fetch some VMA associated flags, such as mlock() and madvise(), thus the patches in this series intorduce new field into "smaps" output called VmFlags where all flags associated with the particular VMA is shown in two letter mnemonic.

Quoted 8 lines

Quoted 5 lines

Strictly speaking for c/r we only need mlock/madvise bits but it has been said that providing just a few flags looks somehow inconsistent. So all flags are here now.

Please review. Comments and complains are quite welcome!

Sigh, it's still a pretty nasty-looking interface. Better ideas are welcomed.

Maybe just a string of two-letter short names like

VmFlags: rd wr sh sr

?

I.e. in a way flags line looks in cpuinfo file.

Quoted 21 lines · Pavel Emelyanov

Quoted 9 lines · Andrew Morton

Quoted 6 lines · On Mon, 22 Oct 2012 23:14:52 +0400 Cyrill Gorcunov <[email protected]> wrote:

Hi guys, during c/r sessions we've found that there is no way at the moment to fetch some VMA associated flags, such as mlock() and madvise(), thus the patches in this series intorduce new field into "smaps" output called VmFlags where all flags associated with the particular VMA is shown in two letter mnemonic.

Quoted 8 lines

Quoted 5 lines

Strictly speaking for c/r we only need mlock/madvise bits but it has been said that providing just a few flags looks somehow inconsistent. So all flags are here now.

Please review. Comments and complains are quite welcome!

Sigh, it's still a pretty nasty-looking interface. Better ideas are welcomed.

Maybe just a string of two-letter short names like

Quoted 5 lines

VmFlags: rd wr sh sr

?

I.e. in a way flags line looks in cpuinfo file.

Yup, I think this will be shorter and more agreeable.

...

+VmFlags: rd ex mr mw me de

fs/proc/task_mmu.c +36 -0

+static void show_smap_vma_flags(struct seq_file *m, struct vm_area_struct *vma)
+{
+	/*
+	 * Don't forget to update Documentation/ on changes.
+	 */
+
+#define __seq_show_vmflag(_f, _s)		\
+	do {					\
+		if (vma->vm_flags & (_f))	\
+			seq_puts(m, _s);	\
+	} while (0)
+
+	seq_puts(m, "VmFlags: ");
+
+	__seq_show_vmflag(VM_READ,	"rd ");
+	__seq_show_vmflag(VM_WRITE,	"wr ");
+	__seq_show_vmflag(VM_EXEC,	"ex ");
+	__seq_show_vmflag(VM_SHARED,	"sh ");
+	__seq_show_vmflag(VM_MAYREAD,	"mr ");
+	__seq_show_vmflag(VM_MAYWRITE,	"mw ");
+	__seq_show_vmflag(VM_MAYEXEC,	"me ");
+	__seq_show_vmflag(VM_MAYSHARE,	"ms ");
+	__seq_show_vmflag(VM_GROWSDOWN,	"gd ");
+	__seq_show_vmflag(VM_PFNMAP,	"pf ");
+	__seq_show_vmflag(VM_DENYWRITE,	"dw ");
+	__seq_show_vmflag(VM_LOCKED,	"lo ");
+	__seq_show_vmflag(VM_IO,	"io ");
+	__seq_show_vmflag(VM_SEQ_READ,	"sr ");
+	__seq_show_vmflag(VM_RAND_READ,	"rr ");
+	__seq_show_vmflag(VM_DONTCOPY,	"dc ");
+	__seq_show_vmflag(VM_DONTEXPAND,"de ");
+	__seq_show_vmflag(VM_ACCOUNT,	"ac ");
+	__seq_show_vmflag(VM_NORESERVE,	"nr ");
+	__seq_show_vmflag(VM_HUGETLB,	"ht ");
+	__seq_show_vmflag(VM_NONLINEAR,	"nl ");
+	__seq_show_vmflag(VM_ARCH_1,	"ar ");

Quoted 10 lines

fs/proc/task_mmu.c +10 -0

+	__seq_show_vmflag(VM_DONTEXPAND,"de ");
+	__seq_show_vmflag(VM_ACCOUNT,	"ac ");
+	__seq_show_vmflag(VM_NORESERVE,	"nr ");
+	__seq_show_vmflag(VM_HUGETLB,	"ht ");
+	__seq_show_vmflag(VM_NONLINEAR,	"nl ");
+	__seq_show_vmflag(VM_ARCH_1,	"ar ");
+	__seq_show_vmflag(VM_DONTDUMP,	"dd ");
+	__seq_show_vmflag(VM_MIXEDMAP,	"mm ");
+	__seq_show_vmflag(VM_HUGEPAGE,	"hg ");
+	__seq_show_vmflag(VM_NOHUGEPAGE,"nh ");

This code would be much denser if we did the old

	static const char foo[] = "rdwrexshmrmw...";
	unsigned i;

	for (i = 0; i < BITS_PER_LONG; i++) {
		if (flags & (1 << i))
			seq_printf("%c%c ", foo[i * 2], foo[i * 2 + 1]);
	}

trick. But then we'd have to remember to fix foo[] each time we alter the flags, so maybe let's not do that.

On Mon, Oct 22, 2012 at 02:51:58PM -0700, Andrew Morton wrote:
...

Quoted 10 lines

Quoted 10 lines

fs/proc/task_mmu.c +10 -0

+	__seq_show_vmflag(VM_DONTEXPAND,"de ");
+	__seq_show_vmflag(VM_ACCOUNT,	"ac ");
+	__seq_show_vmflag(VM_NORESERVE,	"nr ");
+	__seq_show_vmflag(VM_HUGETLB,	"ht ");
+	__seq_show_vmflag(VM_NONLINEAR,	"nl ");
+	__seq_show_vmflag(VM_ARCH_1,	"ar ");
+	__seq_show_vmflag(VM_DONTDUMP,	"dd ");
+	__seq_show_vmflag(VM_MIXEDMAP,	"mm ");
+	__seq_show_vmflag(VM_HUGEPAGE,	"hg ");
+	__seq_show_vmflag(VM_NOHUGEPAGE,"nh ");

...

Quoted 11 lines

This code would be much denser if we did the old

	static const char foo[] = "rdwrexshmrmw...";
	unsigned i;

	for (i = 0; i < BITS_PER_LONG; i++) {
		if (flags & (1 << i))
			seq_printf("%c%c ", foo[i * 2], foo[i * 2 + 1]);
	}

Quoted 2 lines

trick. But then we'd have to remember to fix foo[] each time we alter the flags, so maybe let's not do that.

Yup, but not only that, this kind of trick hides associativity between VM_ constant and mnemonic, so on changes one would have to figure out which position some flag has in this foo[] array, so I vote for not use it :-)

Quoted 4 lines

Yup, but not only that, this kind of trick hides associativity between VM_ constant and mnemonic, so on changes one would have to figure out which position some flag has in this foo[] array, so I vote for not use it :-)

Well you could do

struct {
	char x[2];
} y[] = {
	[CLOG2(VM_DONTEXPAND)] =	{ 'd', 'e' },
	[CLOG2(VM_ACCOUNT)] =		{ 'a', 'c' },
	[CLOG2(VM_NORESERVE)] =		{ 'n', 'r' },
};

	...

	for (i = 0; i < BITS_PER_LONG; i++) {
		if (flags & (1 << i))
			seq_printf("%c%c ", y[i][0], y[i][1]);
	}

where CLOG2() is extracted from the guts of ilog2().

I'll stop now :)

On Mon, Oct 22, 2012 at 11:30:25PM -0700, Andrew Morton wrote:
...

Quoted 22 lines

Quoted 4 lines

Yup, but not only that, this kind of trick hides associativity between VM_ constant and mnemonic, so on changes one would have to figure out which position some flag has in this foo[] array, so I vote for not use it :-)

Well you could do

struct {
	char x[2];
} y[] = {
	[CLOG2(VM_DONTEXPAND)] =	{ 'd', 'e' },
	[CLOG2(VM_ACCOUNT)] =		{ 'a', 'c' },
	[CLOG2(VM_NORESERVE)] =		{ 'n', 'r' },
};

	...

	for (i = 0; i < BITS_PER_LONG; i++) {
		if (flags & (1 << i))
			seq_printf("%c%c ", y[i][0], y[i][1]);
	}

Quoted 3 lines

where CLOG2() is extracted from the guts of ilog2().

I'll stop now :)

Yup, this one will be a wy better. Letme try it out :)

Quoted 24 lines · Cyrill Gorcunov

On Mon, Oct 22, 2012 at 11:30:25PM -0700, Andrew Morton wrote:
...

Quoted 22 lines

Quoted 4 lines

Yup, but not only that, this kind of trick hides associativity between VM_ constant and mnemonic, so on changes one would have to figure out which position some flag has in this foo[] array, so I vote for not use it :-)

Well you could do

struct {
	char x[2];
} y[] = {
	[CLOG2(VM_DONTEXPAND)] =	{ 'd', 'e' },
	[CLOG2(VM_ACCOUNT)] =		{ 'a', 'c' },
	[CLOG2(VM_NORESERVE)] =		{ 'n', 'r' },
};

	...

	for (i = 0; i < BITS_PER_LONG; i++) {
		if (flags & (1 << i))
			seq_printf("%c%c ", y[i][0], y[i][1]);
	}

Quoted 5 lines

Quoted 3 lines

where CLOG2() is extracted from the guts of ilog2().

I'll stop now :)

Yup, this one will be a wy better. Letme try it out :)

ilog2 works well enough here as well.
---
From: Cyrill Gorcunov <[email protected]>
Subject: procfs: add VmFlags field in smaps output v3

During c/r sessions we've found that there is no way at the moment to fetch some VMA associated flags, such as mlock() and madvise().

This leads us to a problem -- we don't know if we should call for mlock() and/or madvise() after restore on the vma area we're bringing back to life.

This patch intorduces a new field into "smaps" output called VmFlags, where all set flags associated with the particular VMA is shown as two letter mnemonics.

[ Strictly speaking for c/r we only need mlock/madvise bits but it has been
said that providing just a few flags looks somehow inconsistent. So all flags are here now. ]

This feature is made available on CONFIG_CHECKPOINT_RESTORE=n kernels, as other applications may start to use these fields.

The data is encoded in a somewhat awkward two letters mnemonic form, to encourage userspace to be prepared for fields being added or removed in the future.

Quoted 5 lines

The data is encoded in a somewhat awkward two letters mnemonic form, to encourage userspace to be prepared for fields being added or removed in the future.

Wow. This version generates 1k less kernel bloat than v2! Gee, and I only sent that email as a late-night joke ;)

fs/proc/task_mmu.o with neither patch:
text data bss dec hex filename
14849 112 5312 20273 4f31 fs/proc/task_mmu.o

fs/proc/task_mmu.o with the v2 patch:
16074 112 5776 21962 55ca fs/proc/task_mmu.o

fs/proc/task_mmu.o with the v3 patch:
15446 112 5368 20926 51be fs/proc/task_mmu.o

fs/proc/task_mmu.o with the v3 patch and the below fix:
15123 112 5352 20587 506b fs/proc/task_mmu.o

So the delta has gone from 1700 bytes down to 300. Seems that it pays to be anal about these things ;)

Don't forget the `static'! Without it, the compiler will need to

construct the array as a temporary on the stack each time the function is called - it's just terrible. (There's no reason why the compiler can't insert the static for us as an optimisation, and I think later gcc's may have got smarter about this).

Was there a reason why you added the ".l = " to the initialiser? My gcc is happy without it.

Also... what happens if there's an unrecognised bit set in `flags'? Memory corruption or code skew could cause this. We emit a couple of NULs into the procfs output, which I guess is an OK response to such a condition.

From: Andrew Morton <[email protected]>
Subject: procfs-add-vmflags-field-in-smaps-output-v3-fix

make mnemonics[] static, remove unneeded init code, tidy whitespace

Quoted 5 lines

Also... what happens if there's an unrecognised bit set in `flags'? Memory corruption or code skew could cause this. We emit a couple of NULs into the procfs output, which I guess is an OK response to such a condition.

That's indeed will produce some nil character on the screen. I guess we need some designited init here, say

	[0 ... (BITS_PER_LONG-1)] = { {'-', '-'} },
	...
	[ilog2(VM_READ)]	= { {'r', 'd'} },
	...
	and so on, i'll update on top

                [ilog2(VM_WRITE)]       = { {'w', 'r'} },

Quoted 1 line · Cyrill Gorcunov

                [ilog2(VM_WRITE)]       = { {'w', 'r'} },

since we're being awfully positive about crazy late night ideas, how about something like:

#define MNEM(_VM, _mn) [ilog2(_VM)] = {(const char [2]){_mn}}

	MNEM(VM_WRITE, "wr"),

Quoted 1 line · Cyrill Gorcunov

                [ilog2(VM_WRITE)]       = { {'w', 'r'} },

since we're being awfully positive about crazy late night ideas, how about something like:

#define MNEM(_VM, _mn) [ilog2(_VM)] = {(const char [2]){_mn}}

	MNEM(VM_WRITE, "wr"),