linux-kernel - Re: [patch v2 2/4] mm, compaction: return failed migration target pages back to freelist

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]

Message-ID: <5363B854.3010401@suse.cz>
Date:	Fri, 02 May 2014 17:23:00 +0200
From:	Vlastimil Babka <vbabka@...e.cz>
To:	David Rientjes <rientjes@...gle.com>,
	Andrew Morton <akpm@...ux-foundation.org>
CC:	Mel Gorman <mgorman@...e.de>, Rik van Riel <riel@...hat.com>,
	Joonsoo Kim <iamjoonsoo.kim@....com>,
	Greg Thelen <gthelen@...gle.com>,
	Hugh Dickins <hughd@...gle.com>, linux-kernel@...r.kernel.org,
	linux-mm@...ck.org
Subject: Re: [patch v2 2/4] mm, compaction: return failed migration target
 pages back to freelist

On 05/01/2014 11:35 PM, David Rientjes wrote:
> Memory compaction works by having a "freeing scanner" scan from one end of a
> zone which isolates pages as migration targets while another "migrating scanner"
> scans from the other end of the same zone which isolates pages for migration.
>
> When page migration fails for an isolated page, the target page is returned to
> the system rather than the freelist built by the freeing scanner.  This may
> require the freeing scanner to continue scanning memory after suitable migration
> targets have already been returned to the system needlessly.
>
> This patch returns destination pages to the freeing scanner freelist when page
> migration fails.  This prevents unnecessary work done by the freeing scanner but
> also encourages memory to be as compacted as possible at the end of the zone.

Note that the free scanner work can be further reduced as it does not 
anymore need to restart on the highest pageblock where it isolated 
something. It now does that exactly for the reason that something might 
have been returned back to the allocator and we don't want to miss that.
I'll reply with a patch for that shortly (that applies on top of 
isolate_freepages() changes in -mm plus Joonsoo's not-yet-included patch 
from http://marc.info/?l=linux-mm&m=139841452326021&w=2 )

> Reported-by: Greg Thelen <gthelen@...gle.com>
> Signed-off-by: David Rientjes <rientjes@...gle.com>

Acked-by: Vlastimil Babka <vbabka@...e.cz>

> ---
>   mm/compaction.c | 27 ++++++++++++++++++---------
>   1 file changed, 18 insertions(+), 9 deletions(-)
>
> diff --git a/mm/compaction.c b/mm/compaction.c
> --- a/mm/compaction.c
> +++ b/mm/compaction.c
> @@ -797,23 +797,32 @@ static struct page *compaction_alloc(struct page *migratepage,
>   }
>
>   /*
> - * We cannot control nr_migratepages and nr_freepages fully when migration is
> - * running as migrate_pages() has no knowledge of compact_control. When
> - * migration is complete, we count the number of pages on the lists by hand.
> + * This is a migrate-callback that "frees" freepages back to the isolated
> + * freelist.  All pages on the freelist are from the same zone, so there is no
> + * special handling needed for NUMA.
> + */
> +static void compaction_free(struct page *page, unsigned long data)
> +{
> +	struct compact_control *cc = (struct compact_control *)data;
> +
> +	list_add(&page->lru, &cc->freepages);
> +	cc->nr_freepages++;
> +}
> +
> +/*
> + * We cannot control nr_migratepages fully when migration is running as
> + * migrate_pages() has no knowledge of of compact_control.  When migration is
> + * complete, we count the number of pages on the list by hand.

Actually migrate_pages() returns the number of failed pages except when 
it returns an error. And then we only use the value for the tracepoint. 
I'll send a followup patch for this.

>    */
>   static void update_nr_listpages(struct compact_control *cc)
>   {
>   	int nr_migratepages = 0;
> -	int nr_freepages = 0;
>   	struct page *page;
>
>   	list_for_each_entry(page, &cc->migratepages, lru)
>   		nr_migratepages++;
> -	list_for_each_entry(page, &cc->freepages, lru)
> -		nr_freepages++;
>
>   	cc->nr_migratepages = nr_migratepages;
> -	cc->nr_freepages = nr_freepages;
>   }
>
>   /* possible outcome of isolate_migratepages */
> @@ -1023,8 +1032,8 @@ static int compact_zone(struct zone *zone, struct compact_control *cc)
>   		}
>
>   		nr_migrate = cc->nr_migratepages;
> -		err = migrate_pages(&cc->migratepages, compaction_alloc, NULL,
> -				(unsigned long)cc,
> +		err = migrate_pages(&cc->migratepages, compaction_alloc,
> +				compaction_free, (unsigned long)cc,
>   				cc->sync ? MIGRATE_SYNC_LIGHT : MIGRATE_ASYNC,
>   				MR_COMPACTION);
>   		update_nr_listpages(cc);
>

--
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/