Messages in this thread Patch in this message |  | | Date | Wed, 4 Dec 2024 19:50:14 +0300 | | From | Dan Carpenter <> | | Subject | Re: s390: 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 |
| |
Tejun probably reads everything to linux-block, but let's CC him explicitly.
block/blk-iocost.c 2222 TRACE_IOCG_PATH(iocg_idle, iocg, now, 2223 atomic64_read(&iocg->active_period), 2224 atomic64_read(&ioc->cur_period), vtime); 2225 __propagate_weights(iocg, 0, 0, false, now); ^ Why is "active" zero? __propagate_weights() does a clamp() to 1 as minimum and we've added new build time asserts so this breaks the build.
2226 list_del_init(&iocg->active_list);
The other way to solve this would be to something stupid like:
diff --git a/block/blk-iocost.c b/block/blk-iocost.c index 384aa15e8260..551edd2f661f 100644 --- a/block/blk-iocost.c +++ b/block/blk-iocost.c @@ -1094,7 +1094,7 @@ static void __propagate_weights(struct ioc_gq *iocg, u32 active, u32 inuse, * @active. An active internal node's inuse is solely determined by the * inuse to active ratio of its children regardless of @inuse. */ - if (list_empty(&iocg->active_list) && iocg->child_active_sum) { + if ((list_empty(&iocg->active_list) && iocg->child_active_sum) || active == 0) { inuse = DIV64_U64_ROUND_UP(active * iocg->child_inuse_sum, iocg->child_active_sum); } else { But that seems really stupid.
regards, dan carpenter
On Wed, Dec 04, 2024 at 04:11:33PM +0000, David Laight wrote: > From: Dan Carpenter <dan.carpenter@linaro.org> > > Sent: 04 December 2024 14:39 > > Let's add David to the Cc list because he's the expert on clamp(). > > The traceback info misses the important point. > I can't see the 'inlined from line 2225' message. > > We have (line 1084): > static void __propagate_weights(struct ioc_gq *iocg, u32 active, u32 inuse, > bool save, struct ioc_now *now) > followed by: > inuse = clamp_t(u32, inuse, 1, active); > > But line 2225 has: > __propagate_weights(iocg, 0, 0, false, now); > > With aggressive inlining the compiler sees 'active == 0' > and the lo > hi test correctly triggers. > > The previous version only verified 'lo <= hi' if it was a constant > integer expression - which it isn't here. > > No idea what the code is trying to do, nor what value it expects > clamp(val, 1, 0) to generate - likely to be 0 or 1 depending on > the order of the comparisons. > > David > > - > Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK > Registration No: 1397386 (Wales)
|  |