linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Minchan Kim <minchan@kernel.org>
To: Mel Gorman <mgorman@suse.de>
Cc: Lin Feng <linfeng@cn.fujitsu.com>,
	akpm@linux-foundation.org, bcrl@kvack.org,
	viro@zeniv.linux.org.uk, khlebnikov@openvz.org,
	walken@google.com, kamezawa.hiroyu@jp.fujitsu.com,
	riel@redhat.com, rientjes@google.com,
	isimatu.yasuaki@jp.fujitsu.com, wency@cn.fujitsu.com,
	laijs@cn.fujitsu.com, jiang.liu@huawei.com, zab@redhat.com,
	jmoyer@redhat.com, linux-mm@kvack.org, linux-aio@kvack.org,
	linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH V2 1/2] mm: hotplug: implement non-movable version of get_user_pages() called get_user_pages_non_movable()
Date: Wed, 6 Feb 2013 09:42:34 +0900	[thread overview]
Message-ID: <20130206004234.GD11197@blaptop> (raw)
In-Reply-To: <20130205120137.GG21389@suse.de>

On Tue, Feb 05, 2013 at 12:01:37PM +0000, Mel Gorman wrote:
> On Tue, Feb 05, 2013 at 05:21:52PM +0800, Lin Feng wrote:
> > get_user_pages() always tries to allocate pages from movable zone, which is not
> >  reliable to memory hotremove framework in some case.
> > 
> > This patch introduces a new library function called get_user_pages_non_movable()
> >  to pin pages only from zone non-movable in memory.
> > It's a wrapper of get_user_pages() but it makes sure that all pages come from
> > non-movable zone via additional page migration.
> > 
> > Cc: Andrew Morton <akpm@linux-foundation.org>
> > Cc: Mel Gorman <mgorman@suse.de>
> > Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> > Cc: Yasuaki Ishimatsu <isimatu.yasuaki@jp.fujitsu.com>
> > Cc: Jeff Moyer <jmoyer@redhat.com>
> > Cc: Minchan Kim <minchan@kernel.org>
> > Cc: Zach Brown <zab@redhat.com>
> > Reviewed-by: Tang Chen <tangchen@cn.fujitsu.com>
> > Reviewed-by: Gu Zheng <guz.fnst@cn.fujitsu.com>
> > Signed-off-by: Lin Feng <linfeng@cn.fujitsu.com>
> 
> I already had started the review of V1 before this was sent
> unfortunately. However, I think the feedback I gave for V1 is still
> valid so I'll wait for comments on that review before digging further.

Mel, Andrew

Sorry for making noise if you already confirmed the direction but I have a concern
about that. Because IMHO, we can't expect most of user for MEMORY_HOTPLUG will release
pinned pages immediately. In addtion, MEMORY_HOTPLUG could be used for embedded system
for reducing power by PASR and some drivers in embedded could use GUP anytime and anywhere.
They can't know in advance they will use pinned pages long time or release in short time
because it depends on some event like user's response which is very not predetermined.
So for solving it, we can add some WARN_ON in CMA/MEMORY_HOTPLUG part just in case of
failing migration by page count and then, investigate they are really using GUP and
it's REALLY a culprit. If so, yell to them "Please use GUP_NM instead"?
Yes. it could be done but it would be rather trobulesome job. Even it couldn't be triggered
during QE phase so that trouble doesn't end until all guys uses GUP_NM.
Let's consider another case. Some driver pin the page in very short time
so he decide to use GUP instead of GUP_NM but someday, someuser start to use the driver
very often so although pinning time is very short, it could be forever pinning effect
if the use calls it very often. In the end, we should change it with GUP_NM, again.
IMHO, In future, we ends up changing most of GUP user with GUP_NM if CMA and MEMORY_HOTPLUG
is available all over the world.

So, what's wrong if we replace get_user_pages with get_user_pages_non_movable
in MEMORY_HOTPLUG/CMA without exposing get_user_pages_non_movable?

I mean this

#ifdef CONFIG_MIGRATE_ISOLATE
int get_user_pages()
{
        return __get_user_pages_non_movable();
}
#else
int get_user_pages()
{
        return old_get_user_pages();
}
#endif

IMHO, get_user_pages isn't performance sensitive function. If user was sensitive
about it, he should have tried get_user_pages_fast.
THP degradation by increasing MIGRATE_UNMOVABLE?
Lin said most of GUP pages release the page in short so is it really problem?
Even in embedded, we don't use THP yet but CMA and GUP call would be not too often
but failing of CMA would be critical.

I'd like to hear opinions.

> 
> -- 
> Mel Gorman
> SUSE Labs
> 
> --
> 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>

-- 
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:[~2013-02-06  0:42 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-02-05  9:21 [PATCH V2 0/2] mm: hotplug: implement non-movable version of get_user_pages() to kill long-time pin pages Lin Feng
2013-02-05  9:21 ` [PATCH V2 1/2] mm: hotplug: implement non-movable version of get_user_pages() called get_user_pages_non_movable() Lin Feng
2013-02-05 12:01   ` Mel Gorman
2013-02-06  0:42     ` Minchan Kim [this message]
2013-02-06  0:52       ` Benjamin LaHaise
2013-02-06  9:56       ` Mel Gorman
2013-02-08  2:32         ` Minchan Kim
2013-05-13  9:11         ` Tang Chen
2013-05-13  9:19           ` Mel Gorman
2013-05-13 14:37             ` Benjamin LaHaise
2013-05-13 14:54               ` Jeff Moyer
2013-05-13 15:01                 ` Benjamin LaHaise
2013-05-14  1:24                   ` Tang Chen
2013-05-14 13:58                     ` Benjamin LaHaise
2013-05-14 15:16                       ` chen tang
2013-05-15  2:09                       ` Tang Chen
2013-05-15  7:21                         ` Tang Chen
2013-05-14  3:55             ` Tang Chen
2013-05-15 13:24               ` Mel Gorman
2013-05-16  5:54                 ` Tang Chen
2013-05-17  0:23                   ` [WiP]: aio support for migrating pages (Re: [PATCH V2 1/2] mm: hotplug: implement non-movable version of get_user_pages() called get_user_pages_non_movable()) Benjamin LaHaise
2013-05-17  3:28                     ` Tang Chen
2013-05-17 14:37                       ` Benjamin LaHaise
2013-05-21  2:07                         ` Tang Chen
2013-05-21  2:27                           ` Benjamin LaHaise
2013-06-11  9:42                             ` Tang Chen
2013-06-11 14:45                               ` Benjamin LaHaise
2013-06-28  9:24                                 ` Gu Zheng
2013-07-01  7:23                                 ` Gu Zheng
2013-07-02 18:00                                   ` Benjamin LaHaise
2013-07-03  1:53                                     ` Gu Zheng
2013-07-04  6:51                                     ` Gu Zheng
2013-07-04 11:41                                       ` Benjamin LaHaise
2013-07-05  3:21                                         ` Gu Zheng
2013-05-17 18:17                     ` Zach Brown
2013-05-17 18:30                       ` Benjamin LaHaise
2013-02-20 11:37   ` [PATCH V2 1/2] mm: hotplug: implement non-movable version of get_user_pages() called get_user_pages_non_movable() Wanpeng Li
2013-02-20 11:37   ` Wanpeng Li
     [not found]   ` <20130220113757.GA10124@hacker.(null)>
2013-02-20 12:39     ` Lin Feng
2013-02-05  9:21 ` [PATCH V2 2/2] fs/aio.c: use get_user_pages_non_movable() to pin ring pages when support memory hotremove Lin Feng

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=20130206004234.GD11197@blaptop \
    --to=minchan@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=bcrl@kvack.org \
    --cc=isimatu.yasuaki@jp.fujitsu.com \
    --cc=jiang.liu@huawei.com \
    --cc=jmoyer@redhat.com \
    --cc=kamezawa.hiroyu@jp.fujitsu.com \
    --cc=khlebnikov@openvz.org \
    --cc=laijs@cn.fujitsu.com \
    --cc=linfeng@cn.fujitsu.com \
    --cc=linux-aio@kvack.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mgorman@suse.de \
    --cc=riel@redhat.com \
    --cc=rientjes@google.com \
    --cc=viro@zeniv.linux.org.uk \
    --cc=walken@google.com \
    --cc=wency@cn.fujitsu.com \
    --cc=zab@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).