[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20090414072231.GA7001@localhost>
Date: Tue, 14 Apr 2009 15:22:31 +0800
From: Wu Fengguang <fengguang.wu@...el.com>
To: KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com>
Cc: Andrew Morton <akpm@...ux-foundation.org>,
Andi Kleen <andi@...stfloor.org>,
LKML <linux-kernel@...r.kernel.org>,
"linux-mm@...ck.org" <linux-mm@...ck.org>
Subject: Re: [RFC][PATCH] proc: export more page flags in /proc/kpageflags
On Tue, Apr 14, 2009 at 02:54:40PM +0800, KOSAKI Motohiro wrote:
> 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
How about including PG_unevictable/PG_mlocked?
They shall be meaningful to administrators.
> 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.
You mean to fold PG_head/PG_tail into PG_COMPOUND?
Yes, that's a good simplification for end users.
> >
> > > > 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?
That would not be usable if you are going to do a system wide scan.
PG_NOPAGE could help differentiate the 'no page' case from 'no flags'
case.
However PG_NOPAGE is more about the last resort. The system wide scan
can be made much more efficient if we know the exact memory layouts.
Thanks,
Fengguang
> > > >
> > > > This increases the total number of exported page flags to 25.
> > > >
> > > > Cc: Andi Kleen <andi@...stfloor.org>
> > > > Cc: Matt Mackall <mpm@...enic.com>
> > > > Cc: Alexey Dobriyan <adobriyan@...il.com>
> > > > Signed-off-by: Wu Fengguang <fengguang.wu@...el.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@...r.kernel.org
> > 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@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists