On Tue, 16 Jul 2024 09:16:19 +0200 Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxx>. > > > > > > > > > > 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@xxxxxxxxxx Please let me know if it still not works. Thanks, SJ [...]
