Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751702AbbDAEX1 (ORCPT ); Wed, 1 Apr 2015 00:23:27 -0400 Received: from LGEMRELSE6Q.lge.com ([156.147.1.121]:52310 "EHLO lgemrelse6q.lge.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750751AbbDAEXW (ORCPT ); Wed, 1 Apr 2015 00:23:22 -0400 X-Original-SENDERIP: 10.178.37.108 X-Original-MAILFROM: gioh.kim@lge.com Message-ID: <551B72B7.2030209@lge.com> Date: Wed, 01 Apr 2015 13:23:19 +0900 From: Gioh Kim User-Agent: Mozilla/5.0 (Windows NT 6.1; rv:31.0) Gecko/20100101 Thunderbird/31.5.0 MIME-Version: 1.0 To: Hillf Danton CC: linux-kernel , linux-mm@kvack.org Subject: Re: [PATCH] add generic callbacks into compaction References: <034101d06c2c$9132f0e0$b398d2a0$@alibaba-inc.com> <034201d06c2e$6435d790$2ca186b0$@alibaba-inc.com> In-Reply-To: <034201d06c2e$6435d790$2ca186b0$@alibaba-inc.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 9408 Lines: 256 2015-04-01 오후 12:46에 Hillf Danton 이(가) 쓴 글: >> >> I sent a patch about page allocation for less fragmentation. >> http://permalink.gmane.org/gmane.linux.kernel.mm/130599 >> >> It proposes a page allocator allocates pages in the same pageblock >> for the drivers to move their unmovable pages. Some drivers which comsumes many pages >> and increases system fragmentation use the allocator to move their pages to >> decrease fragmentation. >> >> I think I can try another approach. >> There is a compaction code for balloon pages. >> But the compaction code cannot migrate pages of other drivers. >> If there is a generic migration framework applicable to any drivers, >> drivers can register their migration functions. >> And the compaction can migrate movable pages and also driver's pages. >> >> I'm not familiar with virtualization so I couldn't test this patch yet. > > A RFC, no? YES, this is a RFC. It's my fault. I'm sorry. >> But if mm developers agree with this approach, I will complete this patch. >> >> I would do appreciate any feedback. >> >> Signed-off-by: Gioh Kim >> --- >> drivers/virtio/virtio_balloon.c | 2 ++ >> include/linux/balloon_compaction.h | 23 +++++++++--- >> include/linux/fs.h | 3 ++ >> include/linux/pagemap.h | 26 ++++++++++++++ >> mm/balloon_compaction.c | 68 ++++++++++++++++++++++++++++++++++-- >> mm/compaction.c | 7 ++-- >> mm/migrate.c | 24 ++++++------- >> 7 files changed, 129 insertions(+), 24 deletions(-) >> >> diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c >> index 0413157..cd9b8e4 100644 >> --- a/drivers/virtio/virtio_balloon.c >> +++ b/drivers/virtio/virtio_balloon.c >> @@ -486,6 +486,8 @@ static int virtballoon_probe(struct virtio_device *vdev) >> >> balloon_devinfo_init(&vb->vb_dev_info); >> #ifdef CONFIG_BALLOON_COMPACTION >> + vb->vb_dev_info.mapping = balloon_mapping_alloc(&vb->vb_dev_info, >> + &balloon_aops); >> vb->vb_dev_info.migratepage = virtballoon_migratepage; >> #endif >> >> diff --git a/include/linux/balloon_compaction.h b/include/linux/balloon_compaction.h >> index 9b0a15d..0af32b3 100644 >> --- a/include/linux/balloon_compaction.h >> +++ b/include/linux/balloon_compaction.h >> @@ -62,6 +62,7 @@ struct balloon_dev_info { >> struct list_head pages; /* Pages enqueued & handled to Host */ >> int (*migratepage)(struct balloon_dev_info *, struct page *newpage, >> struct page *page, enum migrate_mode mode); >> + struct address_space *mapping; > > Better if a comment line is added. I got it. >> }; >> >> extern struct page *balloon_page_enqueue(struct balloon_dev_info *b_dev_info); >> @@ -76,10 +77,22 @@ static inline void balloon_devinfo_init(struct balloon_dev_info *balloon) >> } >> >> #ifdef CONFIG_BALLOON_COMPACTION >> -extern bool balloon_page_isolate(struct page *page); >> -extern void balloon_page_putback(struct page *page); >> -extern int balloon_page_migrate(struct page *newpage, >> - struct page *page, enum migrate_mode mode); >> +extern const struct address_space_operations balloon_aops; >> +extern int balloon_page_isolate(struct page *page); >> +extern int balloon_page_putback(struct page *page); >> +extern int balloon_page_migrate(struct address_space *mapping, >> + struct page *newpage, >> + struct page *page, >> + enum migrate_mode mode); >> + >> +extern struct address_space >> +*balloon_mapping_alloc(struct balloon_dev_info *b_dev_info, >> + const struct address_space_operations *a_ops); >> + >> +static inline void balloon_mapping_free(struct address_space *balloon_mapping) >> +{ >> + kfree(balloon_mapping); >> +} >> >> /* >> * __is_movable_balloon_page - helper to perform @page PageBalloon tests >> @@ -123,6 +136,7 @@ static inline bool isolated_balloon_page(struct page *page) >> static inline void balloon_page_insert(struct balloon_dev_info *balloon, >> struct page *page) >> { >> + page->mapping = balloon->mapping; >> __SetPageBalloon(page); >> SetPagePrivate(page); >> set_page_private(page, (unsigned long)balloon); >> @@ -139,6 +153,7 @@ static inline void balloon_page_insert(struct balloon_dev_info *balloon, >> */ >> static inline void balloon_page_delete(struct page *page) >> { >> + page->mapping = NULL; >> __ClearPageBalloon(page); >> set_page_private(page, 0); >> if (PagePrivate(page)) { >> diff --git a/include/linux/fs.h b/include/linux/fs.h >> index b4d71b5..de463b9 100644 >> --- a/include/linux/fs.h >> +++ b/include/linux/fs.h >> @@ -368,6 +368,9 @@ struct address_space_operations { >> */ >> int (*migratepage) (struct address_space *, >> struct page *, struct page *, enum migrate_mode); >> + int (*isolatepage)(struct page *); >> + int (*putbackpage)(struct page *); >> + >> int (*launder_page) (struct page *); >> int (*is_partially_uptodate) (struct page *, unsigned long, >> unsigned long); >> diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h >> index 4b3736f..715b5b2 100644 >> --- a/include/linux/pagemap.h >> +++ b/include/linux/pagemap.h >> @@ -25,6 +25,7 @@ enum mapping_flags { >> AS_MM_ALL_LOCKS = __GFP_BITS_SHIFT + 2, /* under mm_take_all_locks() */ >> AS_UNEVICTABLE = __GFP_BITS_SHIFT + 3, /* e.g., ramdisk, SHM_LOCK */ >> AS_EXITING = __GFP_BITS_SHIFT + 4, /* final truncate in progress */ >> + AS_MIGRATABLE = __GFP_BITS_SHIFT + 5, >> }; >> >> static inline void mapping_set_error(struct address_space *mapping, int error) >> @@ -54,6 +55,31 @@ static inline int mapping_unevictable(struct address_space *mapping) >> return !!mapping; >> } >> >> +static inline void mapping_set_migratable(struct address_space *mapping) >> +{ >> + set_bit(AS_MIGRATABLE, &mapping->flags); >> +} >> + >> +static inline void mapping_clear_migratable(struct address_space *mapping) >> +{ >> + clear_bit(AS_MIGRATABLE, &mapping->flags); >> +} >> + >> +static inline int __mapping_ops(struct address_space *mapping) >> +{ >> + return mapping->a_ops && >> + mapping->a_ops->migratepage && >> + mapping->a_ops->isolatepage && >> + mapping->a_ops->putbackpage; >> +} >> + >> +static inline int mapping_migratable(struct address_space *mapping) >> +{ >> + if (mapping && __mapping_ops(mapping)) > > s/__mapping_ops/__mapping_migratable/ given naming hurts brain? Good. I'll change it. Thank you. > >> + return test_bit(AS_MIGRATABLE, &mapping->flags); >> + return !!mapping; >> +} >> + >> static inline void mapping_set_exiting(struct address_space *mapping) >> { >> set_bit(AS_EXITING, &mapping->flags); >> diff --git a/mm/balloon_compaction.c b/mm/balloon_compaction.c >> index fcad832..2e9d635 100644 >> --- a/mm/balloon_compaction.c >> +++ b/mm/balloon_compaction.c >> @@ -131,8 +131,11 @@ static inline void __putback_balloon_page(struct page *page) >> } >> >> /* __isolate_lru_page() counterpart for a ballooned page */ >> -bool balloon_page_isolate(struct page *page) >> +int balloon_page_isolate(struct page *page) >> { >> + if (!balloon_page_movable(page)) >> + return false; >> + >> /* >> * Avoid burning cycles with pages that are yet under __free_pages(), >> * or just got freed under us. >> @@ -173,8 +176,11 @@ bool balloon_page_isolate(struct page *page) >> } >> >> /* putback_lru_page() counterpart for a ballooned page */ >> -void balloon_page_putback(struct page *page) >> +int balloon_page_putback(struct page *page) >> { >> + if (!isolated_balloon_page(page)) >> + return 0; >> + >> /* >> * 'lock_page()' stabilizes the page and prevents races against >> * concurrent isolation threads attempting to re-isolate it. >> @@ -190,15 +196,20 @@ void balloon_page_putback(struct page *page) >> dump_page(page, "not movable balloon page"); >> } >> unlock_page(page); >> + return 0; >> } >> >> /* move_to_new_page() counterpart for a ballooned page */ >> -int balloon_page_migrate(struct page *newpage, >> +int balloon_page_migrate(struct address_space *mapping, >> + struct page *newpage, > > Looks the newly added mapping is not necessary, yes? Please refer to struct address_space_operations. int (*migratepage) (struct address_space *, struct page *, struct page *, enum migrate_mode); >> struct page *page, enum migrate_mode mode) >> { >> struct balloon_dev_info *balloon = balloon_page_device(page); >> int rc = -EAGAIN; >> >> + if (!isolated_balloon_page(page)) >> + return 0; >> + >> /* >> * Block others from accessing the 'newpage' when we get around to >> * establishing additional references. We should be the only one >> @@ -218,4 +229,55 @@ int balloon_page_migrate(struct page *newpage, >> unlock_page(newpage); >> return rc; >> } >> + >> +/* define the balloon_mapping->a_ops callback to allow balloon page migration */ >> +const struct address_space_operations balloon_aops = { >> + .migratepage = balloon_page_migrate, >> + .isolatepage = balloon_page_isolate, >> + .putbackpage = balloon_page_putback, >> +}; >> +EXPORT_SYMBOL_GPL(balloon_aops); > > -- 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/