* [bug report] crush: remove forcefeed functionality [not found] <caa37f28-a2e8-4e0a-a9ce-a365ce805e4b@stanley.mountain> @ 2026-02-06 13:39 ` Dan Carpenter 2026-02-06 20:44 ` Viacheslav Dubeyko 0 siblings, 1 reply; 3+ messages in thread From: Dan Carpenter @ 2026-02-06 13:39 UTC (permalink / raw) To: Ilya Dryomov, Alex Markuze; +Cc: Sage Weil, ceph-devel, linux-kernel [ Smatch checking is paused while we raise funding. #SadFace https://lore.kernel.org/all/aTaiGSbWZ9DJaGo7@stanley.mountain/ -dan ] Hello Ceph Maintainers, Commit 41ebcc0907c5 ("crush: remove forcefeed functionality") from May 7, 2012 (linux-next), leads to the following Smatch static checker warning: net/ceph/crush/mapper.c:1015 crush_do_rule() warn: iterator 'j' not incremented net/ceph/crush/mapper.c 897 int crush_do_rule(const struct crush_map *map, 898 int ruleno, int x, int *result, int result_max, 899 const __u32 *weight, int weight_max, 900 void *cwin, const struct crush_choose_arg *choose_args) 901 { 902 int result_len; 903 struct crush_work *cw = cwin; 904 int *a = cwin + map->working_size; 905 int *b = a + result_max; 906 int *c = b + result_max; 907 int *w = a; 908 int *o = b; 909 int recurse_to_leaf; 910 int wsize = 0; 911 int osize; 912 const struct crush_rule *rule; 913 __u32 step; 914 int i, j; 915 int numrep; 916 int out_size; 917 /* 918 * the original choose_total_tries value was off by one (it 919 * counted "retries" and not "tries"). add one. 920 */ 921 int choose_tries = map->choose_total_tries + 1; 922 int choose_leaf_tries = 0; 923 /* 924 * the local tries values were counted as "retries", though, 925 * and need no adjustment 926 */ 927 int choose_local_retries = map->choose_local_tries; 928 int choose_local_fallback_retries = map->choose_local_fallback_tries; 929 930 int vary_r = map->chooseleaf_vary_r; 931 int stable = map->chooseleaf_stable; 932 933 if ((__u32)ruleno >= map->max_rules) { 934 dprintk(" bad ruleno %d\n", ruleno); 935 return 0; 936 } 937 938 rule = map->rules[ruleno]; 939 result_len = 0; 940 941 for (step = 0; step < rule->len; step++) { 942 int firstn = 0; 943 const struct crush_rule_step *curstep = &rule->steps[step]; 944 945 switch (curstep->op) { 946 case CRUSH_RULE_TAKE: 947 if ((curstep->arg1 >= 0 && 948 curstep->arg1 < map->max_devices) || 949 (-1-curstep->arg1 >= 0 && 950 -1-curstep->arg1 < map->max_buckets && 951 map->buckets[-1-curstep->arg1])) { 952 w[0] = curstep->arg1; 953 wsize = 1; 954 } else { 955 dprintk(" bad take value %d\n", curstep->arg1); 956 } 957 break; 958 959 case CRUSH_RULE_SET_CHOOSE_TRIES: 960 if (curstep->arg1 > 0) 961 choose_tries = curstep->arg1; 962 break; 963 964 case CRUSH_RULE_SET_CHOOSELEAF_TRIES: 965 if (curstep->arg1 > 0) 966 choose_leaf_tries = curstep->arg1; 967 break; 968 969 case CRUSH_RULE_SET_CHOOSE_LOCAL_TRIES: 970 if (curstep->arg1 >= 0) 971 choose_local_retries = curstep->arg1; 972 break; 973 974 case CRUSH_RULE_SET_CHOOSE_LOCAL_FALLBACK_TRIES: 975 if (curstep->arg1 >= 0) 976 choose_local_fallback_retries = curstep->arg1; 977 break; 978 979 case CRUSH_RULE_SET_CHOOSELEAF_VARY_R: 980 if (curstep->arg1 >= 0) 981 vary_r = curstep->arg1; 982 break; 983 984 case CRUSH_RULE_SET_CHOOSELEAF_STABLE: 985 if (curstep->arg1 >= 0) 986 stable = curstep->arg1; 987 break; 988 989 case CRUSH_RULE_CHOOSELEAF_FIRSTN: 990 case CRUSH_RULE_CHOOSE_FIRSTN: 991 firstn = 1; 992 fallthrough; 993 case CRUSH_RULE_CHOOSELEAF_INDEP: 994 case CRUSH_RULE_CHOOSE_INDEP: 995 if (wsize == 0) 996 break; 997 998 recurse_to_leaf = 999 curstep->op == 1000 CRUSH_RULE_CHOOSELEAF_FIRSTN || 1001 curstep->op == 1002 CRUSH_RULE_CHOOSELEAF_INDEP; 1003 1004 /* reset output */ 1005 osize = 0; 1006 1007 for (i = 0; i < wsize; i++) { 1008 int bno; 1009 numrep = curstep->arg1; 1010 if (numrep <= 0) { 1011 numrep += result_max; 1012 if (numrep <= 0) 1013 continue; 1014 } --> 1015 j = 0; ^^^^^ 1016 /* make sure bucket id is valid */ 1017 bno = -1 - w[i]; 1018 if (bno < 0 || bno >= map->max_buckets) { 1019 /* w[i] is probably CRUSH_ITEM_NONE */ 1020 dprintk(" bad w[i] %d\n", w[i]); 1021 continue; 1022 } 1023 if (firstn) { 1024 int recurse_tries; 1025 if (choose_leaf_tries) 1026 recurse_tries = 1027 choose_leaf_tries; 1028 else if (map->chooseleaf_descend_once) 1029 recurse_tries = 1; 1030 else 1031 recurse_tries = choose_tries; 1032 osize += crush_choose_firstn( 1033 map, 1034 cw, 1035 map->buckets[bno], 1036 weight, weight_max, 1037 x, numrep, 1038 curstep->arg2, 1039 o+osize, j, 1040 result_max-osize, 1041 choose_tries, 1042 recurse_tries, 1043 choose_local_retries, 1044 choose_local_fallback_retries, 1045 recurse_to_leaf, 1046 vary_r, 1047 stable, 1048 c+osize, 1049 0, 1050 choose_args); 1051 } else { 1052 out_size = ((numrep < (result_max-osize)) ? 1053 numrep : (result_max-osize)); 1054 crush_choose_indep( 1055 map, 1056 cw, 1057 map->buckets[bno], 1058 weight, weight_max, 1059 x, out_size, numrep, 1060 curstep->arg2, 1061 o+osize, j, 1062 choose_tries, 1063 choose_leaf_tries ? 1064 choose_leaf_tries : 1, 1065 recurse_to_leaf, 1066 c+osize, 1067 0, 1068 choose_args); 1069 osize += out_size; 1070 } There used to be a j++ around here but it was deleted. 1071 } 1072 1073 if (recurse_to_leaf) 1074 /* copy final _leaf_ values to output set */ 1075 memcpy(o, c, osize*sizeof(*o)); 1076 1077 /* swap o and w arrays */ 1078 swap(o, w); 1079 wsize = osize; 1080 break; 1081 1082 1083 case CRUSH_RULE_EMIT: 1084 for (i = 0; i < wsize && result_len < result_max; i++) { 1085 result[result_len] = w[i]; 1086 result_len++; 1087 } 1088 wsize = 0; 1089 break; 1090 1091 default: 1092 dprintk(" unknown op %d at step %d\n", 1093 curstep->op, step); 1094 break; 1095 } 1096 } 1097 1098 return result_len; 1099 } regards, dan carpenter ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [bug report] crush: remove forcefeed functionality 2026-02-06 13:39 ` [bug report] crush: remove forcefeed functionality Dan Carpenter @ 2026-02-06 20:44 ` Viacheslav Dubeyko 0 siblings, 0 replies; 3+ messages in thread From: Viacheslav Dubeyko @ 2026-02-06 20:44 UTC (permalink / raw) To: idryomov@gmail.com, Alex Markuze, dan.carpenter@linaro.org Cc: ceph-devel@vger.kernel.org, sage@inktank.com, linux-kernel@vger.kernel.org On Fri, 2026-02-06 at 16:39 +0300, Dan Carpenter wrote: > [ Smatch checking is paused while we raise funding. #SadFace > https://urldefense.proofpoint.com/v2/url?u=https-3A__lore.kernel.org_all_aTaiGSbWZ9DJaGo7-40stanley.mountain_&d=DwIBAg&c=BSDicqBQBDjDI9RkVyTcHQ&r=q5bIm4AXMzc8NJu1_RGmnQ2fMWKq4Y4RAkElvUgSs00&m=EbbQA8mLawUrIpoBP1JgkEbj9ykB2zMAgU-BpxccK9crlqQp8eHphKm2eDfswppo&s=4dnJgIrt1z5jJRZwXTmcMBeS0RZ5lg-CZ04H1P9fcrE&e= -dan ] > > Hello Ceph Maintainers, > > Commit 41ebcc0907c5 ("crush: remove forcefeed functionality") from > May 7, 2012 (linux-next), leads to the following Smatch static > checker warning: > > net/ceph/crush/mapper.c:1015 crush_do_rule() > warn: iterator 'j' not incremented Yeah, it looks like an issue. > > net/ceph/crush/mapper.c > 897 int crush_do_rule(const struct crush_map *map, > 898 int ruleno, int x, int *result, int result_max, > 899 const __u32 *weight, int weight_max, > 900 void *cwin, const struct crush_choose_arg *choose_args) > 901 { > 902 int result_len; > 903 struct crush_work *cw = cwin; > 904 int *a = cwin + map->working_size; > 905 int *b = a + result_max; > 906 int *c = b + result_max; > 907 int *w = a; > 908 int *o = b; > 909 int recurse_to_leaf; > 910 int wsize = 0; > 911 int osize; > 912 const struct crush_rule *rule; > 913 __u32 step; > 914 int i, j; > 915 int numrep; > 916 int out_size; > 917 /* > 918 * the original choose_total_tries value was off by one (it > 919 * counted "retries" and not "tries"). add one. > 920 */ > 921 int choose_tries = map->choose_total_tries + 1; > 922 int choose_leaf_tries = 0; > 923 /* > 924 * the local tries values were counted as "retries", though, > 925 * and need no adjustment > 926 */ > 927 int choose_local_retries = map->choose_local_tries; > 928 int choose_local_fallback_retries = map->choose_local_fallback_tries; > 929 > 930 int vary_r = map->chooseleaf_vary_r; > 931 int stable = map->chooseleaf_stable; > 932 > 933 if ((__u32)ruleno >= map->max_rules) { > 934 dprintk(" bad ruleno %d\n", ruleno); > 935 return 0; > 936 } > 937 > 938 rule = map->rules[ruleno]; > 939 result_len = 0; > 940 > 941 for (step = 0; step < rule->len; step++) { > 942 int firstn = 0; > 943 const struct crush_rule_step *curstep = &rule->steps[step]; > 944 > 945 switch (curstep->op) { > 946 case CRUSH_RULE_TAKE: > 947 if ((curstep->arg1 >= 0 && > 948 curstep->arg1 < map->max_devices) || > 949 (-1-curstep->arg1 >= 0 && > 950 -1-curstep->arg1 < map->max_buckets && > 951 map->buckets[-1-curstep->arg1])) { > 952 w[0] = curstep->arg1; > 953 wsize = 1; > 954 } else { > 955 dprintk(" bad take value %d\n", curstep->arg1); > 956 } > 957 break; > 958 > 959 case CRUSH_RULE_SET_CHOOSE_TRIES: > 960 if (curstep->arg1 > 0) > 961 choose_tries = curstep->arg1; > 962 break; > 963 > 964 case CRUSH_RULE_SET_CHOOSELEAF_TRIES: > 965 if (curstep->arg1 > 0) > 966 choose_leaf_tries = curstep->arg1; > 967 break; > 968 > 969 case CRUSH_RULE_SET_CHOOSE_LOCAL_TRIES: > 970 if (curstep->arg1 >= 0) > 971 choose_local_retries = curstep->arg1; > 972 break; > 973 > 974 case CRUSH_RULE_SET_CHOOSE_LOCAL_FALLBACK_TRIES: > 975 if (curstep->arg1 >= 0) > 976 choose_local_fallback_retries = curstep->arg1; > 977 break; > 978 > 979 case CRUSH_RULE_SET_CHOOSELEAF_VARY_R: > 980 if (curstep->arg1 >= 0) > 981 vary_r = curstep->arg1; > 982 break; > 983 > 984 case CRUSH_RULE_SET_CHOOSELEAF_STABLE: > 985 if (curstep->arg1 >= 0) > 986 stable = curstep->arg1; > 987 break; > 988 > 989 case CRUSH_RULE_CHOOSELEAF_FIRSTN: > 990 case CRUSH_RULE_CHOOSE_FIRSTN: > 991 firstn = 1; > 992 fallthrough; > 993 case CRUSH_RULE_CHOOSELEAF_INDEP: > 994 case CRUSH_RULE_CHOOSE_INDEP: > 995 if (wsize == 0) > 996 break; > 997 > 998 recurse_to_leaf = > 999 curstep->op == > 1000 CRUSH_RULE_CHOOSELEAF_FIRSTN || > 1001 curstep->op == > 1002 CRUSH_RULE_CHOOSELEAF_INDEP; > 1003 > 1004 /* reset output */ > 1005 osize = 0; > 1006 > 1007 for (i = 0; i < wsize; i++) { > 1008 int bno; > 1009 numrep = curstep->arg1; > 1010 if (numrep <= 0) { > 1011 numrep += result_max; > 1012 if (numrep <= 0) > 1013 continue; > 1014 } > --> 1015 j = 0; > ^^^^^ It looks like intentional initialization of variable. But let me spend some time to better understand the crush_choose_firstn() and crush_choose_indep() logic and the history of this function modifications in commits. Thanks, Slava. > > 1016 /* make sure bucket id is valid */ > 1017 bno = -1 - w[i]; > 1018 if (bno < 0 || bno >= map->max_buckets) { > 1019 /* w[i] is probably CRUSH_ITEM_NONE */ > 1020 dprintk(" bad w[i] %d\n", w[i]); > 1021 continue; > 1022 } > 1023 if (firstn) { > 1024 int recurse_tries; > 1025 if (choose_leaf_tries) > 1026 recurse_tries = > 1027 choose_leaf_tries; > 1028 else if (map->chooseleaf_descend_once) > 1029 recurse_tries = 1; > 1030 else > 1031 recurse_tries = choose_tries; > 1032 osize += crush_choose_firstn( > 1033 map, > 1034 cw, > 1035 map->buckets[bno], > 1036 weight, weight_max, > 1037 x, numrep, > 1038 curstep->arg2, > 1039 o+osize, j, > 1040 result_max-osize, > 1041 choose_tries, > 1042 recurse_tries, > 1043 choose_local_retries, > 1044 choose_local_fallback_retries, > 1045 recurse_to_leaf, > 1046 vary_r, > 1047 stable, > 1048 c+osize, > 1049 0, > 1050 choose_args); > 1051 } else { > 1052 out_size = ((numrep < (result_max-osize)) ? > 1053 numrep : (result_max-osize)); > 1054 crush_choose_indep( > 1055 map, > 1056 cw, > 1057 map->buckets[bno], > 1058 weight, weight_max, > 1059 x, out_size, numrep, > 1060 curstep->arg2, > 1061 o+osize, j, > 1062 choose_tries, > 1063 choose_leaf_tries ? > 1064 choose_leaf_tries : 1, > 1065 recurse_to_leaf, > 1066 c+osize, > 1067 0, > 1068 choose_args); > 1069 osize += out_size; > 1070 } > > There used to be a j++ around here but it was deleted. > > 1071 } > 1072 > 1073 if (recurse_to_leaf) > 1074 /* copy final _leaf_ values to output set */ > 1075 memcpy(o, c, osize*sizeof(*o)); > 1076 > 1077 /* swap o and w arrays */ > 1078 swap(o, w); > 1079 wsize = osize; > 1080 break; > 1081 > 1082 > 1083 case CRUSH_RULE_EMIT: > 1084 for (i = 0; i < wsize && result_len < result_max; i++) { > 1085 result[result_len] = w[i]; > 1086 result_len++; > 1087 } > 1088 wsize = 0; > 1089 break; > 1090 > 1091 default: > 1092 dprintk(" unknown op %d at step %d\n", > 1093 curstep->op, step); > 1094 break; > 1095 } > 1096 } > 1097 > 1098 return result_len; > 1099 } > > regards, > dan carpenter ^ permalink raw reply [flat|nested] 3+ messages in thread
* [bug report] crush: remove forcefeed functionality
@ 2024-05-04 11:23 Dan Carpenter
0 siblings, 0 replies; 3+ messages in thread
From: Dan Carpenter @ 2024-05-04 11:23 UTC (permalink / raw)
To: sage; +Cc: ceph-devel
Hello Sage Weil,
Commit 41ebcc0907c5 ("crush: remove forcefeed functionality") from
May 7, 2012 (linux-next), leads to the following Smatch static
checker warning:
net/ceph/crush/mapper.c:1012 crush_do_rule()
warn: iterator 'j' not incremented
net/ceph/crush/mapper.c
1004 for (i = 0; i < wsize; i++) {
1005 int bno;
1006 numrep = curstep->arg1;
1007 if (numrep <= 0) {
1008 numrep += result_max;
1009 if (numrep <= 0)
1010 continue;
1011 }
--> 1012 j = 0;
^^^^^^
The 2012 commit removed the j++ so now j is always 0.
1013 /* make sure bucket id is valid */
1014 bno = -1 - w[i];
1015 if (bno < 0 || bno >= map->max_buckets) {
1016 /* w[i] is probably CRUSH_ITEM_NONE */
1017 dprintk(" bad w[i] %d\n", w[i]);
1018 continue;
1019 }
1020 if (firstn) {
1021 int recurse_tries;
1022 if (choose_leaf_tries)
1023 recurse_tries =
1024 choose_leaf_tries;
1025 else if (map->chooseleaf_descend_once)
1026 recurse_tries = 1;
1027 else
1028 recurse_tries = choose_tries;
1029 osize += crush_choose_firstn(
1030 map,
1031 cw,
1032 map->buckets[bno],
1033 weight, weight_max,
1034 x, numrep,
1035 curstep->arg2,
1036 o+osize, j,
1037 result_max-osize,
1038 choose_tries,
1039 recurse_tries,
1040 choose_local_retries,
1041 choose_local_fallback_retries,
1042 recurse_to_leaf,
1043 vary_r,
1044 stable,
1045 c+osize,
1046 0,
1047 choose_args);
1048 } else {
1049 out_size = ((numrep < (result_max-osize)) ?
1050 numrep : (result_max-osize));
1051 crush_choose_indep(
1052 map,
1053 cw,
1054 map->buckets[bno],
1055 weight, weight_max,
1056 x, out_size, numrep,
1057 curstep->arg2,
1058 o+osize, j,
1059 choose_tries,
1060 choose_leaf_tries ?
1061 choose_leaf_tries : 1,
1062 recurse_to_leaf,
1063 c+osize,
1064 0,
1065 choose_args);
1066 osize += out_size;
1067 }
1068 }
1069
1070 if (recurse_to_leaf)
1071 /* copy final _leaf_ values to output set */
1072 memcpy(o, c, osize*sizeof(*o));
1073
1074 /* swap o and w arrays */
1075 swap(o, w);
1076 wsize = osize;
1077 break;
1078
1079
1080 case CRUSH_RULE_EMIT:
1081 for (i = 0; i < wsize && result_len < result_max; i++) {
1082 result[result_len] = w[i];
1083 result_len++;
1084 }
1085 wsize = 0;
1086 break;
1087
1088 default:
1089 dprintk(" unknown op %d at step %d\n",
1090 curstep->op, step);
1091 break;
1092 }
1093 }
1094
1095 return result_len;
1096 }
regards,
dan carpenter
^ permalink raw reply [flat|nested] 3+ messages in threadend of thread, other threads:[~2026-02-06 20:44 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <caa37f28-a2e8-4e0a-a9ce-a365ce805e4b@stanley.mountain>
2026-02-06 13:39 ` [bug report] crush: remove forcefeed functionality Dan Carpenter
2026-02-06 20:44 ` Viacheslav Dubeyko
2024-05-04 11:23 Dan Carpenter
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox