linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: SeongJae Park <sjpark@amazon.com>
To: <Jonathan.Cameron@Huawei.com>, SeongJae Park <sjpark@amazon.com>
Cc: <akpm@linux-foundation.org>, SeongJae Park <sjpark@amazon.de>,
	<aarcange@redhat.com>, <acme@kernel.org>,
	<alexander.shishkin@linux.intel.com>, <amit@kernel.org>,
	<benh@kernel.crashing.org>, <brendan.d.gregg@gmail.com>,
	<brendanhiggins@google.com>, <cai@lca.pw>,
	<colin.king@canonical.com>, <corbet@lwn.net>, <dwmw@amazon.com>,
	<irogers@google.com>, <jolsa@redhat.com>, <kirill@shutemov.name>,
	<mark.rutland@arm.com>, <mgorman@suse.de>, <minchan@kernel.org>,
	<mingo@redhat.com>, <namhyung@kernel.org>, <peterz@infradead.org>,
	<rdunlap@infradead.org>, <riel@surriel.com>,
	<rientjes@google.com>, <rostedt@goodmis.org>, <sblbir@amazon.com>,
	<shakeelb@google.com>, <shuah@kernel.org>, <sj38.park@gmail.com>,
	<snu@amazon.de>, <vbabka@suse.cz>, <vdavydov.dev@gmail.com>,
	<yang.shi@linux.alibaba.com>, <ying.huang@intel.com>,
	<linux-damon@amazon.com>, <linux-mm@kvack.org>,
	<linux-doc@vger.kernel.org>, <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v10 06/16] mm/damon: Split regions into 4 subregions if possible
Date: Thu, 7 May 2020 10:28:36 +0200	[thread overview]
Message-ID: <20200507082836.32145-1-sjpark@amazon.com> (raw)
In-Reply-To: <20200505110815.10532-7-sjpark@amazon.com> (raw)

On Tue, 5 May 2020 13:08:05 +0200 SeongJae Park <sjpark@amazon.com> wrote:

> From: SeongJae Park <sjpark@amazon.de>
> 
> Suppose that there are a very wide and cold region and a hot region, and
> both regions are identified by DAMON.  And then, the middle small region
> inside the very wide and cold region becomes hot.  DAMON will not be
> able to identify this new region because the adaptive regions adjustment
> mechanism splits each region to only two subregions.
> 
> This commit modifies the logic to split each region to 4 subregions if
> possible so that such problematic region can eventually identified.

As you can check with the 'Evaluations' section in the cover letter of this
patchset, the extend of the slowdown DAMON's recording feature made to the
target workload has doulbed (0.55% with v9, 1.02% with this patchset).  It's
still just a small number, but the worst case slowdown is 5.29%.  It was only
1.88% before.  I suspect this patch is the reason, as this is the only one
major change we made after v9.

For the reason, I would like to make this patch to split each region into 3 sub
regions instead of 4 subregions.  It will reduce the overhead a little bit
while still allow the corner case regions be eventually detected.  If you have
other concerns or opinions, please let me know.


Thanks,
SeongJae Park

> 
> Suggested-by: James Cameron <Jonathan.Cameron@Huawei.com>
> Signed-off-by: SeongJae Park <sjpark@amazon.de>
> ---
>  mm/damon.c | 44 +++++++++++++++++++++++++++-----------------
>  1 file changed, 27 insertions(+), 17 deletions(-)
> 
> diff --git a/mm/damon.c b/mm/damon.c
> index cec946197e13..342f905927a0 100644
> --- a/mm/damon.c
> +++ b/mm/damon.c
> @@ -650,26 +650,32 @@ static void damon_split_region_at(struct damon_ctx *ctx,
>  	damon_insert_region(new, r, damon_next_region(r));
>  }
>  
> -/* Split every region in the given task into two randomly-sized regions */
> -static void damon_split_regions_of(struct damon_ctx *ctx, struct damon_task *t)
> +/* Split every region in the given task into 'nr_subs' regions */
> +static void damon_split_regions_of(struct damon_ctx *ctx,
> +				     struct damon_task *t, int nr_subs)
>  {
>  	struct damon_region *r, *next;
> -	unsigned long sz_orig_region, sz_left_region;
> +	unsigned long sz_region, sz_sub = 0;
> +	int i;
>  
>  	damon_for_each_region_safe(r, next, t) {
> -		sz_orig_region = r->vm_end - r->vm_start;
> -
> -		/*
> -		 * Randomly select size of left sub-region to be at least
> -		 * 10 percent and at most 90% of original region
> -		 */
> -		sz_left_region = ALIGN_DOWN(damon_rand(1, 10) * sz_orig_region
> -					    / 10, MIN_REGION);
> -		/* Do not allow blank region */
> -		if (sz_left_region == 0 || sz_left_region >= sz_orig_region)
> -			continue;
> -
> -		damon_split_region_at(ctx, r, sz_left_region);
> +		sz_region = r->vm_end - r->vm_start;
> +
> +		for (i = 0; i < nr_subs - 1 &&
> +				sz_region > 2 * MIN_REGION; i++) {
> +			/*
> +			 * Randomly select size of left sub-region to be at
> +			 * least 10 percent and at most 90% of original region
> +			 */
> +			sz_sub = ALIGN_DOWN(damon_rand(1, 10) *
> +					sz_region / 10, MIN_REGION);
> +			/* Do not allow blank region */
> +			if (sz_sub == 0 || sz_sub >= sz_region)
> +				continue;
> +
> +			damon_split_region_at(ctx, r, sz_sub);
> +			sz_region = sz_sub;
> +		}
>  	}
>  }
>  
> @@ -687,14 +693,18 @@ static void kdamond_split_regions(struct damon_ctx *ctx)
>  {
>  	struct damon_task *t;
>  	unsigned int nr_regions = 0;
> +	int nr_subregions = 2;
>  
>  	damon_for_each_task(ctx, t)
>  		nr_regions += nr_damon_regions(t);
>  	if (nr_regions > ctx->max_nr_regions / 2)
>  		return;
>  
> +	if (nr_regions < ctx->max_nr_regions / 4)
> +		nr_subregions = 4;
> +
>  	damon_for_each_task(ctx, t)
> -		damon_split_regions_of(ctx, t);
> +		damon_split_regions_of(ctx, t, nr_subregions);
>  }
>  
>  /*
> -- 
> 2.17.1
> 


  reply	other threads:[~2020-05-07  8:29 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-05 11:07 [PATCH v10 00/16] Introduce Data Access MONitor (DAMON) SeongJae Park
2020-05-05 11:08 ` [PATCH v10 01/16] scripts/spelling: Add a few more typos SeongJae Park
2020-05-05 11:08 ` [PATCH v10 02/16] mm/page_ext: Export lookup_page_ext() to GPL modules SeongJae Park
2020-05-05 11:08 ` [PATCH v10 03/16] mm: Introduce Data Access MONitor (DAMON) SeongJae Park
2020-05-05 11:08 ` [PATCH v10 04/16] mm/damon: Implement region based sampling SeongJae Park
2020-05-05 11:08 ` [PATCH v10 05/16] mm/damon: Adaptively adjust regions SeongJae Park
2020-05-05 11:08 ` [PATCH v10 06/16] mm/damon: Split regions into 4 subregions if possible SeongJae Park
2020-05-07  8:28   ` SeongJae Park [this message]
2020-05-05 11:08 ` [PATCH v10 07/16] mm/damon: Apply dynamic memory mapping changes SeongJae Park
2020-05-05 11:08 ` [PATCH v10 08/16] mm/damon: Implement callbacks SeongJae Park
2020-05-05 11:08 ` [PATCH v10 09/16] mm/damon: Implement access pattern recording SeongJae Park
2020-05-05 11:08 ` [PATCH v10 10/16] mm/damon: Add debugfs interface SeongJae Park
2020-05-05 11:08 ` [PATCH v10 11/16] mm/damon: Add tracepoints SeongJae Park
2020-05-05 11:08 ` [PATCH v10 12/16] tools: Add a minimal user-space tool for DAMON SeongJae Park
2020-05-05 11:08 ` [PATCH v10 13/16] Documentation/admin-guide/mm: Add a document " SeongJae Park
2020-05-05 11:08 ` [PATCH v10 14/16] mm/damon: Add kunit tests SeongJae Park
2020-05-05 11:08 ` [PATCH v10 15/16] mm/damon: Add user space selftests SeongJae Park
2020-05-05 11:08 ` [PATCH v10 16/16] MAINTAINERS: Update for DAMON SeongJae Park

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=20200507082836.32145-1-sjpark@amazon.com \
    --to=sjpark@amazon.com \
    --cc=Jonathan.Cameron@Huawei.com \
    --cc=aarcange@redhat.com \
    --cc=acme@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=amit@kernel.org \
    --cc=benh@kernel.crashing.org \
    --cc=brendan.d.gregg@gmail.com \
    --cc=brendanhiggins@google.com \
    --cc=cai@lca.pw \
    --cc=colin.king@canonical.com \
    --cc=corbet@lwn.net \
    --cc=dwmw@amazon.com \
    --cc=irogers@google.com \
    --cc=jolsa@redhat.com \
    --cc=kirill@shutemov.name \
    --cc=linux-damon@amazon.com \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mark.rutland@arm.com \
    --cc=mgorman@suse.de \
    --cc=minchan@kernel.org \
    --cc=mingo@redhat.com \
    --cc=namhyung@kernel.org \
    --cc=peterz@infradead.org \
    --cc=rdunlap@infradead.org \
    --cc=riel@surriel.com \
    --cc=rientjes@google.com \
    --cc=rostedt@goodmis.org \
    --cc=sblbir@amazon.com \
    --cc=shakeelb@google.com \
    --cc=shuah@kernel.org \
    --cc=sj38.park@gmail.com \
    --cc=sjpark@amazon.de \
    --cc=snu@amazon.de \
    --cc=vbabka@suse.cz \
    --cc=vdavydov.dev@gmail.com \
    --cc=yang.shi@linux.alibaba.com \
    --cc=ying.huang@intel.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;
as well as URLs for NNTP newsgroup(s).