Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754955AbbG1AVk (ORCPT ); Mon, 27 Jul 2015 20:21:40 -0400 Received: from lgeamrelo04.lge.com ([156.147.1.127]:60602 "EHLO lgeamrelo04.lge.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754696AbbG1AVh (ORCPT ); Mon, 27 Jul 2015 20:21:37 -0400 X-Original-SENDERIP: 10.178.37.108 X-Original-MAILFROM: gioh.kim@lge.com Message-ID: <55B6CB0C.8050604@lge.com> Date: Tue, 28 Jul 2015 09:21:32 +0900 From: Gioh Kim User-Agent: Mozilla/5.0 (Windows NT 6.1; rv:31.0) Gecko/20100101 Thunderbird/31.7.0 MIME-Version: 1.0 To: Konstantin Khlebnikov , Vlastimil Babka CC: Jeff Layton , Bruce Fields , Joonsoo Kim , Al Viro , "Michael S. Tsirkin" , Minchan Kim , Rafael Aquini , linux-fsdevel , virtualization@lists.linux-foundation.org, Linux Kernel Mailing List , Linux API , "linux-mm@kvack.org" , dri-devel , Andrew Morton , Gioh Kim Subject: Re: [PATCH 2/4] mm/compaction: enable mobile-page migration References: <1436776519-17337-1-git-send-email-gioh.kim@lge.com> <1436776519-17337-3-git-send-email-gioh.kim@lge.com> <55B63851.1080100@suse.cz> In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 9937 Lines: 280 > On Mon, Jul 27, 2015 at 4:55 PM, Vlastimil Babka wrote: >> On 07/13/2015 10:35 AM, Gioh Kim wrote: >>> >>> From: Gioh Kim >>> >>> Add framework to register callback functions and check page mobility. >>> There are some modes for page isolation so that isolate interface >>> has arguments of page address and isolation mode while putback >>> interface has only page address as argument. >> >> >> Note that unlike what subject suggest, this doesn't really enable >> mobile-page migration inside compaction, since that only happens with patch >> 3. This might theoretically affect some cherry-pick backports that don't >> care about balloon pages. I can imagine that can easily happen in the world >> of mobile devices? >> It would thus be somewhat cleaner if this patch was complete in that sense. You have a point. Current 2/4 patch is lack of calling migrate/isolate. It is not complete without 3/4. I'll add calling migrate/isolate() at next spin. >> >>> Signed-off-by: Gioh Kim >>> Acked-by: Rafael Aquini >>> --- >>> fs/proc/page.c | 3 ++ >>> include/linux/compaction.h | 80 >>> ++++++++++++++++++++++++++++++++++ >>> include/linux/fs.h | 2 + >>> include/linux/page-flags.h | 19 ++++++++ >>> include/uapi/linux/kernel-page-flags.h | 1 + >>> 5 files changed, 105 insertions(+) >>> >>> diff --git a/fs/proc/page.c b/fs/proc/page.c >>> index 7eee2d8..a4f5a00 100644 >>> --- a/fs/proc/page.c >>> +++ b/fs/proc/page.c >>> @@ -146,6 +146,9 @@ u64 stable_page_flags(struct page *page) >>> if (PageBalloon(page)) >>> u |= 1 << KPF_BALLOON; >>> >>> + if (PageMobile(page)) >>> + u |= 1 << KPF_MOBILE; >> >> >> PageMovable() would probably be as good a name and correspond to >> MIGRATE_MOVABLE somewhat, unlike a completely new term. Whatever driver >> starts to using this should probably change allocation flags to allocate >> MIGRATE_MOVABLE, so that it works fine with what fragmentation avoidance >> expects. Guess I should have said that earlier, but can you still >> reconsider? > > Well, I've suggested to name it "mobile" because there's already a lot of things > called "movable". Mobile pages are special subset of movable pages: they > are non-lru pages and define their own rules of moving in address > space operations. > > Also there's a little pun: I guess main user will zram which is used > mostly in embedded/mobile devices. I like "mobile". "movable" is for pages allocated with GFP_MOVABLE. I think "movable" is a little ambiguous in this situation. > >> >>> + >>> u |= kpf_copy_bit(k, KPF_LOCKED, PG_locked); >>> >>> u |= kpf_copy_bit(k, KPF_SLAB, PG_slab); >>> diff --git a/include/linux/compaction.h b/include/linux/compaction.h >>> index aa8f61c..f693072 100644 >>> --- a/include/linux/compaction.h >>> +++ b/include/linux/compaction.h >>> @@ -1,6 +1,9 @@ >>> #ifndef _LINUX_COMPACTION_H >>> #define _LINUX_COMPACTION_H >>> >>> +#include >>> +#include >>> + >>> /* Return values for compact_zone() and try_to_compact_pages() */ >>> /* compaction didn't start as it was deferred due to past failures */ >>> #define COMPACT_DEFERRED 0 >>> @@ -51,6 +54,70 @@ extern void compaction_defer_reset(struct zone *zone, >>> int order, >>> bool alloc_success); >>> extern bool compaction_restarting(struct zone *zone, int order); >>> >>> +static inline bool mobile_page(struct page *page) >>> +{ >>> + return page->mapping && (PageMobile(page) || PageBalloon(page)); >>> +} >> >> >> I would put this definition to linux/page-flags.h and rename it to >> page_mobile (or better page_movable()), which is more common ordering. >> >>> + >>> +static inline bool isolate_mobilepage(struct page *page, isolate_mode_t >>> mode) >> >> >> Does this have to be in compaction.h? The only user is compaction.c so >> probably move it there, and if there ever is another module using this in >> the future, we can move it to a more appropriate place and declare it in >> e.g. mm/internal.h. I think compaction.c is suitable. >> >> >>> +{ >>> + bool ret = false; >>> + >>> + /* >>> + * Avoid burning cycles with pages that are yet under >>> __free_pages(), >>> + * or just got freed under us. >>> + * >>> + * In case we 'win' a race for a mobile page being freed under us >>> and >>> + * raise its refcount preventing __free_pages() from doing its job >>> + * the put_page() at the end of this block will take care of >>> + * release this page, thus avoiding a nasty leakage. >>> + */ >>> + if (unlikely(!get_page_unless_zero(page))) >>> + goto out; >>> + >>> + /* >>> + * As mobile pages are not isolated from LRU lists, concurrent >>> + * compaction threads can race against page migration functions >>> + * as well as race against the releasing a page. >>> + * >>> + * In order to avoid having an already isolated mobile page >>> + * being (wrongly) re-isolated while it is under migration, >>> + * or to avoid attempting to isolate pages being released, >>> + * lets be sure we have the page lock >>> + * before proceeding with the mobile page isolation steps. >>> + */ >>> + if (unlikely(!trylock_page(page))) >>> + goto out_putpage; >>> + >>> + if (!(mobile_page(page) && page->mapping->a_ops->isolatepage)) >>> + goto out_not_isolated; >>> + ret = page->mapping->a_ops->isolatepage(page, mode); >>> + if (!ret) >>> + goto out_not_isolated; >>> + unlock_page(page); >>> + return ret; >>> + >>> +out_not_isolated: >>> + unlock_page(page); >>> +out_putpage: >>> + put_page(page); >>> +out: >>> + return ret; >>> +} >>> + >>> +static inline void putback_mobilepage(struct page *page) >> >> >> Likewise, this could go to migrate.c. Or maybe together with >> isolate_mobilepage() if you don't want to split them. I got it. >> >> >>> +{ >>> + /* >>> + * 'lock_page()' stabilizes the page and prevents races against >>> + * concurrent isolation threads attempting to re-isolate it. >>> + */ >>> + lock_page(page); >>> + if (page->mapping && page->mapping->a_ops->putbackpage) >>> + page->mapping->a_ops->putbackpage(page); >>> + unlock_page(page); >>> + /* drop the extra ref count taken for mobile page isolation */ >>> + put_page(page); >>> +} >>> #else >>> static inline unsigned long try_to_compact_pages(gfp_t gfp_mask, >>> unsigned int order, int alloc_flags, >>> @@ -83,6 +150,19 @@ static inline bool compaction_deferred(struct zone >>> *zone, int order) >>> return true; >>> } >>> >>> +static inline bool mobile_page(struct page *page) >>> +{ >>> + return false; >>> +} >>> + >>> +static inline bool isolate_mobilepage(struct page *page, isolate_mode_t >>> mode) >>> +{ >>> + return false; >>> +} >>> + >>> +static inline void putback_mobilepage(struct page *page) >>> +{ >>> +} >>> #endif /* CONFIG_COMPACTION */ >>> >>> #if defined(CONFIG_COMPACTION) && defined(CONFIG_SYSFS) && >>> defined(CONFIG_NUMA) >>> diff --git a/include/linux/fs.h b/include/linux/fs.h >>> index a0653e5..2cc4b24 100644 >>> --- a/include/linux/fs.h >>> +++ b/include/linux/fs.h >>> @@ -396,6 +396,8 @@ struct address_space_operations { >>> */ >>> int (*migratepage) (struct address_space *, >>> struct page *, struct page *, enum migrate_mode); >>> + bool (*isolatepage) (struct page *, isolate_mode_t); >>> + void (*putbackpage) (struct page *); >>> int (*launder_page) (struct page *); >>> int (*is_partially_uptodate) (struct page *, unsigned long, >>> unsigned long); >>> diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h >>> index f34e040..abef145 100644 >>> --- a/include/linux/page-flags.h >>> +++ b/include/linux/page-flags.h >>> @@ -582,6 +582,25 @@ static inline void __ClearPageBalloon(struct page >>> *page) >>> atomic_set(&page->_mapcount, -1); >>> } >>> >>> +#define PAGE_MOBILE_MAPCOUNT_VALUE (-255) >>> + >>> +static inline int PageMobile(struct page *page) >>> +{ >>> + return atomic_read(&page->_mapcount) == >>> PAGE_MOBILE_MAPCOUNT_VALUE; >>> +} >>> + >>> +static inline void __SetPageMobile(struct page *page) >>> +{ >>> + VM_BUG_ON_PAGE(atomic_read(&page->_mapcount) != -1, page); >>> + atomic_set(&page->_mapcount, PAGE_MOBILE_MAPCOUNT_VALUE); >>> +} >>> + >>> +static inline void __ClearPageMobile(struct page *page) >>> +{ >>> + VM_BUG_ON_PAGE(!PageMobile(page), page); >>> + atomic_set(&page->_mapcount, -1); >>> +} >>> + >>> /* >>> * If network-based swap is enabled, sl*b must keep track of whether >>> pages >>> * were allocated from pfmemalloc reserves. >>> diff --git a/include/uapi/linux/kernel-page-flags.h >>> b/include/uapi/linux/kernel-page-flags.h >>> index a6c4962..d50d9e8 100644 >>> --- a/include/uapi/linux/kernel-page-flags.h >>> +++ b/include/uapi/linux/kernel-page-flags.h >>> @@ -33,6 +33,7 @@ >>> #define KPF_THP 22 >>> #define KPF_BALLOON 23 >>> #define KPF_ZERO_PAGE 24 >>> +#define KPF_MOBILE 25 >>> >>> >>> #endif /* _UAPILINUX_KERNEL_PAGE_FLAGS_H */ >>> >> > -- 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/