linux-kernel - Re: [PATCH V2 1/2] mm: hotplug: implement non-movable version of get_user_pages() called get_user_pages_non

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 6 Feb 2013 09:42:34 +0900
From:	Minchan Kim <minchan@...nel.org>
To:	Mel Gorman <mgorman@...e.de>
Cc:	Lin Feng <linfeng@...fujitsu.com>, akpm@...ux-foundation.org,
	bcrl@...ck.org, viro@...iv.linux.org.uk, khlebnikov@...nvz.org,
	walken@...gle.com, kamezawa.hiroyu@...fujitsu.com, riel@...hat.com,
	rientjes@...gle.com, isimatu.yasuaki@...fujitsu.com,
	wency@...fujitsu.com, laijs@...fujitsu.com, jiang.liu@...wei.com,
	zab@...hat.com, jmoyer@...hat.com, linux-mm@...ck.org,
	linux-aio@...ck.org, linux-fsdevel@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH V2 1/2] mm: hotplug: implement non-movable version of
 get_user_pages() called get_user_pages_non_movable()

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@...ux-foundation.org>
> > Cc: Mel Gorman <mgorman@...e.de>
> > Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@...fujitsu.com>
> > Cc: Yasuaki Ishimatsu <isimatu.yasuaki@...fujitsu.com>
> > Cc: Jeff Moyer <jmoyer@...hat.com>
> > Cc: Minchan Kim <minchan@...nel.org>
> > Cc: Zach Brown <zab@...hat.com>
> > Reviewed-by: Tang Chen <tangchen@...fujitsu.com>
> > Reviewed-by: Gu Zheng <guz.fnst@...fujitsu.com>
> > Signed-off-by: Lin Feng <linfeng@...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@...ck.org.  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont@...ck.org"> email@...ck.org </a>

-- 
Kind regards,
Minchan Kim
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/