FAILED: patch "[PATCH] mm/damon/core: merge regions aggressively when max_nr_regions" failed to apply to 6.1-stable tree

FAILED: patch "[PATCH] mm/damon/core: merge regions aggressively when max_nr_regions" failed to apply to 6.1-stable tree

[gregkh]

The patch below does not apply to the 6.1-stable tree.
If someone wants it applied there, or to any other stable or longterm
tree, then please email the backport, including the original git commit
id to <stable@vger.kernel.org>.

To reproduce the conflict and resubmit, you may use the following commands:

git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-6.1.y
git checkout FETCH_HEAD
git cherry-pick -x 310d6c15e9104c99d5d9d0ff8e5383a79da7d5e6
# <resolve conflicts, build, test, etc.>
git commit -s
git send-email --to '<stable@vger.kernel.org>' --in-reply-to '2024071547-slum-anemic-a0cc@gregkh' --subject-prefix 'PATCH 6.1.y' HEAD^..

Possible dependencies:

310d6c15e910 ("mm/damon/core: merge regions aggressively when max_nr_regions is unmet")

thanks,

greg k-h

------------------ original commit in Linus's tree ------------------

From 310d6c15e9104c99d5d9d0ff8e5383a79da7d5e6 Mon Sep 17 00:00:00 2001
From: SeongJae Park <sj@kernel.org>
Date: Mon, 24 Jun 2024 10:58:14 -0700
Subject: [PATCH] mm/damon/core: merge regions aggressively when max_nr_regions
 is unmet

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>

diff --git a/mm/damon/core.c b/mm/damon/core.c
index 6392f1cc97a3..e66823d6b10b 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -1358,14 +1358,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);
 }
 
 /*

Re: FAILED: patch "[PATCH] mm/damon/core: merge regions aggressively when max_nr_regions" failed to apply to 6.1-stable tree

[SeongJae Park]

Hi Greg,

On Mon, 15 Jul 2024 13:34:48 +0200 <gregkh@linuxfoundation.org> wrote:

> 
> The patch below does not apply to the 6.1-stable tree.
> If someone wants it applied there, or to any other stable or longterm
> tree, then please email the backport, including the original git commit
> id to <stable@vger.kernel.org>.
> 
> To reproduce the conflict and resubmit, you may use the following commands:
> 
> git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-6.1.y
> git checkout FETCH_HEAD
> git cherry-pick -x 310d6c15e9104c99d5d9d0ff8e5383a79da7d5e6

But this doesn't reproduce the failure on my machine, like below?

    $ git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-6.1.y
    [...]
    $ git checkout FETCH_HEAD
    [...]
    HEAD is now at cac15753b8ce Linux 6.1.99
    $ git cherry-pick -x 310d6c15e9104c99d5d9d0ff8e5383a79da7d5e6
    Auto-merging mm/damon/core.c
    [detached HEAD ecd04159c5f3] mm/damon/core: merge regions aggressively when max_nr_regions is unmet
     Date: Mon Jun 24 10:58:14 2024 -0700
     1 file changed, 19 insertions(+), 2 deletions(-)

Could the faulure depend on something such as git version?  For such a case,
attaching the resulting patch below.


Thanks,
SJ

[...]

====================== >8 ========================
From ecd04159c5f3c58dc540864d48e9480c7617717c Mon Sep 17 00:00:00 2001
From: SeongJae Park <sj@kernel.org>
Date: Mon, 24 Jun 2024 10:58:14 -0700
Subject: [PATCH 6.1.y] mm/damon/core: merge regions aggressively when
 max_nr_regions is unmet

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: FAILED: patch "[PATCH] mm/damon/core: merge regions aggressively when max_nr_regions" failed to apply to 6.1-stable tree

[Greg KH]

On Mon, Jul 15, 2024 at 09:57:17AM -0700, SeongJae Park wrote:
> Hi Greg,
> 
> On Mon, 15 Jul 2024 13:34:48 +0200 <gregkh@linuxfoundation.org> wrote:
> 
> > 
> > The patch below does not apply to the 6.1-stable tree.
> > If someone wants it applied there, or to any other stable or longterm
> > tree, then please email the backport, including the original git commit
> > id to <stable@vger.kernel.org>.
> > 
> > To reproduce the conflict and resubmit, you may use the following commands:
> > 
> > git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-6.1.y
> > git checkout FETCH_HEAD
> > git cherry-pick -x 310d6c15e9104c99d5d9d0ff8e5383a79da7d5e6
> 
> But this doesn't reproduce the failure on my machine, like below?
> 
>     $ git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-6.1.y
>     [...]
>     $ git checkout FETCH_HEAD
>     [...]
>     HEAD is now at cac15753b8ce Linux 6.1.99
>     $ git cherry-pick -x 310d6c15e9104c99d5d9d0ff8e5383a79da7d5e6
>     Auto-merging mm/damon/core.c
>     [detached HEAD ecd04159c5f3] mm/damon/core: merge regions aggressively when max_nr_regions is unmet
>      Date: Mon Jun 24 10:58:14 2024 -0700
>      1 file changed, 19 insertions(+), 2 deletions(-)

Try building it:

  DESCEND objtool
  CALL    scripts/checksyscalls.sh
  CC      mm/damon/core.o
In file included from ./include/linux/kernel.h:26,
                 from ./arch/x86/include/asm/percpu.h:27,
                 from ./arch/x86/include/asm/preempt.h:6,
                 from ./include/linux/preempt.h:79,
                 from ./include/linux/spinlock.h:56,
                 from ./include/linux/swait.h:7,
                 from ./include/linux/completion.h:12,
                 from ./include/linux/damon.h:11,
                 from mm/damon/core.c:10:
mm/damon/core.c: In function ‘kdamond_merge_regions’:
./include/linux/minmax.h:20:35: error: comparison of distinct pointer types lacks a cast [-Werror]
   20 |         (!!(sizeof((typeof(x) *)1 == (typeof(y) *)1)))
      |                                   ^~
./include/linux/minmax.h:26:18: note: in expansion of macro ‘__typecheck’
   26 |                 (__typecheck(x, y) && __no_side_effects(x, y))
      |                  ^~~~~~~~~~~
./include/linux/minmax.h:36:31: note: in expansion of macro ‘__safe_cmp’
   36 |         __builtin_choose_expr(__safe_cmp(x, y), \
      |                               ^~~~~~~~~~
./include/linux/minmax.h:52:25: note: in expansion of macro ‘__careful_cmp’
   52 | #define max(x, y)       __careful_cmp(x, y, >)
      |                         ^~~~~~~~~~~~~
mm/damon/core.c:946:29: note: in expansion of macro ‘max’
  946 |                 threshold = max(1, threshold * 2);
      |                             ^~~
cc1: all warnings being treated as errors
make[3]: *** [scripts/Makefile.build:250: mm/damon/core.o] Error 1
make[2]: *** [scripts/Makefile.build:503: mm/damon] Error 2
make[1]: *** [scripts/Makefile.build:503: mm] Error 2

Re: FAILED: patch "[PATCH] mm/damon/core: merge regions aggressively when max_nr_regions" failed to apply to 6.1-stable tree

[SeongJae Park]

On Mon, 15 Jul 2024 19:12:29 +0200 Greg KH <gregkh@linuxfoundation.org> wrote:

> On Mon, Jul 15, 2024 at 09:57:17AM -0700, SeongJae Park wrote:
> > Hi Greg,
> > 
> > On Mon, 15 Jul 2024 13:34:48 +0200 <gregkh@linuxfoundation.org> wrote:
> > 
> > > 
> > > The patch below does not apply to the 6.1-stable tree.
> > > If someone wants it applied there, or to any other stable or longterm
> > > tree, then please email the backport, including the original git commit
> > > id to <stable@vger.kernel.org>.
> > > 
> > > To reproduce the conflict and resubmit, you may use the following commands:
> > > 
> > > git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-6.1.y
> > > git checkout FETCH_HEAD
> > > git cherry-pick -x 310d6c15e9104c99d5d9d0ff8e5383a79da7d5e6
> > 
> > But this doesn't reproduce the failure on my machine, like below?
> > 
> >     $ git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-6.1.y
> >     [...]
> >     $ git checkout FETCH_HEAD
> >     [...]
> >     HEAD is now at cac15753b8ce Linux 6.1.99
> >     $ git cherry-pick -x 310d6c15e9104c99d5d9d0ff8e5383a79da7d5e6
> >     Auto-merging mm/damon/core.c
> >     [detached HEAD ecd04159c5f3] mm/damon/core: merge regions aggressively when max_nr_regions is unmet
> >      Date: Mon Jun 24 10:58:14 2024 -0700
> >      1 file changed, 19 insertions(+), 2 deletions(-)
> 
> Try building it:
> 
>   DESCEND objtool
>   CALL    scripts/checksyscalls.sh
>   CC      mm/damon/core.o
> In file included from ./include/linux/kernel.h:26,
>                  from ./arch/x86/include/asm/percpu.h:27,
>                  from ./arch/x86/include/asm/preempt.h:6,
>                  from ./include/linux/preempt.h:79,
>                  from ./include/linux/spinlock.h:56,
>                  from ./include/linux/swait.h:7,
>                  from ./include/linux/completion.h:12,
>                  from ./include/linux/damon.h:11,
>                  from mm/damon/core.c:10:
> mm/damon/core.c: In function \u2018kdamond_merge_regions\u2019:
> ./include/linux/minmax.h:20:35: error: comparison of distinct pointer types lacks a cast [-Werror]
>    20 |         (!!(sizeof((typeof(x) *)1 == (typeof(y) *)1)))
>       |                                   ^~
> ./include/linux/minmax.h:26:18: note: in expansion of macro \u2018__typecheck\u2019
>    26 |                 (__typecheck(x, y) && __no_side_effects(x, y))
>       |                  ^~~~~~~~~~~
> ./include/linux/minmax.h:36:31: note: in expansion of macro \u2018__safe_cmp\u2019
>    36 |         __builtin_choose_expr(__safe_cmp(x, y), \
>       |                               ^~~~~~~~~~
> ./include/linux/minmax.h:52:25: note: in expansion of macro \u2018__careful_cmp\u2019
>    52 | #define max(x, y)       __careful_cmp(x, y, >)
>       |                         ^~~~~~~~~~~~~
> mm/damon/core.c:946:29: note: in expansion of macro \u2018max\u2019
>   946 |                 threshold = max(1, threshold * 2);
>       |                             ^~~
> cc1: all warnings being treated as errors
> make[3]: *** [scripts/Makefile.build:250: mm/damon/core.o] Error 1
> make[2]: *** [scripts/Makefile.build:503: mm/damon] Error 2
> make[1]: *** [scripts/Makefile.build:503: mm] Error 2

Thank you for sharing this.

I found the issue can be fixed in two ways.  I'd like to know what way you'd
prefer.

The first way is adding below simple fix to mm/damon/core.c file.

--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -943,7 +943,7 @@ static void kdamond_merge_regions(struct damon_ctx *c, unsigned int threshold,
                        damon_merge_regions_of(t, threshold, sz_limit);
                        nr_regions += damon_nr_regions(t);
                }
-               threshold = max(1, threshold * 2);
+               threshold = max(1U, threshold * 2);
        } while (nr_regions > c->attrs.max_nr_regions &&
                        threshold / 2 < max_thres);
 }

The second way is adding upstream commits that avoids the warning of DAMON code
on >=6.6 kernels.  Specifically, commit 867046cc7027 ("minmax: relax check to
allow comparison between unsigned arguments and signed constants") is needed.
However, the commit cannot cleanly cherry-picked on its own.  Cherry-picking
the commit together with below commits (listed latest one first) made all
commits cleanly be picked and the warning disappears.

4ead534fba42 minmax: allow comparisons of 'int' against 'unsigned char/short'
d03eba99f5bf minmax: allow min()/max()/clamp() if the arguments have the same signedness.
2122e2a4efc2 minmax: clamp more efficiently by avoiding extra comparison
5efcecd9a3b1 minmax: sanity check constant bounds when clamping

Same warning happens on 5.15.y.  In the case, adding the minmax.h upstream
commits only adds more build errors.  To remove those, yet another upstream
commit, namely commit a49a64b5bf19 ("tracing: Define the is_signed_type() macro
once") need to be cherry-picked.

IMHO, the second way is more complex but right for long term, since future
commits for stable tree may also have similar issue.  It is not a strong
opinion, and either ways work for me.

So, Greg, what would you prefer?


Thanks,
SJ

Re: FAILED: patch "[PATCH] mm/damon/core: merge regions aggressively when max_nr_regions" failed to apply to 6.1-stable tree

[Greg KH]

On Mon, Jul 15, 2024 at 12:59:45PM -0700, SeongJae Park wrote:
> On Mon, 15 Jul 2024 19:12:29 +0200 Greg KH <gregkh@linuxfoundation.org> wrote:
> 
> > On Mon, Jul 15, 2024 at 09:57:17AM -0700, SeongJae Park wrote:
> > > Hi Greg,
> > > 
> > > On Mon, 15 Jul 2024 13:34:48 +0200 <gregkh@linuxfoundation.org> wrote:
> > > 
> > > > 
> > > > The patch below does not apply to the 6.1-stable tree.
> > > > If someone wants it applied there, or to any other stable or longterm
> > > > tree, then please email the backport, including the original git commit
> > > > id to <stable@vger.kernel.org>.
> > > > 
> > > > To reproduce the conflict and resubmit, you may use the following commands:
> > > > 
> > > > git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-6.1.y
> > > > git checkout FETCH_HEAD
> > > > git cherry-pick -x 310d6c15e9104c99d5d9d0ff8e5383a79da7d5e6
> > > 
> > > But this doesn't reproduce the failure on my machine, like below?
> > > 
> > >     $ git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-6.1.y
> > >     [...]
> > >     $ git checkout FETCH_HEAD
> > >     [...]
> > >     HEAD is now at cac15753b8ce Linux 6.1.99
> > >     $ git cherry-pick -x 310d6c15e9104c99d5d9d0ff8e5383a79da7d5e6
> > >     Auto-merging mm/damon/core.c
> > >     [detached HEAD ecd04159c5f3] mm/damon/core: merge regions aggressively when max_nr_regions is unmet
> > >      Date: Mon Jun 24 10:58:14 2024 -0700
> > >      1 file changed, 19 insertions(+), 2 deletions(-)
> > 
> > Try building it:
> > 
> >   DESCEND objtool
> >   CALL    scripts/checksyscalls.sh
> >   CC      mm/damon/core.o
> > In file included from ./include/linux/kernel.h:26,
> >                  from ./arch/x86/include/asm/percpu.h:27,
> >                  from ./arch/x86/include/asm/preempt.h:6,
> >                  from ./include/linux/preempt.h:79,
> >                  from ./include/linux/spinlock.h:56,
> >                  from ./include/linux/swait.h:7,
> >                  from ./include/linux/completion.h:12,
> >                  from ./include/linux/damon.h:11,
> >                  from mm/damon/core.c:10:
> > mm/damon/core.c: In function \u2018kdamond_merge_regions\u2019:
> > ./include/linux/minmax.h:20:35: error: comparison of distinct pointer types lacks a cast [-Werror]
> >    20 |         (!!(sizeof((typeof(x) *)1 == (typeof(y) *)1)))
> >       |                                   ^~
> > ./include/linux/minmax.h:26:18: note: in expansion of macro \u2018__typecheck\u2019
> >    26 |                 (__typecheck(x, y) && __no_side_effects(x, y))
> >       |                  ^~~~~~~~~~~
> > ./include/linux/minmax.h:36:31: note: in expansion of macro \u2018__safe_cmp\u2019
> >    36 |         __builtin_choose_expr(__safe_cmp(x, y), \
> >       |                               ^~~~~~~~~~
> > ./include/linux/minmax.h:52:25: note: in expansion of macro \u2018__careful_cmp\u2019
> >    52 | #define max(x, y)       __careful_cmp(x, y, >)
> >       |                         ^~~~~~~~~~~~~
> > mm/damon/core.c:946:29: note: in expansion of macro \u2018max\u2019
> >   946 |                 threshold = max(1, threshold * 2);
> >       |                             ^~~
> > cc1: all warnings being treated as errors
> > make[3]: *** [scripts/Makefile.build:250: mm/damon/core.o] Error 1
> > make[2]: *** [scripts/Makefile.build:503: mm/damon] Error 2
> > make[1]: *** [scripts/Makefile.build:503: mm] Error 2
> 
> Thank you for sharing this.
> 
> I found the issue can be fixed in two ways.  I'd like to know what way you'd
> prefer.
> 
> The first way is adding below simple fix to mm/damon/core.c file.
> 
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -943,7 +943,7 @@ static void kdamond_merge_regions(struct damon_ctx *c, unsigned int threshold,
>                         damon_merge_regions_of(t, threshold, sz_limit);
>                         nr_regions += damon_nr_regions(t);
>                 }
> -               threshold = max(1, threshold * 2);
> +               threshold = max(1U, threshold * 2);
>         } while (nr_regions > c->attrs.max_nr_regions &&
>                         threshold / 2 < max_thres);
>  }
> 
> The second way is adding upstream commits that avoids the warning of DAMON code
> on >=6.6 kernels.  Specifically, commit 867046cc7027 ("minmax: relax check to
> allow comparison between unsigned arguments and signed constants") is needed.
> However, the commit cannot cleanly cherry-picked on its own.  Cherry-picking
> the commit together with below commits (listed latest one first) made all
> commits cleanly be picked and the warning disappears.
> 
> 4ead534fba42 minmax: allow comparisons of 'int' against 'unsigned char/short'
> d03eba99f5bf minmax: allow min()/max()/clamp() if the arguments have the same signedness.
> 2122e2a4efc2 minmax: clamp more efficiently by avoiding extra comparison
> 5efcecd9a3b1 minmax: sanity check constant bounds when clamping

I just tried the above, and then added your commit, but I still get the
same build error.  Did you try it?

I would love to get the minmax stuff properly backported to 6.1 (and
older if possible), as we have run into this same issue with many
changes over the years.

> Same warning happens on 5.15.y.  In the case, adding the minmax.h upstream
> commits only adds more build errors.  To remove those, yet another upstream
> commit, namely commit a49a64b5bf19 ("tracing: Define the is_signed_type() macro
> once") need to be cherry-picked.
> 
> IMHO, the second way is more complex but right for long term, since future
> commits for stable tree may also have similar issue.  It is not a strong
> opinion, and either ways work for me.
> 
> So, Greg, what would you prefer?

I would prefer the second way, IF it works.  In my limited testing right
now, I couldn't get it to work at all.  Can you send a series of
backported patches that work for you so that I ensure that I'm not just
doing something stupid on my end?

thanks,

greg k-h

Re: FAILED: patch "[PATCH] mm/damon/core: merge regions aggressively when max_nr_regions" failed to apply to 6.1-stable tree

[SeongJae Park]

On Tue, 16 Jul 2024 09:16:19 +0200 Greg KH <gregkh@linuxfoundation.org> wrote:

> On Mon, Jul 15, 2024 at 12:59:45PM -0700, SeongJae Park wrote:
> > On Mon, 15 Jul 2024 19:12:29 +0200 Greg KH <gregkh@linuxfoundation.org> wrote:
> > 
> > > On Mon, Jul 15, 2024 at 09:57:17AM -0700, SeongJae Park wrote:
> > > > Hi Greg,
> > > > 
> > > > On Mon, 15 Jul 2024 13:34:48 +0200 <gregkh@linuxfoundation.org> wrote:
> > > > 
> > > > > 
> > > > > The patch below does not apply to the 6.1-stable tree.
> > > > > If someone wants it applied there, or to any other stable or longterm
> > > > > tree, then please email the backport, including the original git commit
> > > > > id to <stable@vger.kernel.org>.
> > > > > 
> > > > > To reproduce the conflict and resubmit, you may use the following commands:
> > > > > 
> > > > > git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-6.1.y
> > > > > git checkout FETCH_HEAD
> > > > > git cherry-pick -x 310d6c15e9104c99d5d9d0ff8e5383a79da7d5e6
> > > > 
> > > > But this doesn't reproduce the failure on my machine, like below?
> > > > 
> > > >     $ git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-6.1.y
> > > >     [...]
> > > >     $ git checkout FETCH_HEAD
> > > >     [...]
> > > >     HEAD is now at cac15753b8ce Linux 6.1.99
> > > >     $ git cherry-pick -x 310d6c15e9104c99d5d9d0ff8e5383a79da7d5e6
> > > >     Auto-merging mm/damon/core.c
> > > >     [detached HEAD ecd04159c5f3] mm/damon/core: merge regions aggressively when max_nr_regions is unmet
> > > >      Date: Mon Jun 24 10:58:14 2024 -0700
> > > >      1 file changed, 19 insertions(+), 2 deletions(-)
> > > 
> > > Try building it:
> > > 
[...]
> > > mm/damon/core.c:946:29: note: in expansion of macro \u2018max\u2019
> > >   946 |                 threshold = max(1, threshold * 2);
> > >       |                             ^~~
> > > cc1: all warnings being treated as errors
> > > make[3]: *** [scripts/Makefile.build:250: mm/damon/core.o] Error 1
> > > make[2]: *** [scripts/Makefile.build:503: mm/damon] Error 2
> > > make[1]: *** [scripts/Makefile.build:503: mm] Error 2
> > 
> > Thank you for sharing this.
> > 
> > I found the issue can be fixed in two ways.  I'd like to know what way you'd
> > prefer.
> > 
> > The first way is adding below simple fix to mm/damon/core.c file.
> > 
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> > @@ -943,7 +943,7 @@ static void kdamond_merge_regions(struct damon_ctx *c, unsigned int threshold,
> >                         damon_merge_regions_of(t, threshold, sz_limit);
> >                         nr_regions += damon_nr_regions(t);
> >                 }
> > -               threshold = max(1, threshold * 2);
> > +               threshold = max(1U, threshold * 2);
> >         } while (nr_regions > c->attrs.max_nr_regions &&
> >                         threshold / 2 < max_thres);
> >  }
> > 
> > The second way is adding upstream commits that avoids the warning of DAMON code
> > on >=6.6 kernels.  Specifically, commit 867046cc7027 ("minmax: relax check to
> > allow comparison between unsigned arguments and signed constants") is needed.
> > However, the commit cannot cleanly cherry-picked on its own.  Cherry-picking
> > the commit together with below commits (listed latest one first) made all
> > commits cleanly be picked and the warning disappears.
> > 
> > 4ead534fba42 minmax: allow comparisons of 'int' against 'unsigned char/short'
> > d03eba99f5bf minmax: allow min()/max()/clamp() if the arguments have the same signedness.
> > 2122e2a4efc2 minmax: clamp more efficiently by avoiding extra comparison
> > 5efcecd9a3b1 minmax: sanity check constant bounds when clamping
> 
> I just tried the above, and then added your commit, but I still get the
> same build error.  Did you try it?

Yes, I tried and confirmed that on my machine.  I'm wondering if you applied
only the four patches on the above list?  To fix the warning, the commit
867046cc7027 ("minmax: relax check to allow comparison between unsigned
arguments and signed constants") should also be applied on top of those.

Nonetheless, with more testing, I found it causes yet another error on my kunit
build.  It required further picking commit f6e9d38f8eb0 ("minmax: fix header
inclusions") to fix it.

> 
> I would love to get the minmax stuff properly backported to 6.1 (and
> older if possible), as we have run into this same issue with many
> changes over the years.

Yes, I agree.

> 
> > Same warning happens on 5.15.y.  In the case, adding the minmax.h upstream
> > commits only adds more build errors.  To remove those, yet another upstream
> > commit, namely commit a49a64b5bf19 ("tracing: Define the is_signed_type() macro
> > once") need to be cherry-picked.
> > 
> > IMHO, the second way is more complex but right for long term, since future
> > commits for stable tree may also have similar issue.  It is not a strong
> > opinion, and either ways work for me.
> > 
> > So, Greg, what would you prefer?
> 
> I would prefer the second way, IF it works.  In my limited testing right
> now, I couldn't get it to work at all.  Can you send a series of
> backported patches that work for you so that I ensure that I'm not just
> doing something stupid on my end?

Thank you for the kind and clear answer.  I just posted the patches:
https://lore.kernel.org/20240716175205.51280-1-sj@kernel.org

Please let me know if it still not works.


Thanks,
SJ

[...]