Re: + procfs-add-vmflags-field-in-smaps-output-v3-fix-2.patch added to -mm tree
On Wed, Oct 24, 2012 at 09:26:20PM +1100, Stephen Rothwell wrote: > > Hi Stephen, could you please elaborate, which bad usage hiding could be > > there? > > I though it might suppress a warning if someone added an entry with a >2 > character string, but testing shows that it does not, so that is OK. > There is still no need for the cast, though, or the second set of > braces. The C standard allows fixed length char arrays to be initialised > by string constants and discards the trailing NUL if necessary. OK, fair enough. I'll post a new patch with all concerns fixed in one pass. Thanks all for comments! -- 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/
Re: + procfs-add-vmflags-field-in-smaps-output-v3-fix-2.patch added to -mm tree
Hi Cyrill,
On Wed, 24 Oct 2012 13:59:59 +0400 Cyrill Gorcunov wrote:
>
> On Wed, Oct 24, 2012 at 08:47:46PM +1100, Stephen Rothwell wrote:
> >
> > On Wed, 24 Oct 2012 12:45:15 +0400 Cyrill Gorcunov
> > wrote:
> > >
> > > static void show_smap_vma_flags(struct seq_file *m, struct
> > > vm_area_struct *vma)
> > > {
> > > +#define __VM_FLAG(_f, _s)[ilog2(_f)] = {(const char [2]){_s}}
> >
> > I really don't think you need the cast (and it may hide bad usages) or
> > the second set of braces. Thus:
>
> Hi Stephen, could you please elaborate, which bad usage hiding could be there?
I though it might suppress a warning if someone added an entry with a >2
character string, but testing shows that it does not, so that is OK.
There is still no need for the cast, though, or the second set of
braces. The C standard allows fixed length char arrays to be initialised
by string constants and discards the trailing NUL if necessary.
> > Further is there any reason for the struct?
> >
> > #define __VM_FLAG(_f, _s) [ilog2(_f)] = _s
> >
> > static const char mnemonics[BITS_PER_LONG][2] = {
> > ...
> > };
>
> Well, good point, though the benefit of using stucture here
> could be easier way for extension if needed. The compiled result
> is the same as for plain array or structure, so I would rather
> stick with struct here, until contrary proved (I mean it's not
> a problem to update the patch and use array here but I still
> think struct it better in long term). If you guys think that I
> should move it to array -- no problem, i may update ;)
One thing we learn about early (especially working at IBM) is to resist
the temptation to over engineer :-)
--
Cheers,
Stephen Rothwells...@canb.auug.org.au
pgpMGQdLwCSGF.pgp
Description: PGP signature
Re: + procfs-add-vmflags-field-in-smaps-output-v3-fix-2.patch added to -mm tree
On Wed, Oct 24, 2012 at 08:47:46PM +1100, Stephen Rothwell wrote:
> Hi Cyrill,
>
> On Wed, 24 Oct 2012 12:45:15 +0400 Cyrill Gorcunov
> wrote:
> >
> > static void show_smap_vma_flags(struct seq_file *m, struct vm_area_struct
> > *vma)
> > {
> > +#define __VM_FLAG(_f, _s) [ilog2(_f)] = {(const char [2]){_s}}
>
> I really don't think you need the cast (and it may hide bad usages) or
> the second set of braces. Thus:
Hi Stephen, could you please elaborate, which bad usage hiding could be there?
> Further is there any reason for the struct?
>
> #define __VM_FLAG(_f, _s) [ilog2(_f)] = _s
>
> static const char mnemonics[BITS_PER_LONG][2] = {
> ...
> };
Well, good point, though the benefit of using stucture here
could be easier way for extension if needed. The compiled result
is the same as for plain array or structure, so I would rather
stick with struct here, until contrary proved (I mean it's not
a problem to update the patch and use array here but I still
think struct it better in long term). If you guys think that I
should move it to array -- no problem, i may update ;)
> > seq_puts(m, "VmFlags: ");
> > for (i = 0; i < BITS_PER_LONG; i++) {
> > - if (vma->vm_flags & (1 << i))
> > + if (vma->vm_flags & (1ul << i)) {
> > seq_printf(m, "%c%c ",
> >mnemonics[i].l[0],
> >mnemonics[i].l[1]);
>
> mnemonics[i][0], mnemonics[i][1]
>
> /me looks for another bike shed :-)
Heh :)
--
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/
Re: + procfs-add-vmflags-field-in-smaps-output-v3-fix-2.patch added to -mm tree
On Wed, Oct 24, 2012 at 11:24:04AM +0200, Peter Zijlstra wrote:
> On Wed, 2012-10-24 at 12:45 +0400, Cyrill Gorcunov wrote:
> > for (i = 0; i < BITS_PER_LONG; i++) {
> > - if (vma->vm_flags & (1 << i))
> > + if (vma->vm_flags & (1ul << i)) {
>
> for_each_set_bit(i, >vm_flags, BITS_PER_LONG) {
Yeah, thanks (for some reason forgot that we have such helper)
---
From: Cyrill Gorcunov
Subject: procfs-add-vmflags-field-in-smaps-output-v3-fix-on-top-3
[by a.p.zijlstra@] make assignment more readable and use for_each_set_bit
Signed-off-by: Cyrill Gorcunov
Cc: Pavel Emelyanov
Cc: Peter Zijlstra
Cc: Stephen Rothwell
Cc: Andrew Morton
---
fs/proc/task_mmu.c | 69 +++--
1 file changed, 36 insertions(+), 33 deletions(-)
Index: linux-2.6.git/fs/proc/task_mmu.c
===
--- linux-2.6.git.orig/fs/proc/task_mmu.c
+++ linux-2.6.git/fs/proc/task_mmu.c
@@ -482,6 +482,8 @@ static int smaps_pte_range(pmd_t *pmd, u
static void show_smap_vma_flags(struct seq_file *m, struct vm_area_struct *vma)
{
+#define __VM_FLAG(_f, _s) [ilog2(_f)] = {(const char [2]){_s}}
+
/*
* Don't forget to update Documentation/ on changes.
*/
@@ -491,46 +493,47 @@ static void show_smap_vma_flags(struct s
/*
* In case if we meet a flag we don't know about.
*/
- [0 ... (BITS_PER_LONG-1)] = { {'?', '?'} },
+ [0 ... (BITS_PER_LONG-1)] = { (const char [2]){"??"} },
- [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'} },
+ __VM_FLAG(VM_READ, "rd"),
+ __VM_FLAG(VM_WRITE, "wr"),
+ __VM_FLAG(VM_EXEC, "ex"),
+ __VM_FLAG(VM_SHARED,"sh"),
+ __VM_FLAG(VM_MAYREAD, "mr"),
+ __VM_FLAG(VM_MAYWRITE, "mw"),
+ __VM_FLAG(VM_MAYEXEC, "me"),
+ __VM_FLAG(VM_MAYSHARE, "ms"),
+ __VM_FLAG(VM_GROWSDOWN, "gd"),
+ __VM_FLAG(VM_PFNMAP,"pf"),
+ __VM_FLAG(VM_DENYWRITE, "dw"),
+ __VM_FLAG(VM_LOCKED,"lo"),
+ __VM_FLAG(VM_IO,"io"),
+ __VM_FLAG(VM_SEQ_READ, "sr"),
+ __VM_FLAG(VM_RAND_READ, "rr"),
+ __VM_FLAG(VM_DONTCOPY, "dc"),
+ __VM_FLAG(VM_DONTEXPAND,"de"),
+ __VM_FLAG(VM_ACCOUNT, "ac"),
+ __VM_FLAG(VM_NORESERVE, "nr"),
+ __VM_FLAG(VM_HUGETLB, "ht"),
+ __VM_FLAG(VM_NONLINEAR, "nl"),
+ __VM_FLAG(VM_ARCH_1,"ar"),
+ __VM_FLAG(VM_DONTDUMP, "dd"),
+ __VM_FLAG(VM_MIXEDMAP, "mm"),
+ __VM_FLAG(VM_HUGEPAGE, "hg"),
+ __VM_FLAG(VM_NOHUGEPAGE,"nh"),
+ __VM_FLAG(VM_MERGEABLE, "mg"),
};
size_t i;
seq_puts(m, "VmFlags: ");
- for (i = 0; i < BITS_PER_LONG; i++) {
- if (vma->vm_flags & (1 << i))
- seq_printf(m, "%c%c ",
- mnemonics[i].l[0],
- mnemonics[i].l[1]);
+ for_each_set_bit(i, >vm_flags, BITS_PER_LONG) {
+ seq_printf(m, "%c%c ",
+ mnemonics[i].l[0],
+ mnemonics[i].l[1]);
}
seq_putc(m, '\n');
+
+#undef
Re: + procfs-add-vmflags-field-in-smaps-output-v3-fix-2.patch added to -mm tree
Hi Cyrill,
On Wed, 24 Oct 2012 12:45:15 +0400 Cyrill Gorcunov wrote:
>
> static void show_smap_vma_flags(struct seq_file *m, struct vm_area_struct
> *vma)
> {
> +#define __VM_FLAG(_f, _s)[ilog2(_f)] = {(const char [2]){_s}}
I really don't think you need the cast (and it may hide bad usages) or
the second set of braces. Thus:
#define __VM_FLAG(_f, _s) [ilog2(_f)] = {_s}
> +
> /*
>* Don't forget to update Documentation/ on changes.
>*/
> @@ -491,46 +493,49 @@ static void show_smap_vma_flags(struct s
> /*
>* In case if we meet a flag we don't know about.
>*/
> - [0 ... (BITS_PER_LONG-1)] = { {'?', '?'} },
> + [0 ... (BITS_PER_LONG-1)] = { (const char [2]){"??"} },
And here.
Further is there any reason for the struct?
#define __VM_FLAG(_f, _s) [ilog2(_f)] = _s
static const char mnemonics[BITS_PER_LONG][2] = {
...
};
> seq_puts(m, "VmFlags: ");
> for (i = 0; i < BITS_PER_LONG; i++) {
> - if (vma->vm_flags & (1 << i))
> + if (vma->vm_flags & (1ul << i)) {
> seq_printf(m, "%c%c ",
> mnemonics[i].l[0],
> mnemonics[i].l[1]);
mnemonics[i][0], mnemonics[i][1]
/me looks for another bike shed :-)
--
Cheers,
Stephen Rothwells...@canb.auug.org.au
pgpx1r5tq56sm.pgp
Description: PGP signature
Re: + procfs-add-vmflags-field-in-smaps-output-v3-fix-2.patch added to -mm tree
On Wed, 2012-10-24 at 12:45 +0400, Cyrill Gorcunov wrote:
> for (i = 0; i < BITS_PER_LONG; i++) {
> - if (vma->vm_flags & (1 << i))
> + if (vma->vm_flags & (1ul << i)) {
for_each_set_bit(i, >vm_flags, BITS_PER_LONG) {
> seq_printf(m, "%c%c ",
>mnemonics[i].l[0],
>mnemonics[i].l[1]);
> + }
> }
--
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/
Re: + procfs-add-vmflags-field-in-smaps-output-v3-fix-2.patch added to -mm tree
On Tue, Oct 23, 2012 at 03:02:59PM -0700, a...@linux-foundation.org wrote:
>
> The patch titled
> Subject: procfs-add-vmflags-field-in-smaps-output-v3-fix-2
> has been added to the -mm tree. Its filename is
> procfs-add-vmflags-field-in-smaps-output-v3-fix-2.patch
>
> --
> From: Cyrill Gorcunov
> Subject: procfs-add-vmflags-field-in-smaps-output-v3-fix-2
>
> Use designated init to assign "??" mnemonic on unknown flags.
>
> Signed-off-by: Cyrill Gorcunov
> Cc: Pavel Emelyanov
> Cc: Peter Zijlstra
> Signed-off-by: Andrew Morton
Hi Andrew, could you stack the patch below on top, this one
should be a final for sure ;-)
---
From: Cyrill Gorcunov
Subject: procfs-add-vmflags-field-in-smaps-output-v3-fix-on-top-2
Make assignment more readable [by peterz@] and fix bit
tests (we need unsigned long here explicitly stated).
Signed-off-by: Cyrill Gorcunov
Cc: Pavel Emelyanov
Cc: Peter Zijlstra
Cc: Stephen Rothwell
Cc: Andrew Morton
---
fs/proc/task_mmu.c | 63 -
1 file changed, 34 insertions(+), 29 deletions(-)
Index: linux-2.6.git/fs/proc/task_mmu.c
===
--- linux-2.6.git.orig/fs/proc/task_mmu.c
+++ linux-2.6.git/fs/proc/task_mmu.c
@@ -482,6 +482,8 @@ static int smaps_pte_range(pmd_t *pmd, u
static void show_smap_vma_flags(struct seq_file *m, struct vm_area_struct *vma)
{
+#define __VM_FLAG(_f, _s) [ilog2(_f)] = {(const char [2]){_s}}
+
/*
* Don't forget to update Documentation/ on changes.
*/
@@ -491,46 +493,49 @@ static void show_smap_vma_flags(struct s
/*
* In case if we meet a flag we don't know about.
*/
- [0 ... (BITS_PER_LONG-1)] = { {'?', '?'} },
+ [0 ... (BITS_PER_LONG-1)] = { (const char [2]){"??"} },
- [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'} },
+ __VM_FLAG(VM_READ, "rd"),
+ __VM_FLAG(VM_WRITE, "wr"),
+ __VM_FLAG(VM_EXEC, "ex"),
+ __VM_FLAG(VM_SHARED,"sh"),
+ __VM_FLAG(VM_MAYREAD, "mr"),
+ __VM_FLAG(VM_MAYWRITE, "mw"),
+ __VM_FLAG(VM_MAYEXEC, "me"),
+ __VM_FLAG(VM_MAYSHARE, "ms"),
+ __VM_FLAG(VM_GROWSDOWN, "gd"),
+ __VM_FLAG(VM_PFNMAP,"pf"),
+ __VM_FLAG(VM_DENYWRITE, "dw"),
+ __VM_FLAG(VM_LOCKED,"lo"),
+ __VM_FLAG(VM_IO,"io"),
+ __VM_FLAG(VM_SEQ_READ, "sr"),
+ __VM_FLAG(VM_RAND_READ, "rr"),
+ __VM_FLAG(VM_DONTCOPY, "dc"),
+ __VM_FLAG(VM_DONTEXPAND,"de"),
+ __VM_FLAG(VM_ACCOUNT, "ac"),
+ __VM_FLAG(VM_NORESERVE, "nr"),
+ __VM_FLAG(VM_HUGETLB, "ht"),
+ __VM_FLAG(VM_NONLINEAR, "nl"),
+ __VM_FLAG(VM_ARCH_1,"ar"),
+ __VM_FLAG(VM_DONTDUMP, "dd"),
+ __VM_FLAG(VM_MIXEDMAP, "mm"),
+ __VM_FLAG(VM_HUGEPAGE, "hg"),
+ __VM_FLAG(VM_NOHUGEPAGE,"nh"),
+ __VM_FLAG(VM_MERGEABLE, "mg"),
};
size_t i;
seq_puts(m, "VmFlags: ");
for (i = 0; i < BITS_PER_LONG; i++) {
- if (vma->vm_flags & (1 << i))
+ if (vma->vm_flags & (1ul << i)) {
Re: + procfs-add-vmflags-field-in-smaps-output-v3-fix-2.patch added to -mm tree
On Tue, Oct 23, 2012 at 03:02:59PM -0700, a...@linux-foundation.org wrote:
The patch titled
Subject: procfs-add-vmflags-field-in-smaps-output-v3-fix-2
has been added to the -mm tree. Its filename is
procfs-add-vmflags-field-in-smaps-output-v3-fix-2.patch
--
From: Cyrill Gorcunov gorcu...@openvz.org
Subject: procfs-add-vmflags-field-in-smaps-output-v3-fix-2
Use designated init to assign ?? mnemonic on unknown flags.
Signed-off-by: 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
Hi Andrew, could you stack the patch below on top, this one
should be a final for sure ;-)
---
From: Cyrill Gorcunov gorcu...@openvz.org
Subject: procfs-add-vmflags-field-in-smaps-output-v3-fix-on-top-2
Make assignment more readable [by peterz@] and fix bit
tests (we need unsigned long here explicitly stated).
Signed-off-by: Cyrill Gorcunov gorcu...@openvz.org
Cc: Pavel Emelyanov xe...@parallels.com
Cc: Peter Zijlstra a.p.zijls...@chello.nl
Cc: Stephen Rothwell s...@canb.auug.org.au
Cc: Andrew Morton a...@linux-foundation.org
---
fs/proc/task_mmu.c | 63 -
1 file changed, 34 insertions(+), 29 deletions(-)
Index: linux-2.6.git/fs/proc/task_mmu.c
===
--- linux-2.6.git.orig/fs/proc/task_mmu.c
+++ linux-2.6.git/fs/proc/task_mmu.c
@@ -482,6 +482,8 @@ static int smaps_pte_range(pmd_t *pmd, u
static void show_smap_vma_flags(struct seq_file *m, struct vm_area_struct *vma)
{
+#define __VM_FLAG(_f, _s) [ilog2(_f)] = {(const char [2]){_s}}
+
/*
* Don't forget to update Documentation/ on changes.
*/
@@ -491,46 +493,49 @@ static void show_smap_vma_flags(struct s
/*
* In case if we meet a flag we don't know about.
*/
- [0 ... (BITS_PER_LONG-1)] = { {'?', '?'} },
+ [0 ... (BITS_PER_LONG-1)] = { (const char [2]){??} },
- [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'} },
+ __VM_FLAG(VM_READ, rd),
+ __VM_FLAG(VM_WRITE, wr),
+ __VM_FLAG(VM_EXEC, ex),
+ __VM_FLAG(VM_SHARED,sh),
+ __VM_FLAG(VM_MAYREAD, mr),
+ __VM_FLAG(VM_MAYWRITE, mw),
+ __VM_FLAG(VM_MAYEXEC, me),
+ __VM_FLAG(VM_MAYSHARE, ms),
+ __VM_FLAG(VM_GROWSDOWN, gd),
+ __VM_FLAG(VM_PFNMAP,pf),
+ __VM_FLAG(VM_DENYWRITE, dw),
+ __VM_FLAG(VM_LOCKED,lo),
+ __VM_FLAG(VM_IO,io),
+ __VM_FLAG(VM_SEQ_READ, sr),
+ __VM_FLAG(VM_RAND_READ, rr),
+ __VM_FLAG(VM_DONTCOPY, dc),
+ __VM_FLAG(VM_DONTEXPAND,de),
+ __VM_FLAG(VM_ACCOUNT, ac),
+ __VM_FLAG(VM_NORESERVE, nr),
+ __VM_FLAG(VM_HUGETLB, ht),
+ __VM_FLAG(VM_NONLINEAR, nl),
+ __VM_FLAG(VM_ARCH_1,ar),
+ __VM_FLAG(VM_DONTDUMP, dd),
+ __VM_FLAG(VM_MIXEDMAP, mm),
+ __VM_FLAG(VM_HUGEPAGE, hg),
+ __VM_FLAG(VM_NOHUGEPAGE,nh),
+ __VM_FLAG(VM_MERGEABLE, mg),
};
size_t i;
seq_puts(m, VmFlags: );
Re: + procfs-add-vmflags-field-in-smaps-output-v3-fix-2.patch added to -mm tree
On Wed, 2012-10-24 at 12:45 +0400, Cyrill Gorcunov wrote:
for (i = 0; i BITS_PER_LONG; i++) {
- if (vma-vm_flags (1 i))
+ if (vma-vm_flags (1ul i)) {
for_each_set_bit(i, vma-vm_flags, BITS_PER_LONG) {
seq_printf(m, %c%c ,
mnemonics[i].l[0],
mnemonics[i].l[1]);
+ }
}
--
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/
Re: + procfs-add-vmflags-field-in-smaps-output-v3-fix-2.patch added to -mm tree
Hi Cyrill,
On Wed, 24 Oct 2012 12:45:15 +0400 Cyrill Gorcunov gorcu...@openvz.org wrote:
static void show_smap_vma_flags(struct seq_file *m, struct vm_area_struct
*vma)
{
+#define __VM_FLAG(_f, _s)[ilog2(_f)] = {(const char [2]){_s}}
I really don't think you need the cast (and it may hide bad usages) or
the second set of braces. Thus:
#define __VM_FLAG(_f, _s) [ilog2(_f)] = {_s}
+
/*
* Don't forget to update Documentation/ on changes.
*/
@@ -491,46 +493,49 @@ static void show_smap_vma_flags(struct s
/*
* In case if we meet a flag we don't know about.
*/
- [0 ... (BITS_PER_LONG-1)] = { {'?', '?'} },
+ [0 ... (BITS_PER_LONG-1)] = { (const char [2]){??} },
And here.
Further is there any reason for the struct?
#define __VM_FLAG(_f, _s) [ilog2(_f)] = _s
static const char mnemonics[BITS_PER_LONG][2] = {
...
};
seq_puts(m, VmFlags: );
for (i = 0; i BITS_PER_LONG; i++) {
- if (vma-vm_flags (1 i))
+ if (vma-vm_flags (1ul i)) {
seq_printf(m, %c%c ,
mnemonics[i].l[0],
mnemonics[i].l[1]);
mnemonics[i][0], mnemonics[i][1]
/me looks for another bike shed :-)
--
Cheers,
Stephen Rothwells...@canb.auug.org.au
pgpx1r5tq56sm.pgp
Description: PGP signature
Re: + procfs-add-vmflags-field-in-smaps-output-v3-fix-2.patch added to -mm tree
On Wed, Oct 24, 2012 at 11:24:04AM +0200, Peter Zijlstra wrote:
On Wed, 2012-10-24 at 12:45 +0400, Cyrill Gorcunov wrote:
for (i = 0; i BITS_PER_LONG; i++) {
- if (vma-vm_flags (1 i))
+ if (vma-vm_flags (1ul i)) {
for_each_set_bit(i, vma-vm_flags, BITS_PER_LONG) {
Yeah, thanks (for some reason forgot that we have such helper)
---
From: Cyrill Gorcunov gorcu...@openvz.org
Subject: procfs-add-vmflags-field-in-smaps-output-v3-fix-on-top-3
[by a.p.zijlstra@] make assignment more readable and use for_each_set_bit
Signed-off-by: Cyrill Gorcunov gorcu...@openvz.org
Cc: Pavel Emelyanov xe...@parallels.com
Cc: Peter Zijlstra a.p.zijls...@chello.nl
Cc: Stephen Rothwell s...@canb.auug.org.au
Cc: Andrew Morton a...@linux-foundation.org
---
fs/proc/task_mmu.c | 69 +++--
1 file changed, 36 insertions(+), 33 deletions(-)
Index: linux-2.6.git/fs/proc/task_mmu.c
===
--- linux-2.6.git.orig/fs/proc/task_mmu.c
+++ linux-2.6.git/fs/proc/task_mmu.c
@@ -482,6 +482,8 @@ static int smaps_pte_range(pmd_t *pmd, u
static void show_smap_vma_flags(struct seq_file *m, struct vm_area_struct *vma)
{
+#define __VM_FLAG(_f, _s) [ilog2(_f)] = {(const char [2]){_s}}
+
/*
* Don't forget to update Documentation/ on changes.
*/
@@ -491,46 +493,47 @@ static void show_smap_vma_flags(struct s
/*
* In case if we meet a flag we don't know about.
*/
- [0 ... (BITS_PER_LONG-1)] = { {'?', '?'} },
+ [0 ... (BITS_PER_LONG-1)] = { (const char [2]){??} },
- [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'} },
+ __VM_FLAG(VM_READ, rd),
+ __VM_FLAG(VM_WRITE, wr),
+ __VM_FLAG(VM_EXEC, ex),
+ __VM_FLAG(VM_SHARED,sh),
+ __VM_FLAG(VM_MAYREAD, mr),
+ __VM_FLAG(VM_MAYWRITE, mw),
+ __VM_FLAG(VM_MAYEXEC, me),
+ __VM_FLAG(VM_MAYSHARE, ms),
+ __VM_FLAG(VM_GROWSDOWN, gd),
+ __VM_FLAG(VM_PFNMAP,pf),
+ __VM_FLAG(VM_DENYWRITE, dw),
+ __VM_FLAG(VM_LOCKED,lo),
+ __VM_FLAG(VM_IO,io),
+ __VM_FLAG(VM_SEQ_READ, sr),
+ __VM_FLAG(VM_RAND_READ, rr),
+ __VM_FLAG(VM_DONTCOPY, dc),
+ __VM_FLAG(VM_DONTEXPAND,de),
+ __VM_FLAG(VM_ACCOUNT, ac),
+ __VM_FLAG(VM_NORESERVE, nr),
+ __VM_FLAG(VM_HUGETLB, ht),
+ __VM_FLAG(VM_NONLINEAR, nl),
+ __VM_FLAG(VM_ARCH_1,ar),
+ __VM_FLAG(VM_DONTDUMP, dd),
+ __VM_FLAG(VM_MIXEDMAP, mm),
+ __VM_FLAG(VM_HUGEPAGE, hg),
+ __VM_FLAG(VM_NOHUGEPAGE,nh),
+ __VM_FLAG(VM_MERGEABLE, mg),
};
size_t i;
seq_puts(m, VmFlags: );
- for (i = 0; i BITS_PER_LONG; i++) {
- if (vma-vm_flags (1 i))
- seq_printf(m, %c%c ,
- mnemonics[i].l[0],
- mnemonics[i].l[1]);
+ for_each_set_bit(i, vma-vm_flags, BITS_PER_LONG) {
+ seq_printf(m, %c%c ,
+ mnemonics[i].l[0],
+ mnemonics[i].l[1]);
Re: + procfs-add-vmflags-field-in-smaps-output-v3-fix-2.patch added to -mm tree
On Wed, Oct 24, 2012 at 08:47:46PM +1100, Stephen Rothwell wrote:
Hi Cyrill,
On Wed, 24 Oct 2012 12:45:15 +0400 Cyrill Gorcunov gorcu...@openvz.org
wrote:
static void show_smap_vma_flags(struct seq_file *m, struct vm_area_struct
*vma)
{
+#define __VM_FLAG(_f, _s) [ilog2(_f)] = {(const char [2]){_s}}
I really don't think you need the cast (and it may hide bad usages) or
the second set of braces. Thus:
Hi Stephen, could you please elaborate, which bad usage hiding could be there?
Further is there any reason for the struct?
#define __VM_FLAG(_f, _s) [ilog2(_f)] = _s
static const char mnemonics[BITS_PER_LONG][2] = {
...
};
Well, good point, though the benefit of using stucture here
could be easier way for extension if needed. The compiled result
is the same as for plain array or structure, so I would rather
stick with struct here, until contrary proved (I mean it's not
a problem to update the patch and use array here but I still
think struct it better in long term). If you guys think that I
should move it to array -- no problem, i may update ;)
seq_puts(m, VmFlags: );
for (i = 0; i BITS_PER_LONG; i++) {
- if (vma-vm_flags (1 i))
+ if (vma-vm_flags (1ul i)) {
seq_printf(m, %c%c ,
mnemonics[i].l[0],
mnemonics[i].l[1]);
mnemonics[i][0], mnemonics[i][1]
/me looks for another bike shed :-)
Heh :)
--
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/
Re: + procfs-add-vmflags-field-in-smaps-output-v3-fix-2.patch added to -mm tree
Hi Cyrill,
On Wed, 24 Oct 2012 13:59:59 +0400 Cyrill Gorcunov gorcu...@openvz.org wrote:
On Wed, Oct 24, 2012 at 08:47:46PM +1100, Stephen Rothwell wrote:
On Wed, 24 Oct 2012 12:45:15 +0400 Cyrill Gorcunov gorcu...@openvz.org
wrote:
static void show_smap_vma_flags(struct seq_file *m, struct
vm_area_struct *vma)
{
+#define __VM_FLAG(_f, _s)[ilog2(_f)] = {(const char [2]){_s}}
I really don't think you need the cast (and it may hide bad usages) or
the second set of braces. Thus:
Hi Stephen, could you please elaborate, which bad usage hiding could be there?
I though it might suppress a warning if someone added an entry with a 2
character string, but testing shows that it does not, so that is OK.
There is still no need for the cast, though, or the second set of
braces. The C standard allows fixed length char arrays to be initialised
by string constants and discards the trailing NUL if necessary.
Further is there any reason for the struct?
#define __VM_FLAG(_f, _s) [ilog2(_f)] = _s
static const char mnemonics[BITS_PER_LONG][2] = {
...
};
Well, good point, though the benefit of using stucture here
could be easier way for extension if needed. The compiled result
is the same as for plain array or structure, so I would rather
stick with struct here, until contrary proved (I mean it's not
a problem to update the patch and use array here but I still
think struct it better in long term). If you guys think that I
should move it to array -- no problem, i may update ;)
One thing we learn about early (especially working at IBM) is to resist
the temptation to over engineer :-)
--
Cheers,
Stephen Rothwells...@canb.auug.org.au
pgpMGQdLwCSGF.pgp
Description: PGP signature
Re: + procfs-add-vmflags-field-in-smaps-output-v3-fix-2.patch added to -mm tree
On Wed, Oct 24, 2012 at 09:26:20PM +1100, Stephen Rothwell wrote: Hi Stephen, could you please elaborate, which bad usage hiding could be there? I though it might suppress a warning if someone added an entry with a 2 character string, but testing shows that it does not, so that is OK. There is still no need for the cast, though, or the second set of braces. The C standard allows fixed length char arrays to be initialised by string constants and discards the trailing NUL if necessary. OK, fair enough. I'll post a new patch with all concerns fixed in one pass. Thanks all for comments! -- 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/
