Messages in this thread | | | Date | Wed, 24 Feb 2010 01:04:02 +0900 | Subject | Re: [patch 1/3] vmscan: factor out page reference checks | From | Minchan Kim <> |
| |
On Wed, Feb 24, 2010 at 12:40 AM, Johannes Weiner <hannes@cmpxchg.org> wrote: > 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?
I hope as it is if we don't have any better idea and you don't feel it strong. But let's listen to other's opinion. maybe they have a good idea.
Thanks, Hannes.
-- Kind regards, Minchan Kim -- 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/
| |