[PATCH 6.1.y 0/7] Backport patches for DAMON merge regions fix

[PATCH 6.1.y 0/7] Backport patches for DAMON merge regions fix

[SeongJae Park]

Commit 310d6c15e910 ("mm/damon/core: merge regions aggressively when
max_nr_regions") causes a build warning [1] on 6.1.y.  That was due to
unnecessarily strict type check from max().

Fix the warning by backporting a minmax.h upstream commit that made the
type check less strict for unnecessary case, and upstream commits that
it depends on.

Note that all patches except the third one ("minmax: fix header
inclusions") are clean cherry-picks of upstream commit.  For the third
one, a minor conflict fix was needed.

[1] https://lore.kernel.org/2024071519-janitor-robe-779f@gregkh

Andy Shevchenko (1):
  minmax: fix header inclusions

David Laight (3):
  minmax: allow min()/max()/clamp() if the arguments have the same
    signedness.
  minmax: allow comparisons of 'int' against 'unsigned char/short'
  minmax: relax check to allow comparison between unsigned arguments and
    signed constants

Jason A. Donenfeld (2):
  minmax: sanity check constant bounds when clamping
  minmax: clamp more efficiently by avoiding extra comparison

SeongJae Park (1):
  mm/damon/core: merge regions aggressively when max_nr_regions is unmet

 include/linux/minmax.h | 89 ++++++++++++++++++++++++++++++------------
 mm/damon/core.c        | 21 +++++++++-
 2 files changed, 83 insertions(+), 27 deletions(-)


base-commit: 291e563ecab1ea89c70172ecf0d6bff7b725d3cb
-- 
2.39.2

[PATCH 6.1.y 7/7] mm/damon/core: merge regions aggressively when max_nr_regions is unmet

[SeongJae Park]

commit 310d6c15e9104c99d5d9d0ff8e5383a79da7d5e6 upstream.

DAMON keeps the number of regions under max_nr_regions by skipping regions
split operations when doing so can make the number higher than the limit.
It works well for preventing violation of the limit.  But, if somehow the
violation happens, it cannot recovery well depending on the situation.  In
detail, if the real number of regions having different access pattern is
higher than the limit, the mechanism cannot reduce the number below the
limit.  In such a case, the system could suffer from high monitoring
overhead of DAMON.

The violation can actually happen.  For an example, the user could reduce
max_nr_regions while DAMON is running, to be lower than the current number
of regions.  Fix the problem by repeating the merge operations with
increasing aggressiveness in kdamond_merge_regions() for the case, until
the limit is met.

[sj@kernel.org: increase regions merge aggressiveness while respecting min_nr_regions]
  Link: https://lkml.kernel.org/r/20240626164753.46270-1-sj@kernel.org
[sj@kernel.org: ensure max threshold attempt for max_nr_regions violation]
  Link: https://lkml.kernel.org/r/20240627163153.75969-1-sj@kernel.org
Link: https://lkml.kernel.org/r/20240624175814.89611-1-sj@kernel.org
Fixes: b9a6ac4e4ede ("mm/damon: adaptively adjust regions")
Signed-off-by: SeongJae Park <sj@kernel.org>
Cc: <stable@vger.kernel.org>	[5.15+]
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
(cherry picked from commit 310d6c15e9104c99d5d9d0ff8e5383a79da7d5e6)
---
 mm/damon/core.c | 21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/mm/damon/core.c b/mm/damon/core.c
index 5db9bec8ae67..ab5c351b276c 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -921,14 +921,31 @@ static void damon_merge_regions_of(struct damon_target *t, unsigned int thres,
  * access frequencies are similar.  This is for minimizing the monitoring
  * overhead under the dynamically changeable access pattern.  If a merge was
  * unnecessarily made, later 'kdamond_split_regions()' will revert it.
+ *
+ * The total number of regions could be higher than the user-defined limit,
+ * max_nr_regions for some cases.  For example, the user can update
+ * max_nr_regions to a number that lower than the current number of regions
+ * while DAMON is running.  For such a case, repeat merging until the limit is
+ * met while increasing @threshold up to possible maximum level.
  */
 static void kdamond_merge_regions(struct damon_ctx *c, unsigned int threshold,
 				  unsigned long sz_limit)
 {
 	struct damon_target *t;
+	unsigned int nr_regions;
+	unsigned int max_thres;
 
-	damon_for_each_target(t, c)
-		damon_merge_regions_of(t, threshold, sz_limit);
+	max_thres = c->attrs.aggr_interval /
+		(c->attrs.sample_interval ?  c->attrs.sample_interval : 1);
+	do {
+		nr_regions = 0;
+		damon_for_each_target(t, c) {
+			damon_merge_regions_of(t, threshold, sz_limit);
+			nr_regions += damon_nr_regions(t);
+		}
+		threshold = max(1, threshold * 2);
+	} while (nr_regions > c->attrs.max_nr_regions &&
+			threshold / 2 < max_thres);
 }
 
 /*
-- 
2.39.2

Re: [PATCH 6.1.y 0/7] Backport patches for DAMON merge regions fix

[Greg KH]

On Tue, Jul 16, 2024 at 10:51:58AM -0700, SeongJae Park wrote:
> Commit 310d6c15e910 ("mm/damon/core: merge regions aggressively when
> max_nr_regions") causes a build warning [1] on 6.1.y.  That was due to
> unnecessarily strict type check from max().
> 
> Fix the warning by backporting a minmax.h upstream commit that made the
> type check less strict for unnecessary case, and upstream commits that
> it depends on.
> 
> Note that all patches except the third one ("minmax: fix header
> inclusions") are clean cherry-picks of upstream commit.  For the third
> one, a minor conflict fix was needed.
> 
> [1] https://lore.kernel.org/2024071519-janitor-robe-779f@gregkh

Thanks for these, I'll queue them up after this round of -rc releases go
out in a few days.

greg k-h

Patch "mm/damon/core: merge regions aggressively when max_nr_regions is unmet" has been added to the 6.1-stable tree

[gregkh]

This is a note to let you know that I've just added the patch titled

    mm/damon/core: merge regions aggressively when max_nr_regions is unmet

to the 6.1-stable tree which can be found at:
    http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
     mm-damon-core-merge-regions-aggressively-when-max_nr_regions-is-unmet.patch
and it can be found in the queue-6.1 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@vger.kernel.org> know about it.


From stable+bounces-60285-greg=kroah.com@vger.kernel.org Tue Jul 16 19:53:31 2024
From: SeongJae Park <sj@kernel.org>
Date: Tue, 16 Jul 2024 10:52:05 -0700
Subject: mm/damon/core: merge regions aggressively when max_nr_regions is unmet
To: stable@vger.kernel.org, gregkh@linuxfoundation.org
Cc: SeongJae Park <sj@kernel.org>, Andrew Morton <akpm@linux-foundation.org>, damon@lists.linux.dev, linux-mm@kvack.org, linux-kernel@vger.kernel.org
Message-ID: <20240716175205.51280-8-sj@kernel.org>

From: SeongJae Park <sj@kernel.org>

commit 310d6c15e9104c99d5d9d0ff8e5383a79da7d5e6 upstream.

DAMON keeps the number of regions under max_nr_regions by skipping regions
split operations when doing so can make the number higher than the limit.
It works well for preventing violation of the limit.  But, if somehow the
violation happens, it cannot recovery well depending on the situation.  In
detail, if the real number of regions having different access pattern is
higher than the limit, the mechanism cannot reduce the number below the
limit.  In such a case, the system could suffer from high monitoring
overhead of DAMON.

The violation can actually happen.  For an example, the user could reduce
max_nr_regions while DAMON is running, to be lower than the current number
of regions.  Fix the problem by repeating the merge operations with
increasing aggressiveness in kdamond_merge_regions() for the case, until
the limit is met.

[sj@kernel.org: increase regions merge aggressiveness while respecting min_nr_regions]
  Link: https://lkml.kernel.org/r/20240626164753.46270-1-sj@kernel.org
[sj@kernel.org: ensure max threshold attempt for max_nr_regions violation]
  Link: https://lkml.kernel.org/r/20240627163153.75969-1-sj@kernel.org
Link: https://lkml.kernel.org/r/20240624175814.89611-1-sj@kernel.org
Fixes: b9a6ac4e4ede ("mm/damon: adaptively adjust regions")
Signed-off-by: SeongJae Park <sj@kernel.org>
Cc: <stable@vger.kernel.org>	[5.15+]
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
(cherry picked from commit 310d6c15e9104c99d5d9d0ff8e5383a79da7d5e6)
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 mm/damon/core.c |   21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -921,14 +921,31 @@ static void damon_merge_regions_of(struc
  * access frequencies are similar.  This is for minimizing the monitoring
  * overhead under the dynamically changeable access pattern.  If a merge was
  * unnecessarily made, later 'kdamond_split_regions()' will revert it.
+ *
+ * The total number of regions could be higher than the user-defined limit,
+ * max_nr_regions for some cases.  For example, the user can update
+ * max_nr_regions to a number that lower than the current number of regions
+ * while DAMON is running.  For such a case, repeat merging until the limit is
+ * met while increasing @threshold up to possible maximum level.
  */
 static void kdamond_merge_regions(struct damon_ctx *c, unsigned int threshold,
 				  unsigned long sz_limit)
 {
 	struct damon_target *t;
+	unsigned int nr_regions;
+	unsigned int max_thres;
 
-	damon_for_each_target(t, c)
-		damon_merge_regions_of(t, threshold, sz_limit);
+	max_thres = c->attrs.aggr_interval /
+		(c->attrs.sample_interval ?  c->attrs.sample_interval : 1);
+	do {
+		nr_regions = 0;
+		damon_for_each_target(t, c) {
+			damon_merge_regions_of(t, threshold, sz_limit);
+			nr_regions += damon_nr_regions(t);
+		}
+		threshold = max(1, threshold * 2);
+	} while (nr_regions > c->attrs.max_nr_regions &&
+			threshold / 2 < max_thres);
 }
 
 /*


Patches currently in stable-queue which might be from kroah.com@vger.kernel.org are

queue-6.1/minmax-allow-min-max-clamp-if-the-arguments-have-the-same-signedness.patch
queue-6.1/minmax-sanity-check-constant-bounds-when-clamping.patch
queue-6.1/minmax-clamp-more-efficiently-by-avoiding-extra-comparison.patch
queue-6.1/minmax-relax-check-to-allow-comparison-between-unsigned-arguments-and-signed-constants.patch
queue-6.1/minmax-fix-header-inclusions.patch
queue-6.1/mm-damon-core-merge-regions-aggressively-when-max_nr_regions-is-unmet.patch
queue-6.1/minmax-allow-comparisons-of-int-against-unsigned-char-short.patch

Re: [PATCH 6.1.y 0/7] Backport patches for DAMON merge regions fix

[Greg KH]

On Tue, Jul 16, 2024 at 08:23:46PM +0200, Greg KH wrote:
> On Tue, Jul 16, 2024 at 10:51:58AM -0700, SeongJae Park wrote:
> > Commit 310d6c15e910 ("mm/damon/core: merge regions aggressively when
> > max_nr_regions") causes a build warning [1] on 6.1.y.  That was due to
> > unnecessarily strict type check from max().
> > 
> > Fix the warning by backporting a minmax.h upstream commit that made the
> > type check less strict for unnecessary case, and upstream commits that
> > it depends on.
> > 
> > Note that all patches except the third one ("minmax: fix header
> > inclusions") are clean cherry-picks of upstream commit.  For the third
> > one, a minor conflict fix was needed.
> > 
> > [1] https://lore.kernel.org/2024071519-janitor-robe-779f@gregkh
> 
> Thanks for these, I'll queue them up after this round of -rc releases go
> out in a few days.

All now queued up, MANY thanks for this, it fixed a different build
error that just got added to the 6.1 queue yesterday, as this problem
kept happening over and over due to backports breaking in different
ways.

greg k-h