On Thu, 7 Jan 2016, Jim Harris wrote:

On Thu, Jan 7, 2016 at 9:27 AM, Ravi Pokala <[email protected]> wrote:
...
+ * Used for calculating number of CPUs to assign to each core and number
of I/O
+ *  queues to allocate per controller.
+ */
+#define NVME_CEILING(num, div)        ((((num) - 1) / (div)) + 1)
+

...

I'm surprised that this isn't in <sys/param.h>, along with
roundup()/rounddown()/etc. Finding the ceiling like this is probably pretty
common, so shouldn't it be added to the common header so everyone can use
it?

Good catch.  howmany() does exactly this, just expressed differently.  I'll
switch over to that.  Thanks!

howmany() doesn't do exactly this.  It has diferent bugs / range of
applicability,  I see about 10.  Subtracting 1 instead of adding more
than 1 gives overflow in different cases.  Even the simple and not
unreasonable case num = 0 gives overflow in NVME_CEILING() if div happens
to be unsigned and not 1: NVME_CEILING(0, 2U) = UINTMAX / 2 + 1 =
0x80000000 with 32-bit ints.

Getting to 10 different bugs requires considering unreasonable cases
with negative div or num, and nonsense cases with div = 0, and
reasonable but unsupported cases with floating point args.  C's broken
division for negative integer values makes the details large.
NVME_CEILING() is closer to supporting negative values than howmany()
-- the magic 1 that it adds is to adjust for division of non-negative
values rounding down.  For division of negative values, the adjustment
should be by -1 or 0, depending on the C bug and on whether you want
to round negative results towards 0 or minus infinity.

Howmany() is undocumented so its bugs are harder to see than in your own
macro.  Another one is that it is an unsafe macro with the name of a safe
function-like API.  This naming convention is more important for undocumented
APIs.  NVME_CEILING() follows it in reverse.  Subtracting 1 instead of
adding (div) - 1 is less robust but avoids multiple evaluation of div.

Bruce
_______________________________________________
[email protected] mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "[email protected]"

Reply via email to