Re: [RFC][PATCH] proc: export more page flags in /proc/kpageflags

[KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>]

Hi

> On Tue, Apr 14, 2009 at 12:37:10PM +0800, KOSAKI Motohiro wrote:
> > > Export the following page flags in /proc/kpageflags,
> > > just in case they will be useful to someone:
> > > 
> > > - PG_swapcache
> > > - PG_swapbacked
> > > - PG_mappedtodisk
> > > - PG_reserved
> > > - PG_private
> > > - PG_private_2
> > > - PG_owner_priv_1
> > > 
> > > - PG_head
> > > - PG_tail
> > > - PG_compound
> > > 
> > > - PG_unevictable
> > > - PG_mlocked
> > > 
> > > - PG_poison
> > 
> > Sorry, NAK this.
> > We shouldn't expose internal flags. please choice useful flags only.
> 
> OK. So are there anyone interested in any of these flags? Thanks!
> 
> My rational to export most page flags is that hopefully they could
> help debugging kernel at some random situations..

I think,

> > > - PG_mappedtodisk
> > > - PG_reserved
> > > - PG_private
> > > - PG_private_2
> > > - PG_owner_priv_1
> > > 
> > > - PG_head
> > > - PG_tail
> > > 
> > > - PG_unevictable
> > > - PG_mlocked

this 9 flags shouldn't exported.
I can't imazine administrator use what purpose those flags.

> > > - PG_swapcache
> > > - PG_swapbacked
> > > - PG_poison
> > > - PG_compound

I can agree this 4 flags.
However pagemap lack's hugepage considering.
if PG_compound exporting, we need more work.


> 
> > > Also add the following two pseudo page flags:
> > > 
> > > - PG_MMAP:   whether the page is memory mapped

hm, I can agree it.


> > > - PG_NOPAGE: whether the page is present

PM_NOT_PRESENT isn't enough?


> > > 
> > > This increases the total number of exported page flags to 25.
> > > 
> > > Cc: Andi Kleen <andi@firstfloor.org>
> > > Cc: Matt Mackall <mpm@selenic.com>
> > > Cc: Alexey Dobriyan <adobriyan@gmail.com>
> > > Signed-off-by: Wu Fengguang <fengguang.wu@intel.com>
> > > ---
> > >  fs/proc/page.c |  112 +++++++++++++++++++++++++++++++++--------------
> > >  1 file changed, 81 insertions(+), 31 deletions(-)
> > > 
> > > --- mm.orig/fs/proc/page.c
> > > +++ mm/fs/proc/page.c
> > > @@ -68,20 +68,86 @@ static const struct file_operations proc
> > >  
> > >  /* These macros are used to decouple internal flags from exported ones */
> > >  
> > > -#define KPF_LOCKED     0
> > > -#define KPF_ERROR      1
> > > -#define KPF_REFERENCED 2
> > > -#define KPF_UPTODATE   3
> > > -#define KPF_DIRTY      4
> > > -#define KPF_LRU        5
> > > -#define KPF_ACTIVE     6
> > > -#define KPF_SLAB       7
> > > -#define KPF_WRITEBACK  8
> > > -#define KPF_RECLAIM    9
> > > -#define KPF_BUDDY     10
> > > +enum {
> > > +	KPF_LOCKED,		/*  0 */
> > > +	KPF_ERROR,		/*  1 */
> > > +	KPF_REFERENCED,		/*  2 */
> > > +	KPF_UPTODATE,		/*  3 */
> > > +	KPF_DIRTY,		/*  4 */
> > > +	KPF_LRU,		/*  5 */
> > > +	KPF_ACTIVE,		/*  6 */
> > > +	KPF_SLAB,		/*  7 */
> > > +	KPF_WRITEBACK,		/*  8 */
> > > +	KPF_RECLAIM,		/*  9 */
> > > +	KPF_BUDDY,		/* 10 */
> > > +	KPF_MMAP,		/* 11 */
> > > +	KPF_SWAPCACHE,		/* 12 */
> > > +	KPF_SWAPBACKED,		/* 13 */
> > > +	KPF_MAPPEDTODISK,	/* 14 */
> > > +	KPF_RESERVED,		/* 15 */
> > > +	KPF_PRIVATE,		/* 16 */
> > > +	KPF_PRIVATE2,		/* 17 */
> > > +	KPF_OWNER_PRIVATE,	/* 18 */
> > > +	KPF_COMPOUND_HEAD,	/* 19 */
> > > +	KPF_COMPOUND_TAIL,	/* 20 */
> > > +	KPF_UNEVICTABLE,	/* 21 */
> > > +	KPF_MLOCKED,		/* 22 */
> > > +	KPF_POISON,		/* 23 */
> > > +	KPF_NOPAGE,		/* 24 */
> > > +	KPF_NUM
> > > +};
> > 
> > this is userland export value. then enum is wrong idea.
> > explicit name-number relationship is better. it prevent unintetional
> > ABI break.
> 
> Right, that's the reason I add the /* number */ comments.
> Anyway, it would be better to use explicit #defines.
> 
> Thanks,
> Fengguang
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/