linux-kernel - Re: [PATCH v2 1/2] mm/compaction: do not count migratepages when unnecessary

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: <xa1ty4ybvtq4.fsf@mina86.com>
Date:	Fri, 09 May 2014 08:48:19 -0700
From:	Michal Nazarewicz <mina86@...a86.com>
To:	Vlastimil Babka <vbabka@...e.cz>,
	Andrew Morton <akpm@...ux-foundation.org>,
	David Rientjes <rientjes@...gle.com>
Cc:	Hugh Dickins <hughd@...gle.com>, Greg Thelen <gthelen@...gle.com>,
	linux-kernel@...r.kernel.org, linux-mm@...ck.org,
	Vlastimil Babka <vbabka@...e.cz>,
	Minchan Kim <minchan@...nel.org>, Mel Gorman <mgorman@...e.de>,
	Joonsoo Kim <iamjoonsoo.kim@....com>,
	Bartlomiej Zolnierkiewicz <b.zolnierkie@...sung.com>,
	Christoph Lameter <cl@...ux.com>,
	Rik van Riel <riel@...hat.com>
Subject: Re: [PATCH v2 1/2] mm/compaction: do not count migratepages when unnecessary

On Wed, May 07 2014, Vlastimil Babka wrote:
> During compaction, update_nr_listpages() has been used to count remaining
> non-migrated and free pages after a call to migrage_pages(). The freepages
> counting has become unneccessary, and it turns out that migratepages counting
> is also unnecessary in most cases.
>
> The only situation when it's needed to count cc->migratepages is when
> migrate_pages() returns with a negative error code. Otherwise, the non-negative
> return value is the number of pages that were not migrated, which is exactly
> the count of remaining pages in the cc->migratepages list.
>
> Furthermore, any non-zero count is only interesting for the tracepoint of
> mm_compaction_migratepages events, because after that all remaining unmigrated
> pages are put back and their count is set to 0.
>
> This patch therefore removes update_nr_listpages() completely, and changes the
> tracepoint definition so that the manual counting is done only when the
> tracepoint is enabled, and only when migrate_pages() returns a negative error
> code.
>
> Furthermore, migrate_pages() and the tracepoints won't be called when there's
> nothing to migrate. This potentially avoids some wasted cycles and reduces the
> volume of uninteresting mm_compaction_migratepages events where "nr_migrated=0
> nr_failed=0". In the stress-highalloc mmtest, this was about 75% of the events.
> The mm_compaction_isolate_migratepages event is better for determining that
> nothing was isolated for migration, and this one was just duplicating the info.
>
> Signed-off-by: Vlastimil Babka <vbabka@...e.cz>
> Reviewed-by: Naoya Horiguchi <n-horiguchi@...jp.nec.com>
> Cc: Minchan Kim <minchan@...nel.org>
> Cc: Mel Gorman <mgorman@...e.de>
> Cc: Joonsoo Kim <iamjoonsoo.kim@....com>
> Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@...sung.com>
> Cc: Michal Nazarewicz <mina86@...a86.com>

Acked-by: Michal Nazarewicz <mina86@...a86.com>

One tiny comment below:

> Cc: Christoph Lameter <cl@...ux.com>
> Cc: Rik van Riel <riel@...hat.com>
> ---
>  v2: checkpack and other non-functional fixes suggested by Naoya Horiguchi
>
>  include/trace/events/compaction.h | 26 ++++++++++++++++++++++----
>  mm/compaction.c                   | 31 +++++++------------------------
>  2 files changed, 29 insertions(+), 28 deletions(-)
>
> diff --git a/include/trace/events/compaction.h b/include/trace/events/compaction.h
> index 06f544e..aacaf0f 100644
> --- a/include/trace/events/compaction.h
> +++ b/include/trace/events/compaction.h
> @@ -58,7 +61,22 @@ TRACE_EVENT(mm_compaction_migratepages,
>  	),
>  
>  	TP_fast_assign(
> -		__entry->nr_migrated = nr_migrated;
> +		unsigned long nr_failed = 0;
> +		struct page *page;
> +
> +		/*
> +		 * migrate_pages() returns either a non-negative number
> +		 * with the number of pages that failed migration, or an
> +		 * error code, in which case we need to count the remaining
> +		 * pages manually
> +		 */
> +		if (migrate_rc >= 0)
> +			nr_failed = migrate_rc;
> +		else
> +			list_for_each_entry(page, migratepages, lru)
> +				nr_failed++;

list_for_each would suffice here.

> +
> +		__entry->nr_migrated = nr_all - nr_failed;
>  		__entry->nr_failed = nr_failed;
>  	),
>  

-- 
Best regards,                                         _     _
.o. | Liege of Serenely Enlightened Majesty of      o' \,=./ `o
..o | Computer Science,  Michał “mina86” Nazarewicz    (o o)
ooo +--<mpn@...gle.com>--<xmpp:mina86@...ber.org>--ooO--(_)--Ooo--


Download attachment "signature.asc" of type "application/pgp-signature" (836 bytes)