Re: [PATCH 1/2] Create a DIV_ROUND_CLOSEST macro to do division with rounding

From: Jean Delvare
Date: Mon Aug 03 2009 - 08:21:47 EST


Hi Peter,

On Mon, 03 Aug 2009 13:57:52 +0200, Peter Zijlstra wrote:
> On Mon, 2008-11-10 at 17:01 -0800, Darrick J. Wong wrote:
> > Create a helper macro to divide two numbers and round the result to the
> > nearest whole number. This is a helper macro for hwmon drivers that
> > want to convert incoming sysfs values per standard hwmon practice, though
> > the macro itself can be used by anyone.
>
> Its too late to rename this now isn't it :-/

We can always rename things, but it is costly, so we only do that if we
have a good reason (e.g. the original name caused confusion.)

>
> DIV_ROUND_{UP,CEIL}
> DIV_ROUND_{DOWN,FLOOR}

The latter is the simple integer division, so there is no point in
defining a macro for it. The former we already have.

>
> I get.
>
> The proposed thing is simply DIV_ROUND, but this DIV_ROUND_CLOSEST name
> is just wonky.

I can't disagree. I even think I argued about it back then, but then
finally gave up. You should have participated in the debate when it was
hot rather than 8 months later :(

>
> Also, shouldn't it be something like ?
>
> DIV_ROUND(x, y) (((x) + (((y)+1)/2)) / (y))

No, that wouldn't work. Simple example, 10/3 is 3.33. (10+3/2)/3 is 3
which is the correct rounded value. (10+(3+1)/2)/3 is 4 which is not
the correct rounded value.

> Also, do we want the default rounding to be symmetric or asymmetric?

I don't think we care at all. This macro will rarely be called on
negative numbers, and even then...

>
> I think round to half even or something would be nice, but would add an
> extra conditional, not sure its worth it (round away from zero only
> works for signed numbers and it would also cost one conditional).

Definitely not worth the extra cost. We do the rounding because it is
not too expensive and adds value. Fine-tuning it will cost more than is
worth, plus would lead to endless debates about what is the best
rounding strategy.

If anyone really needs additional control on rounding, they better do
not rely on a general helper, but instead implement their own rounding
function.

--
Jean Delvare
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/