Re: [sj:damon/next 18/20] mm/damon/core.c:752:44: warning: variable 'goal' used in loop condition not modified in loop body

[SeongJae Park <sj@kernel.org>]

On Mon, 26 Feb 2024 08:33:16 +0800 kernel test robot <lkp@intel.com> wrote:

> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/sj/linux.git damon/next
> head:   a9e8680aa284cf5fd994087542cfa9e6f5f491b4
> commit: 74d79c7a174ab26a8f99bc482dd6b067c93bc554 [18/20] mm/damon: implement DAMON context input-only update function
> config: i386-buildonly-randconfig-003-20240226 (https://download.01.org/0day-ci/archive/20240226/202402260800.F3OIgZl9-lkp@intel.com/config)
> compiler: clang version 17.0.6 (https://github.com/llvm/llvm-project 6009708b4367171ccdbf4b5905cb6a803753fe18)
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240226/202402260800.F3OIgZl9-lkp@intel.com/reproduce)
> 
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202402260800.F3OIgZl9-lkp@intel.com/
> 
> All warnings (new ones prefixed by >>):
> 
> >> mm/damon/core.c:752:44: warning: variable 'goal' used in loop condition not modified in loop body [-Wfor-loop-analysis]
>      752 |         for (goal = damos_nth_quota_goal(i, src); goal; )
>          |                                                   ^~~~
> >> mm/damon/core.c:799:42: warning: variable 'filter' used in loop condition not modified in loop body [-Wfor-loop-analysis]
>      799 |         for (filter = damos_nth_filter(i, src); filter;)
>          |                                                 ^~~~~~
> >> mm/damon/core.c:853:42: warning: variable 'scheme' used in loop condition not modified in loop body [-Wfor-loop-analysis]
>      853 |         for (scheme = damon_nth_scheme(i, src); scheme;)
>          |                                                 ^~~~~~
> >> mm/damon/core.c:841:13: warning: variable 'err' set but not used [-Wunused-but-set-variable]
>      841 |         int i = 0, err;
>          |                    ^
> >> mm/damon/core.c:929:37: warning: variable 't' used in loop condition not modified in loop body [-Wfor-loop-analysis]
>      929 |         for (t = damon_nth_target(i, src); t;)
>          |                                            ^
>    5 warnings generated.

Thank you for this nice report!

I fixed the warnings with below changes, and squashed the diff on the
problem-caused commit.  I confirmed the warning is disappeared after the change
using the awesome reproducer that you provided :)

--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -734,7 +734,7 @@ static void damos_update_quota_goals(struct damos_quota *dst,
                struct damos_quota *src)
 {
        struct damos_quota_goal *goal, *next;
-       int i = 0;
+       int i = 0, j = 0;

        damos_for_each_quota_goal_safe(goal, next, dst) {
                struct damos_quota_goal *src_goal =
@@ -749,8 +749,11 @@ static void damos_update_quota_goals(struct damos_quota *dst,
                }
                damos_destroy_quota_goal(goal);
        }
-       for (goal = damos_nth_quota_goal(i, src); goal; )
+       damos_for_each_quota_goal_safe(goal, next, src) {
+               if (j++ < i)
+                       continue;
                damos_move_quota_goal(dst, goal);
+       }
 }

 static struct damos_filter *damos_nth_filter(int n, struct damos *s)
@@ -768,7 +771,7 @@ static struct damos_filter *damos_nth_filter(int n, struct damos *s)
 static int damos_update_filters(struct damos *dst, struct damos *src)
 {
        struct damos_filter *filter, *next;
-       int i = 0;
+       int i = 0, j = 0;

        damos_for_each_filter_safe(filter, next, dst) {
                struct damos_filter *src_filter = damos_nth_filter(i++, src);
@@ -783,8 +786,11 @@ static int damos_update_filters(struct damos *dst, struct damos *src)
                damos_destroy_filter(filter);
        }

-       for (filter = damos_nth_filter(i, src); filter;)
+       damos_for_each_filter_safe(filter, next, src) {
+               if (j++ < i)
+                       continue;
                damos_move_filter(dst, filter);
+       }
        return 0;
 }

@@ -825,20 +831,25 @@ static int damon_update_scheme(struct damos *dst, struct damos *src)
 static int damon_update_schemes(struct damon_ctx *dst, struct damon_ctx *src)
 {
        struct damos *scheme, *next;
-       int i = 0, err;
+       int i = 0, j = 0, err;

        damon_for_each_scheme_safe(scheme, next, dst) {
                struct damos *src_scheme = damon_nth_scheme(i++, src);

                if (src_scheme) {
                        err = damon_update_scheme(scheme, src_scheme);
+                       if (err)
+                               return err;
                        continue;
                }
                damon_destroy_scheme(scheme);
        }

-       for (scheme = damon_nth_scheme(i, src); scheme;)
+       damon_for_each_scheme_safe(scheme, next, src) {
+               if (j++ < i)
+                       continue;
                damon_move_scheme(dst, scheme);
+       }
        return 0;
 }

@@ -892,7 +903,7 @@ static struct damon_target *damon_nth_target(int n, struct damon_ctx *ctx)
 static int damon_update_targets(struct damon_ctx *dst, struct damon_ctx *src)
 {
        struct damon_target *t, *next;
-       int i = 0, err;
+       int i = 0, j = 0, err;

        damon_for_each_target_safe(t, next, dst) {
                struct damon_target *src_target = damon_nth_target(i++, src);
@@ -913,8 +924,11 @@ static int damon_update_targets(struct damon_ctx *dst, struct damon_ctx *src)
                damon_destroy_target(t);
        }

-       for (t = damon_nth_target(i, src); t;)
+       damon_for_each_target_safe(t, next, src) {
+               if (j++ < i)
+                       continue;
                damon_move_target(dst, t);
+       }
        return 0;
 }

[...]