On Tue, 23 Oct 2012 11:15:49 +0400
Cyrill Gorcunov <gorcu...@openvz.org> wrote:

> On Tue, Oct 23, 2012 at 10:34:30AM +0400, Cyrill Gorcunov wrote:
> > On Mon, Oct 22, 2012 at 11:30:25PM -0700, Andrew Morton wrote:
> > ...
> > > > 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 :)
> > 
> > Yup, this one will be a wy better. Letme try it out :)
> 
> ilog2 works well enough here as well.
> ---
> From: Cyrill Gorcunov <gorcu...@openvz.org>
> 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.
> 

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 <a...@linux-foundation.org>
Subject: procfs-add-vmflags-field-in-smaps-output-v3-fix

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

Cc: Cyrill Gorcunov <gorcu...@openvz.org>
Cc: Pavel Emelyanov <xe...@parallels.com>
Cc: Peter Zijlstra <a.p.zijls...@chello.nl>
Signed-off-by: Andrew Morton <a...@linux-foundation.org>
---

 fs/proc/task_mmu.c |   58 ++++++++++++++++++++-----------------------
 1 file changed, 28 insertions(+), 30 deletions(-)

diff -puN 
Documentation/filesystems/proc.txt~procfs-add-vmflags-field-in-smaps-output-v3-fix
 Documentation/filesystems/proc.txt
diff -puN fs/proc/task_mmu.c~procfs-add-vmflags-field-in-smaps-output-v3-fix 
fs/proc/task_mmu.c
--- a/fs/proc/task_mmu.c~procfs-add-vmflags-field-in-smaps-output-v3-fix
+++ a/fs/proc/task_mmu.c
@@ -531,39 +531,37 @@ static void show_smap_vma_flags(struct s
        /*
         * Don't forget to update Documentation/ on changes.
         */
-
-       const struct {
+       static const struct {
                const char l[2];
        } mnemonics[BITS_PER_LONG] = {
-               [ilog2(VM_READ)]        = { .l = {'r', 'd'} },
-               [ilog2(VM_WRITE)]       = { .l = {'w', 'r'} },
-               [ilog2(VM_EXEC)]        = { .l = {'e', 'x'} },
-               [ilog2(VM_SHARED)]      = { .l = {'s', 'h'} },
-               [ilog2(VM_MAYREAD)]     = { .l = {'m', 'r'} },
-               [ilog2(VM_MAYWRITE)]    = { .l = {'m', 'w'} },
-               [ilog2(VM_MAYEXEC)]     = { .l = {'m', 'e'} },
-               [ilog2(VM_MAYSHARE)]    = { .l = {'m', 's'} },
-               [ilog2(VM_GROWSDOWN)]   = { .l = {'g', 'd'} },
-               [ilog2(VM_PFNMAP)]      = { .l = {'p', 'f'} },
-               [ilog2(VM_DENYWRITE)]   = { .l = {'d', 'w'} },
-               [ilog2(VM_LOCKED)]      = { .l = {'l', 'o'} },
-               [ilog2(VM_IO)]          = { .l = {'i', 'o'} },
-               [ilog2(VM_SEQ_READ)]    = { .l = {'s', 'r'} },
-               [ilog2(VM_RAND_READ)]   = { .l = {'r', 'r'} },
-               [ilog2(VM_DONTCOPY)]    = { .l = {'d', 'c'} },
-               [ilog2(VM_DONTEXPAND)]  = { .l = {'d', 'e'} },
-               [ilog2(VM_ACCOUNT)]     = { .l = {'a', 'c'} },
-               [ilog2(VM_NORESERVE)]   = { .l = {'n', 'r'} },
-               [ilog2(VM_HUGETLB)]     = { .l = {'h', 't'} },
-               [ilog2(VM_NONLINEAR)]   = { .l = {'n', 'l'} },
-               [ilog2(VM_ARCH_1)]      = { .l = {'a', 'r'} },
-               [ilog2(VM_DONTDUMP)]    = { .l = {'d', 'd'} },
-               [ilog2(VM_MIXEDMAP)]    = { .l = {'m', 'm'} },
-               [ilog2(VM_HUGEPAGE)]    = { .l = {'h', 'g'} },
-               [ilog2(VM_NOHUGEPAGE)]  = { .l = {'n', 'h'} },
-               [ilog2(VM_MERGEABLE)]   = { .l = {'m', 'g'} },
+               [ilog2(VM_READ)]        = { {'r', 'd'} },
+               [ilog2(VM_WRITE)]       = { {'w', 'r'} },
+               [ilog2(VM_EXEC)]        = { {'e', 'x'} },
+               [ilog2(VM_SHARED)]      = { {'s', 'h'} },
+               [ilog2(VM_MAYREAD)]     = { {'m', 'r'} },
+               [ilog2(VM_MAYWRITE)]    = { {'m', 'w'} },
+               [ilog2(VM_MAYEXEC)]     = { {'m', 'e'} },
+               [ilog2(VM_MAYSHARE)]    = { {'m', 's'} },
+               [ilog2(VM_GROWSDOWN)]   = { {'g', 'd'} },
+               [ilog2(VM_PFNMAP)]      = { {'p', 'f'} },
+               [ilog2(VM_DENYWRITE)]   = { {'d', 'w'} },
+               [ilog2(VM_LOCKED)]      = { {'l', 'o'} },
+               [ilog2(VM_IO)]          = { {'i', 'o'} },
+               [ilog2(VM_SEQ_READ)]    = { {'s', 'r'} },
+               [ilog2(VM_RAND_READ)]   = { {'r', 'r'} },
+               [ilog2(VM_DONTCOPY)]    = { {'d', 'c'} },
+               [ilog2(VM_DONTEXPAND)]  = { {'d', 'e'} },
+               [ilog2(VM_ACCOUNT)]     = { {'a', 'c'} },
+               [ilog2(VM_NORESERVE)]   = { {'n', 'r'} },
+               [ilog2(VM_HUGETLB)]     = { {'h', 't'} },
+               [ilog2(VM_NONLINEAR)]   = { {'n', 'l'} },
+               [ilog2(VM_ARCH_1)]      = { {'a', 'r'} },
+               [ilog2(VM_DONTDUMP)]    = { {'d', 'd'} },
+               [ilog2(VM_MIXEDMAP)]    = { {'m', 'm'} },
+               [ilog2(VM_HUGEPAGE)]    = { {'h', 'g'} },
+               [ilog2(VM_NOHUGEPAGE)]  = { {'n', 'h'} },
+               [ilog2(VM_MERGEABLE)]   = { {'m', 'g'} },
        };
-
        size_t i;
 
        seq_puts(m, "VmFlags: ");
_

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to