linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Minchan Kim <minchan@kernel.org>
To: Michal Nazarewicz <mina86@mina86.com>
Cc: Marek Szyprowski <m.szyprowski@samsung.com>,
	Mel Gorman <mgorman@suse.de>, Rik van Riel <riel@redhat.com>,
	Kamezawa Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	linux-kernel@vger.kernel.org, linux-mm@kvack.org
Subject: Re: [RFC 2/2] cma: support MIGRATE_DISCARD
Date: Thu, 16 Aug 2012 08:20:23 +0900	[thread overview]
Message-ID: <20120815232023.GA15225@bbox> (raw)
In-Reply-To: <xa1t7gt1pnck.fsf@mina86.com>

Hi Michal,

On Tue, Aug 14, 2012 at 04:19:55PM +0200, Michal Nazarewicz wrote:
> Minchan Kim <minchan@kernel.org> writes:
> > This patch introudes MIGRATE_DISCARD mode in migration.
> > It drop clean cache pages instead of migration so that
> > migration latency could be reduced. Of course, it could
> > evict code pages but latency of big contiguous memory
> > is more important than some background application's slow down
> > in mobile embedded enviroment.
> >
> > Signed-off-by: Minchan Kim <minchan@kernel.org>
> 
> This looks good to me.
> 
> > ---
> >  include/linux/migrate_mode.h |   11 +++++++---
> >  mm/migrate.c                 |   50 +++++++++++++++++++++++++++++++++---------
> >  mm/page_alloc.c              |    2 +-
> >  3 files changed, 49 insertions(+), 14 deletions(-)
> >
> > diff --git a/include/linux/migrate_mode.h b/include/linux/migrate_mode.h
> > index ebf3d89..04ca19c 100644
> > --- a/include/linux/migrate_mode.h
> > +++ b/include/linux/migrate_mode.h
> > @@ -6,11 +6,16 @@
> >   *	on most operations but not ->writepage as the potential stall time
> >   *	is too significant
> >   * MIGRATE_SYNC will block when migrating pages
> > + * MIGRTATE_DISCARD will discard clean cache page instead of migration
> > + *
> > + * MIGRATE_ASYNC, MIGRATE_SYNC_LIGHT, MIGRATE_SYNC shouldn't be used
> > + * together as OR flag.
> >   */
> >  enum migrate_mode {
> > -	MIGRATE_ASYNC,
> > -	MIGRATE_SYNC_LIGHT,
> > -	MIGRATE_SYNC,
> > +	MIGRATE_ASYNC = 1 << 0,
> > +	MIGRATE_SYNC_LIGHT = 1 << 1,
> > +	MIGRATE_SYNC = 1 << 2,
> > +	MIGRATE_DISCARD = 1 << 3,
> >  };
> 
> Since CMA is the only user of MIGRATE_DISCARD it may be worth it to
> guard it inside an #ifdef, eg:
> 
> #ifdef CONFIG_CMA
> 	MIGRATE_DISCARD = 1 << 3,
> #define is_migrate_discard(mode) (((mode) & MIGRATE_DISCARD) == MIGRATE_DISCARD)

The mode bit can be used with other bits like MIGRATE_SYNC|MIGRATE_DISCARD.
So it is correct that (mode & MIGRATE_DISCARD).

Anyway, I don't want to fold it into only CMA because I think we can
have a pontential users in mm.
For example, memory-hotplug case. No enough free memory in the system
but lots of page cache page as a migration source, then we can remove
page cache page instead of migration and it might be better than failing
memory-hotremove.

In summary, I want to open it for potential usecases in future if anyone
doesn't oppose strongly.

> #endif
> 
>   
> >  #endif		/* MIGRATE_MODE_H_INCLUDED */
> > diff --git a/mm/migrate.c b/mm/migrate.c
> > index 77ed2d7..8119a59 100644
> > --- a/mm/migrate.c
> > +++ b/mm/migrate.c
> > @@ -685,9 +685,12 @@ static int __unmap_and_move(struct page *page, struct page *newpage,
> >  	int remap_swapcache = 1;
> >  	struct mem_cgroup *mem;
> >  	struct anon_vma *anon_vma = NULL;
> > +	enum ttu_flags ttu_flags;
> > +	bool discard_mode = false;
> > +	bool file = false;
> >  
> >  	if (!trylock_page(page)) {
> > -		if (!force || mode == MIGRATE_ASYNC)
> > +		if (!force || mode & MIGRATE_ASYNC)

It's not wrong technically but for readability, NP.

> 
> +		if (!force || (mode & MIGRATE_ASYNC))
> 
> >  			goto out;
> >  
> >  		/*
> 
> 
> -- 
> Best regards,                                         _     _
> .o. | Liege of Serenely Enlightened Majesty of      o' \,=./ `o
> ..o | Computer Science,  MichaA? a??mina86a?? Nazarewicz    (o o)
> ooo +----<email/xmpp: mpn@google.com>--------------ooO--(_)--Ooo--





-- 
Kind regards,
Minchan Kim

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  reply	other threads:[~2012-08-15 23:18 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-08-14  8:57 [RFC 0/2] Reduce alloc_contig_range latency Minchan Kim
2012-08-14  8:57 ` [RFC 1/2] cma: remove __reclaim_pages Minchan Kim
2012-08-15 18:49   ` Rik van Riel
2012-08-16 13:58   ` Mel Gorman
2012-08-17  1:05     ` Minchan Kim
2012-08-17 14:48       ` Marek Szyprowski
2012-08-14  8:57 ` [RFC 2/2] cma: support MIGRATE_DISCARD Minchan Kim
2012-08-14 14:19   ` Michal Nazarewicz
2012-08-15 23:20     ` Minchan Kim [this message]
2012-08-16 13:17       ` Michal Nazarewicz
2012-08-15 18:58   ` Rik van Riel
2012-08-15 23:33     ` Minchan Kim
2012-08-16  0:15       ` Minchan Kim

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:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

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

  git send-email \
    --in-reply-to=20120815232023.GA15225@bbox \
    --to=minchan@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=kamezawa.hiroyu@jp.fujitsu.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=m.szyprowski@samsung.com \
    --cc=mgorman@suse.de \
    --cc=mina86@mina86.com \
    --cc=riel@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* 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).