| 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
| ||
|
Date: Wed, 4 Jul 2012 11:17:57 +0100
From: Mel Gorman <mel@....ul.ie>
To: Lai Jiangshan <laijs@...fujitsu.com>
Cc: Chris Metcalf <cmetcalf@...era.com>, Len Brown <lenb@...nel.org>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Andi Kleen <andi@...stfloor.org>, Julia Lawall <julia@...u.dk>,
David Howells <dhowells@...hat.com>,
Benjamin Herrenschmidt <benh@...nel.crashing.org>,
Kay Sievers <kay.sievers@...y.org>,
Ingo Molnar <mingo@...e.hu>,
Paul Gortmaker <paul.gortmaker@...driver.com>,
Daniel Kiper <dkiper@...-space.pl>,
Andrew Morton <akpm@...ux-foundation.org>,
Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>,
Michal Hocko <mhocko@...e.cz>,
KAMEZAWA Hiroyuki <kamezawa.hiroyu@...fujitsu.com>,
Minchan Kim <minchan@...nel.org>,
Michal Nazarewicz <mina86@...a86.com>,
Marek Szyprowski <m.szyprowski@...sung.com>,
Rik van Riel <riel@...hat.com>,
Bjorn Helgaas <bhelgaas@...gle.com>,
Christoph Lameter <cl@...ux.com>,
David Rientjes <rientjes@...gle.com>,
linux-kernel@...r.kernel.org, linux-acpi@...r.kernel.org,
linux-mm@...ck.org
Subject: Re: [RFC PATCH 1/3 V1] mm, page_alloc: use __rmqueue_smallest when
borrow memory from MIGRATE_CMA
On Wed, Jul 04, 2012 at 03:26:16PM +0800, Lai Jiangshan wrote:
> The pages of MIGRATE_CMA can't not be changed to the other type,
> nor be moved to the other free list.
>
> ==>
> So when we use __rmqueue_fallback() to borrow memory from MIGRATE_CMA,
> one of the highest order page is borrowed and it is split.
> But the free pages resulted by splitting can NOT
> be moved to MIGRATE_MOVABLE.
>
> ==>
> So in the next time of allocation, we NEED to borrow again,
> another one of the highest order page is borrowed from CMA and it is split.
> and results some other new split free pages.
>
Then special case __rmqueue_fallback() to move pages stolen from
MIGRATE_CMA to the MIGRATE_MOVABLE lists but do not change the pageblock
type.
> ==>
> So when __rmqueue_fallback() borrows (highest order)memory from MIGRATE_CMA,
> it introduces fragments at the same time and may waste tlb(only one page is used in
> a pageblock).
>
> Conclusion:
> We should borrows the smallest order memory from MIGRATE_CMA in such case
>
That's excessive and unnecessary special casing. Just move the
MIGRATE_CMA pages to the MIGRATE_MOVABLE lists.
> Result(good):
> 1) use __rmqueue_smallest when borrow memory from MIGRATE_CMA
> 2) __rmqueue_fallback() don't handle CMA, it becomes much simpler
> Result(bad):
> __rmqueue_smallest() can't not be inlined to avoid function call overhead.
>
> Signed-off-by: Lai Jiangshan <laijs@...fujitsu.com>
> ---
> include/linux/mmzone.h | 1 +
> mm/page_alloc.c | 63 ++++++++++++++++--------------------------------
> 2 files changed, 22 insertions(+), 42 deletions(-)
>
> diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
> index bf3404e..979c333 100644
> --- a/include/linux/mmzone.h
> +++ b/include/linux/mmzone.h
> @@ -40,6 +40,7 @@ enum {
> MIGRATE_RECLAIMABLE,
> MIGRATE_MOVABLE,
> MIGRATE_PCPTYPES, /* the number of types on the pcp lists */
> + MIGRATE_PRIME_TYPES = MIGRATE_PCPTYPES,
No explanation why this new name is necessary.
> MIGRATE_RESERVE = MIGRATE_PCPTYPES,
> #ifdef CONFIG_CMA
> /*
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 476ae3e..efc327f 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -893,17 +893,10 @@ struct page *__rmqueue_smallest(struct zone *zone, unsigned int order,
> * This array describes the order lists are fallen back to when
> * the free lists for the desirable migrate type are depleted
> */
> -static int fallbacks[MIGRATE_TYPES][4] = {
> - [MIGRATE_UNMOVABLE] = { MIGRATE_RECLAIMABLE, MIGRATE_MOVABLE, MIGRATE_RESERVE },
> - [MIGRATE_RECLAIMABLE] = { MIGRATE_UNMOVABLE, MIGRATE_MOVABLE, MIGRATE_RESERVE },
> -#ifdef CONFIG_CMA
> - [MIGRATE_MOVABLE] = { MIGRATE_CMA, MIGRATE_RECLAIMABLE, MIGRATE_UNMOVABLE, MIGRATE_RESERVE },
> - [MIGRATE_CMA] = { MIGRATE_RESERVE }, /* Never used */
> -#else
> - [MIGRATE_MOVABLE] = { MIGRATE_RECLAIMABLE, MIGRATE_UNMOVABLE, MIGRATE_RESERVE },
> -#endif
> - [MIGRATE_RESERVE] = { MIGRATE_RESERVE }, /* Never used */
> - [MIGRATE_ISOLATE] = { MIGRATE_RESERVE }, /* Never used */
> +static int fallbacks[MIGRATE_PRIME_TYPES][2] = {
> + [MIGRATE_UNMOVABLE] = { MIGRATE_RECLAIMABLE, MIGRATE_MOVABLE },
> + [MIGRATE_RECLAIMABLE] = { MIGRATE_UNMOVABLE, MIGRATE_MOVABLE },
> + [MIGRATE_MOVABLE] = { MIGRATE_RECLAIMABLE, MIGRATE_UNMOVABLE },
> };
>
And it's completely unclear why it was necessary to rip out the existing
fallback lists. It reworks how fallback lists work for no clear benefit.
> /*
> @@ -995,16 +988,15 @@ __rmqueue_fallback(struct zone *zone, int order, int start_migratetype)
> struct page *page;
> int migratetype, i;
>
> + if (WARN_ON_ONCE(start_migratetype >= MIGRATE_PRIME_TYPES))
> + start_migratetype = MIGRATE_UNMOVABLE;
> +
This should be completely unnecessary. If this warning is hit, the
callers are severely broken.
> /* Find the largest possible block of pages in the other list */
> for (current_order = MAX_ORDER-1; current_order >= order;
> --current_order) {
> - for (i = 0;; i++) {
> + for (i = 0; i < ARRAY_SIZE(fallbacks[0]); i++) {
> migratetype = fallbacks[start_migratetype][i];
>
> - /* MIGRATE_RESERVE handled later if necessary */
> - if (migratetype == MIGRATE_RESERVE)
> - break;
> -
> area = &(zone->free_area[current_order]);
> if (list_empty(&area->free_list[migratetype]))
> continue;
> @@ -1018,17 +1010,10 @@ __rmqueue_fallback(struct zone *zone, int order, int start_migratetype)
> * pages to the preferred allocation list. If falling
> * back for a reclaimable kernel allocation, be more
> * aggressive about taking ownership of free pages
> - *
> - * On the other hand, never change migration
> - * type of MIGRATE_CMA pageblocks nor move CMA
> - * pages on different free lists. We don't
> - * want unmovable pages to be allocated from
> - * MIGRATE_CMA areas.
> */
> - if (!is_migrate_cma(migratetype) &&
> - (unlikely(current_order >= pageblock_order / 2) ||
> - start_migratetype == MIGRATE_RECLAIMABLE ||
> - page_group_by_mobility_disabled)) {
> + if (unlikely(current_order >= pageblock_order / 2) ||
> + start_migratetype == MIGRATE_RECLAIMABLE ||
> + page_group_by_mobility_disabled) {
> int pages;
> pages = move_freepages_block(zone, page,
> start_migratetype);
> @@ -1047,14 +1032,12 @@ __rmqueue_fallback(struct zone *zone, int order, int start_migratetype)
> rmv_page_order(page);
>
> /* Take ownership for orders >= pageblock_order */
> - if (current_order >= pageblock_order &&
> - !is_migrate_cma(migratetype))
> + if (current_order >= pageblock_order)
> change_pageblock_range(page, current_order,
> start_migratetype);
>
> expand(zone, page, order, current_order, area,
> - is_migrate_cma(migratetype)
> - ? migratetype : start_migratetype);
> + start_migratetype);
>
> trace_mm_page_alloc_extfrag(page, order, current_order,
> start_migratetype, migratetype);
> @@ -1075,22 +1058,18 @@ static struct page *__rmqueue(struct zone *zone, unsigned int order,
> {
> struct page *page;
>
> -retry_reserve:
> page = __rmqueue_smallest(zone, order, migratetype);
>
> - if (unlikely(!page) && migratetype != MIGRATE_RESERVE) {
> +#ifdef CONFIG_CMA
> + if (unlikely(!page) && migratetype == MIGRATE_MOVABLE)
> + page = __rmqueue_smallest(zone, order, MIGRATE_CMA);
> +#endif
> +
> + if (unlikely(!page))
> page = __rmqueue_fallback(zone, order, migratetype);
>
> - /*
> - * Use MIGRATE_RESERVE rather than fail an allocation. goto
> - * is used because __rmqueue_smallest is an inline function
> - * and we want just one call site
> - */
> - if (!page) {
> - migratetype = MIGRATE_RESERVE;
> - goto retry_reserve;
> - }
> - }
> + if (unlikely(!page))
> + page = __rmqueue_smallest(zone, order, MIGRATE_RESERVE);
>
> trace_mm_page_alloc_zone_locked(page, order, migratetype);
> return page;
I didn't read this closely. It seems way more complex than necessary to
solve the problem described in the changelog. All you should need is to
special case that a __GFP_MOVABLE allocation using MIGRATE_CMA should
place the unused buddies on the MIGRATE_MOVABLE lists.
--
Mel Gorman
SUSE Labs
--
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/
Powered by blists - more mailing lists