archive mirror
 help / color / mirror / Atom feed
From: Dan Magenheimer <>
To: Andrew Morton <>
Cc: Dave Hansen <>,,,
	Konrad Wilk <>,
	Seth Jennings <>,
	Nitin Gupta <>,
	Nebojsa Trpkovic <>,,
	KAMEZAWA Hiroyuki <>,, Chris Mason <>
Subject: RE: [PATCH] mm: implement WasActive page flag (for improving cleancache)
Date: Thu, 26 Jan 2012 16:56:34 -0800 (PST)	[thread overview]
Message-ID: <ccb76a4d-d453-4faa-93a9-d1ce015255c0@default> (raw)
In-Reply-To: <>

> From: Andrew Morton []
> Subject: Re: [PATCH] mm: implement WasActive page flag (for improving cleancache)

Thanks for the reply!
> On Thu, 26 Jan 2012 13:28:02 -0800 (PST)
> Dan Magenheimer <> wrote:
> > > I do think it also needs to get cleared on the way in to the page
> > > allocator.  Otherwise:
> > >
> > > 	PageSetWasActive(page);
> > > 	free_page(page);
> > > 	...
> > > 	another_user_page = get_free_page()
> > > 	// now cleancache sees the active bit for the prev user
> > >
> > > Or am I missing somewhere it gets cleared non-explicitly somewhere?
> >
> > True, it is not getting cleared and it should be, good catch!
> It should be added to PAGE_FLAGS_CHECK_AT_FREE.

I was thinking of clearing it in free_pages_prepare() before the
call to free_pages_check().  Does that make sense?  If so, then
it could also be added to PAGE_FLAGS_CHECK_AT_FREE, though it might
be a bit redundant.

> > I'll find the place to add the call to ClearPageWasActive() for v2.
> AFAICT this patch consumes our second-last page flag, or close to it.
> We'll all be breaking out in hysterics when the final one is gone.

I'd be OK with only using this on 64-bit systems, though there
are ARM folks playing with zcache that might disagree.  Am I
correct in assuming that your "second-last page flag" concern
applies only to 32-bit systems?

> This does appear to be a make or break thing for cleancache - if we
> can't fix then cleancache is pretty
> much a dead duck.

Hmmm... is that URL correct?  If so, there is some subtlety in
that thread that I am missing as I don't understand the relationship
to cleancache at all?

> But I'm going to ask for great effort to avoid
> consuming another page flag.  Either fix cleancache via other means or,
> much less desirably, find an existing page flag and overload it.

Cleancache isn't broken.  The fix is not a requirement for other
cleancache users (Xen and RAMster), though it is definitely useful.
It's not a _requirement_ for zcache either but definitely helps on
certain workloads and systems, see below.

> And I'm afraid that neither I nor other MM developers are likely to
> help you with "fix cleancache via other means" because we weren't
> provided with any description of what the problem is within cleancache,
> nor how it will be fixed.  All we are given is the assertion "cleancache
> needs this".

The patch comment says:

The patch resolves issues reported with cleancache which occur
especially during streaming workloads on older processors,

I can see that may not be sufficient, so let me expand on it.

First, just as page replacement worked prior to the active/inactive
redesign at 2.6.27, cleancache works without the WasActive page flag.
However, just as pre-2.6.27 page replacement had problems on
streaming workloads, so does cleancache.  The WasActive page flag
is an attempt to pass the same active/inactive info gathered by
the post-2.6.27 kernel into cleancache, with the same objectives and
presumably the same result: improving the "quality" of pages preserved
in memory thus reducing refaults.

Is that clearer?  If so, I'll do better on the description at v2.


  reply	other threads:[~2012-01-27  0:56 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-01-25 21:58 [PATCH] mm: implement WasActive page flag (for improving cleancache) Dan Magenheimer
2012-01-26 17:28 ` Dave Hansen
2012-01-26 21:28   ` Dan Magenheimer
2012-01-27  0:31     ` Andrew Morton
2012-01-27  0:56       ` Dan Magenheimer [this message]
2012-01-27  1:15         ` Andrew Morton
2012-01-27  2:43           ` Dan Magenheimer
2012-01-27  3:33             ` Rik van Riel
2012-01-27  5:15               ` Dan Magenheimer
2012-01-30  8:57                 ` KAMEZAWA Hiroyuki
2012-01-30 22:03                   ` Dan Magenheimer
2012-01-27 13:43             ` James Bottomley
2012-01-27 17:32               ` Dan Magenheimer
2012-01-27 17:54                 ` James Bottomley
2012-01-27 18:46                   ` Dan Magenheimer
2012-01-27 21:49                     ` James Bottomley
2012-01-29  0:50                       ` Rik van Riel
2012-01-29 22:25                         ` James Bottomley
2012-01-27  3:28         ` Rik van Riel
2012-01-27  5:11           ` Dan Magenheimer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ccb76a4d-d453-4faa-93a9-d1ce015255c0@default \ \ \ \ \ \ \ \ \ \ \ \ \ \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).