linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Vlastimil Babka <vbabka@suse.cz>
To: David Rientjes <rientjes@google.com>,
	Andrew Morton <akpm@linux-foundation.org>
Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com>,
	linux-kernel@vger.kernel.org, linux-mm@kvack.org
Subject: Re: [patch] mm, compaction: add vmstats for kcompactd work
Date: Thu, 8 Dec 2016 09:04:12 +0100	[thread overview]
Message-ID: <f25f8fb9-47a9-ebd9-5a7a-95ca6dc324c9@suse.cz> (raw)
In-Reply-To: <alpine.DEB.2.10.1612071749390.69852@chino.kir.corp.google.com>

On 12/08/2016 02:50 AM, David Rientjes wrote:
> A "compact_daemon_wake" vmstat exists that represents the number of times
> kcompactd has woken up.  This doesn't represent how much work it actually
> did, though.
>
> It's useful to understand how much compaction work is being done by
> kcompactd versus other methods such as direct compaction and explicitly
> triggered per-node (or system) compaction.
>
> This adds two new vmstats: "compact_daemon_migrate_scanned" and
> "compact_daemon_free_scanned" to represent the number of pages kcompactd
> has scanned as part of its migration scanner and freeing scanner,
> respectively.
>
> These values are still accounted for in the general
> "compact_migrate_scanned" and "compact_free_scanned" for compatibility.
>
> It could be argued that explicitly triggered compaction could also be
> tracked separately, and that could be added if others find it useful.
>
> Signed-off-by: David Rientjes <rientjes@google.com>

A bit of downside is that stats are only updated when compaction 
finishes, but I guess it's acceptable. Also I don't think the 
compact_control variables need the "total_" prefix, but no strong 
feelings. The explicit zero init should be also unnecessary.

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

> ---
>  include/linux/vm_event_item.h |  1 +
>  mm/compaction.c               | 22 +++++++++++++++++++---
>  mm/internal.h                 |  2 ++
>  mm/vmstat.c                   |  2 ++
>  4 files changed, 24 insertions(+), 3 deletions(-)
>
> diff --git a/include/linux/vm_event_item.h b/include/linux/vm_event_item.h
> --- a/include/linux/vm_event_item.h
> +++ b/include/linux/vm_event_item.h
> @@ -56,6 +56,7 @@ enum vm_event_item { PGPGIN, PGPGOUT, PSWPIN, PSWPOUT,
>  		COMPACTISOLATED,
>  		COMPACTSTALL, COMPACTFAIL, COMPACTSUCCESS,
>  		KCOMPACTD_WAKE,
> +		KCOMPACTD_MIGRATE_SCANNED, KCOMPACTD_FREE_SCANNED,
>  #endif
>  #ifdef CONFIG_HUGETLB_PAGE
>  		HTLB_BUDDY_PGALLOC, HTLB_BUDDY_PGALLOC_FAIL,
> diff --git a/mm/compaction.c b/mm/compaction.c
> --- a/mm/compaction.c
> +++ b/mm/compaction.c
> @@ -548,7 +548,7 @@ static unsigned long isolate_freepages_block(struct compact_control *cc,
>  	if (blockpfn == end_pfn)
>  		update_pageblock_skip(cc, valid_page, total_isolated, false);
>
> -	count_compact_events(COMPACTFREE_SCANNED, nr_scanned);
> +	cc->total_free_scanned += nr_scanned;
>  	if (total_isolated)
>  		count_compact_events(COMPACTISOLATED, total_isolated);
>  	return total_isolated;
> @@ -939,7 +939,7 @@ isolate_migratepages_block(struct compact_control *cc, unsigned long low_pfn,
>  	trace_mm_compaction_isolate_migratepages(start_pfn, low_pfn,
>  						nr_scanned, nr_isolated);
>
> -	count_compact_events(COMPACTMIGRATE_SCANNED, nr_scanned);
> +	cc->total_migrate_scanned += nr_scanned;
>  	if (nr_isolated)
>  		count_compact_events(COMPACTISOLATED, nr_isolated);
>
> @@ -1643,6 +1643,9 @@ static enum compact_result compact_zone(struct zone *zone, struct compact_contro
>  			zone->compact_cached_free_pfn = free_pfn;
>  	}
>
> +	count_compact_events(COMPACTMIGRATE_SCANNED, cc->total_migrate_scanned);
> +	count_compact_events(COMPACTFREE_SCANNED, cc->total_free_scanned);
> +
>  	trace_mm_compaction_end(start_pfn, cc->migrate_pfn,
>  				cc->free_pfn, end_pfn, sync, ret);
>
> @@ -1657,6 +1660,8 @@ static enum compact_result compact_zone_order(struct zone *zone, int order,
>  	struct compact_control cc = {
>  		.nr_freepages = 0,
>  		.nr_migratepages = 0,
> +		.total_migrate_scanned = 0,
> +		.total_free_scanned = 0,
>  		.order = order,
>  		.gfp_mask = gfp_mask,
>  		.zone = zone,
> @@ -1767,6 +1772,8 @@ static void compact_node(int nid)
>  	struct zone *zone;
>  	struct compact_control cc = {
>  		.order = -1,
> +		.total_migrate_scanned = 0,
> +		.total_free_scanned = 0,
>  		.mode = MIGRATE_SYNC,
>  		.ignore_skip_hint = true,
>  		.whole_zone = true,
> @@ -1892,6 +1899,8 @@ static void kcompactd_do_work(pg_data_t *pgdat)
>  	struct zone *zone;
>  	struct compact_control cc = {
>  		.order = pgdat->kcompactd_max_order,
> +		.total_migrate_scanned = 0,
> +		.total_free_scanned = 0,
>  		.classzone_idx = pgdat->kcompactd_classzone_idx,
>  		.mode = MIGRATE_SYNC_LIGHT,
>  		.ignore_skip_hint = true,
> @@ -1899,7 +1908,7 @@ static void kcompactd_do_work(pg_data_t *pgdat)
>  	};
>  	trace_mm_compaction_kcompactd_wake(pgdat->node_id, cc.order,
>  							cc.classzone_idx);
> -	count_vm_event(KCOMPACTD_WAKE);
> +	count_compact_event(KCOMPACTD_WAKE);
>
>  	for (zoneid = 0; zoneid <= cc.classzone_idx; zoneid++) {
>  		int status;
> @@ -1917,6 +1926,8 @@ static void kcompactd_do_work(pg_data_t *pgdat)
>
>  		cc.nr_freepages = 0;
>  		cc.nr_migratepages = 0;
> +		cc.total_migrate_scanned = 0;
> +		cc.total_free_scanned = 0;
>  		cc.zone = zone;
>  		INIT_LIST_HEAD(&cc.freepages);
>  		INIT_LIST_HEAD(&cc.migratepages);
> @@ -1935,6 +1946,11 @@ static void kcompactd_do_work(pg_data_t *pgdat)
>  			defer_compaction(zone, cc.order);
>  		}
>
> +		count_compact_events(KCOMPACTD_MIGRATE_SCANNED,
> +				     cc.total_migrate_scanned);
> +		count_compact_events(KCOMPACTD_FREE_SCANNED,
> +				     cc.total_free_scanned);
> +
>  		VM_BUG_ON(!list_empty(&cc.freepages));
>  		VM_BUG_ON(!list_empty(&cc.migratepages));
>  	}
> diff --git a/mm/internal.h b/mm/internal.h
> --- a/mm/internal.h
> +++ b/mm/internal.h
> @@ -173,6 +173,8 @@ struct compact_control {
>  	struct list_head migratepages;	/* List of pages being migrated */
>  	unsigned long nr_freepages;	/* Number of isolated free pages */
>  	unsigned long nr_migratepages;	/* Number of pages to migrate */
> +	unsigned long total_migrate_scanned;
> +	unsigned long total_free_scanned;
>  	unsigned long free_pfn;		/* isolate_freepages search base */
>  	unsigned long migrate_pfn;	/* isolate_migratepages search base */
>  	unsigned long last_migrated_pfn;/* Not yet flushed page being freed */
> diff --git a/mm/vmstat.c b/mm/vmstat.c
> --- a/mm/vmstat.c
> +++ b/mm/vmstat.c
> @@ -1038,6 +1038,8 @@ const char * const vmstat_text[] = {
>  	"compact_fail",
>  	"compact_success",
>  	"compact_daemon_wake",
> +	"compact_daemon_migrate_scanned",
> +	"compact_daemon_free_scanned",
>  #endif
>
>  #ifdef CONFIG_HUGETLB_PAGE
>
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo@kvack.org.  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
>

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  reply	other threads:[~2016-12-08  8:04 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-12-08  1:50 David Rientjes
2016-12-08  8:04 ` Vlastimil Babka [this message]
2016-12-08 12:35   ` Michal Hocko
2016-12-12 22:20   ` David Rientjes

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=f25f8fb9-47a9-ebd9-5a7a-95ca6dc324c9@suse.cz \
    --to=vbabka@suse.cz \
    --cc=akpm@linux-foundation.org \
    --cc=iamjoonsoo.kim@lge.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=rientjes@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox