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

From: KOSAKI Motohiro
Date: Tue Apr 14 2009 - 02:54:55 EST


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@xxxxxxxxxxxxxx>
> > > Cc: Matt Mackall <mpm@xxxxxxxxxxx>
> > > Cc: Alexey Dobriyan <adobriyan@xxxxxxxxx>
> > > Signed-off-by: Wu Fengguang <fengguang.wu@xxxxxxxxx>
> > > ---
> > > 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@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/



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