linux-kernel - 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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Z1CUIT8zAqWOnot-@slm.duckdns.org>
Date: Wed, 4 Dec 2024 07:40:49 -1000
From: Tejun Heo <tj@...nel.org>
To: Dan Carpenter <dan.carpenter@...aro.org>
Cc: David Laight <David.Laight@...lab.com>,
	Naresh Kamboju <naresh.kamboju@...aro.org>,
	"linux-s390@...r.kernel.org" <linux-s390@...r.kernel.org>,
	clang-built-linux <llvm@...ts.linux.dev>,
	linux-block <linux-block@...r.kernel.org>,
	open list <linux-kernel@...r.kernel.org>,
	"lkft-triage@...ts.linaro.org" <lkft-triage@...ts.linaro.org>,
	Linux Regressions <regressions@...ts.linux.dev>,
	Anders Roxell <anders.roxell@...aro.org>,
	Arnd Bergmann <arnd@...db.de>,
	Nathan Chancellor <nathan@...nel.org>, Jens Axboe <axboe@...nel.dk>
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

Hello,

On Wed, Dec 04, 2024 at 07:50:14PM +0300, Dan Carpenter wrote:
> Tejun probably reads everything to linux-block, but let's CC him explicitly.

Oh, I'm not. Thanks for cc'ing.

> 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.

This is a good catch. It's impressive that this can be caught at compile
time. The upper limit can become zero but the lower limit should win as
that's there to protect against divide by zero, so I think the right thinig
to do is replacing clamp() with max(min()). Is someone interested in writing
up the patch and sending it Jens' way?

Thanks.

-- 
tejun