Re: [patch 1/3] vmscan: factor out page reference checks

From: Johannes Weiner
Date: Tue Feb 23 2010 - 10:40:37 EST


Hello Minchan,

On Tue, Feb 23, 2010 at 11:44:14PM +0900, Minchan Kim wrote:
> On Tue, 2010-02-23 at 15:21 +0100, Johannes Weiner wrote:
> > Hello Minchan,
> >
> > On Tue, Feb 23, 2010 at 10:38:23PM +0900, Minchan Kim wrote:
>
> <snip>
>
> > > >
> > > > if (PageDirty(page)) {
> > > > - if (sc->order <= PAGE_ALLOC_COSTLY_ORDER && referenced)
> > > > + if (references == PAGEREF_RECLAIM_CLEAN)
> > >
> > > How equal PAGEREF_RECLAIM_CLEAN and sc->order <= PAGE_ALLOC_COSTLY_ORDER
> > > && referenced by semantic?
> >
> > It is encoded in page_check_references(). When
> > sc->order <= PAGE_ALLOC_COSTLY_ORDER && referenced
> > it returns PAGEREF_RECLAIM_CLEAN.
> >
> > So
> >
> > - PageDirty() && order < COSTLY && referenced
> > + PageDirty() && references == PAGEREF_RECLAIM_CLEAN
> >
> > is an equivalent transformation. Does this answer your question?
>
> Hmm. I knew it. My point was PAGEREF_RECLAIM_CLEAN seems to be a little
> awkward. I thought PAGEREF_RECLAIM_CLEAN means if the page was clean, it
> can be reclaimed.

But you were thinking right, it is exactly what it means! If
the state is PAGEREF_RECLAIM_CLEAN, reclaim the page if it is clean:

if (PageDirty(page)) {
if (references == PAGEREF_RECLAIM_CLEAN)
goto keep_locked; /* do not reclaim */
...
}

> I think it would be better to rename it with represent "Although it's
> referenced page recently, we can reclaim it if VM try to reclaim high
> order page".

I changed it to PAGEREF_RECLAIM_LUMPY and PAGEREF_RECLAIM, but I felt
it made it worse. It's awkward that we have to communicate that state
at all, maybe it would be better to do

if (PageDirty(page) && referenced_page)
return PAGEREF_KEEP;

in page_check_references()? But doing PageDirty() twice is also kinda
lame.

I don't know. Can we leave it like that for now?

Hannes
--
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/