After a recent change to clamp() and its variants [1] that increases the
coverage of the check that high is greater than low because it can be
done through inlining, certain build configurations (such as s390
defconfig) fail to build with clang with:
block/blk-iocost.c:1101:11: error: call to '__compiletime_assert_557' declared with 'error' attribute: clamp() low limit 1 greater than high limit active
1101 | inuse = clamp_t(u32, inuse, 1, active);
| ^
include/linux/minmax.h:218:36: note: expanded from macro 'clamp_t'
218 | #define clamp_t(type, val, lo, hi) __careful_clamp(type, val, lo, hi)
| ^
include/linux/minmax.h:195:2: note: expanded from macro '__careful_clamp'
195 | __clamp_once(type, val, lo, hi, __UNIQUE_ID(v_), __UNIQUE_ID(l_), __UNIQUE_ID(h_))
| ^
include/linux/minmax.h:188:2: note: expanded from macro '__clamp_once'
188 | BUILD_BUG_ON_MSG(statically_true(ulo > uhi), \
| ^
__propagate_weights() is called with an active value of zero in
ioc_check_iocgs(), which results in the high value being less than the
low value, which is undefined because the value returned depends on the
order of the comparisons.
The purpose of this expression is to ensure inuse is not more than
active and at least 1. This could be written more simply with a ternary
expression that uses min(inuse, active) as the condition so that the
value of that condition can be used if it is not zero and one if it is.
Do this conversion to resolve the error and add a comment to deter
people from turning this back into clamp().
Link: https://lore.kernel.org/r/34d53778977747f19cce2abb287bb3e6@xxxxxxxxxxxxxxxx/ [1]
Suggested-by: David Laight <david.laight@xxxxxxxxxx>
Reported-by: Linux Kernel Functional Testing <lkft@xxxxxxxxxx>
Closes: https://lore.kernel.org/llvm/CA+G9fYsD7mw13wredcZn0L-KBA3yeoVSTuxnss-AEWMN3ha0cA@xxxxxxxxxxxxxx/
Reported-by: kernel test robot <lkp@xxxxxxxxx>
Closes: https://lore.kernel.org/oe-kbuild-all/202412120322.3GfVe3vF-lkp@xxxxxxxxx/
Signed-off-by: Nathan Chancellor <nathan@xxxxxxxxxx>
---
block/blk-iocost.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/block/blk-iocost.c b/block/blk-iocost.c
index 384aa15e8260bd4d83b00f1c03efb87829950327..a5894ec9696e7e8c1011cbda1d849562e1732d31 100644
--- a/block/blk-iocost.c
+++ b/block/blk-iocost.c
@@ -1098,7 +1098,14 @@ static void __propagate_weights(struct ioc_gq *iocg, u32 active, u32 inuse,
inuse = DIV64_U64_ROUND_UP(active * iocg->child_inuse_sum,
iocg->child_active_sum);
} else {
- inuse = clamp_t(u32, inuse, 1, active);
+ /*
+ * It may be tempting to turn this into a clamp expression with
+ * a lower limit of 1 but active may be 0, which cannot be used
+ * as an upper limit in that situation. This expression allows
+ * active to clamp inuse unless it is 0, in which case inuse
+ * becomes 1.
+ */
+ inuse = min(inuse, active) ?: 1;
}
iocg->last_inuse = iocg->inuse;
---
base-commit: 3e42dc9229c5950e84b1ed705f94ed75ed208228
change-id: 20241212-blk-iocost-fix-clamp-error-5816c028b245
Best regards,
--
Nathan Chancellor <nathan@xxxxxxxxxx>
