* [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 thread
* [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
end 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