[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <7ba90240-33ba-4e08-af38-3ec2d40c3837@rasmusvillemoes.dk>
Date: Wed, 25 Oct 2023 10:03:39 +0200
From: Rasmus Villemoes <linux@...musvillemoes.dk>
To: Linus Torvalds <torvalds@...ux-foundation.org>,
Sebastian Reichel <sebastian.reichel@...labora.com>
Cc: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
Michael Turquette <mturquette@...libre.com>,
Stephen Boyd <sboyd@...nel.org>,
David Laight <David.Laight@...lab.com>,
linux-clk@...r.kernel.org, linux-kernel@...r.kernel.org,
Vasily Gorbik <gor@...ux.ibm.com>,
Niklas Schnelle <schnelle@...ux.ibm.com>, kernel@...labora.com
Subject: Re: [PATCH v4 1/3] math.h: add DIV_ROUND_UP_NO_OVERFLOW
On 25/10/2023 00.53, Linus Torvalds wrote:
> I'm hoping that somebody will go "Linus, you're just being
> *completely* silly, it's much easier to do XYZ".
I don't have better suggestions, but a few notes:
Both the existing and new implementation are simply wrong for negative
n, because / doesn't do floor(), it does round-towards-0 (I do believe
that's a bug in the C standard, but not much one can do about that). So
both the old and new say that dru(-5, 5) == 0, dru(-7, 5) == 0, dru(-10,
5) == -1. They are correct for dru(-9, 5) == -1 and other "1 mod d" cases.
But the new implementation disagrees with the old for -d+1 < n < 0: The
old implementation correctly says dru(-3, 5) == 0, but the new gives 1 (!).
Preventing signed types from being used is probably too difficult. But
it would be nice if we could at least catch a negative _value_ and do
something sensible. Can we make the build fail for a negative,
compile-time constant, n? I have no idea what to do for run-time
negative values - we already don't do the math right, but starting to
return a positive number for some negative n's really doesn't seem right.
Aside: I don't think uapi/ stuff can include non-uapi/ stuff.
Aside2: do_div(n-1, d); probably doesn't compile :)
Rasmus
Powered by blists - more mailing lists