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