linux-kernel - Re: [PATCH v2 1/6] math.h: Add macros for rounding to the closest value

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 for Android: free password hash cracker in your pocket

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

Message-ID: <87frqqyw9r.fsf@intel.com>
Date: Tue, 27 Aug 2024 15:40:00 +0300
From: Jani Nikula <jani.nikula@...el.com>
To: Devarsh Thakkar <devarsht@...com>, Andy Shevchenko
 <andriy.shevchenko@...ux.intel.com>
Cc: mchehab@...nel.org, hverkuil-cisco@...all.nl,
 linux-media@...r.kernel.org, linux-kernel@...r.kernel.org,
 sebastian.fricke@...labora.com, linux-doc@...r.kernel.org,
 praneeth@...com, nm@...com, vigneshr@...com, s-jain1@...com,
 r-donadkar@...com, b-brnich@...com, detheridge@...com, p-mantena@...com,
 vijayp@...com, andi.shyti@...ux.intel.com, nicolas@...fresne.ca,
 jirislaby@...nel.org, davidgow@...gle.com, dlatypov@...gle.com,
 corbet@....net, broonie@...nel.org, rdunlap@...radead.org,
 nik.borisov@...e.com, Dave.Martin@....com
Subject: Re: [PATCH v2 1/6] math.h: Add macros for rounding to the closest
 value

On Tue, 27 Aug 2024, Devarsh Thakkar <devarsht@...com> wrote:
> Hi Andy,
>
> Thanks for the review.
>
> On 26/08/24 23:14, Andy Shevchenko wrote:
>> On Mon, Aug 26, 2024 at 08:38:17PM +0530, Devarsh Thakkar wrote:
>>> Add below rounding related macros:
>>>
>>> round_closest_up(x, y) : Rounds x to the closest multiple of y where y is a
>>> power of 2, with a preference to round up in case two nearest values are
>>> possible.
>>>
>>> round_closest_down(x, y) : Rounds x to the closest multiple of y where y is
>>> a power of 2, with a preference to round down in case two nearest values
>>> are possible.
>>>
>>> roundclosest(x, y) : Rounds x to the closest multiple of y, this macro
>>> should generally be used only when y is not multiple of 2 as otherwise
>>> round_closest* macros should be used which are much faster.
>> 
>> I understand the point, but if you need to send a v3, please explain
>> the equivalency between roundclosest() and one (or both?) of the
>> round_closest_*() in case the argument is power-of-2.
>> 
>
> The equivalency between roundclosest w.r.t round_closest is same as
> equivalency between existing macros rounddown w.r.t round_down. Functionally
> both are same but the former is recommended to be used only for the scenario
> where multiple is not power of 2 and latter is faster but is strictly for the
> scenario where multiple is power of 2. I think the same is already summarized
> well in commit message and further elaborated in the patch itself as part of
> header file comments [1] so I personally don't think any update is required
> w.r.t this.

I still don't think rounddown vs. round_down naming is a good example to
model anything after.

I have yet to hear a single compelling argument in favor of having a
single underscore in the middle of a name having implications about the
inputs of a macro/function.

The macros being added here are at 2 or 3 in Rusty's scale [1]. We could
aim for 6 (The name tells you how to use it), but also do opportunistic
8 (The compiler will warn if you get it wrong) for compile-time
constants.

As-is, these, and round_up/round_down, are just setting a trap for an
unsuspecting kernel developer to fall into.


BR,
Jani.


[1] https://ozlabs.org/~rusty/index.cgi/tech/2008-03-30.html


>
> [1]: https://lore.kernel.org/all/20240826150822.4057164-2-devarsht@ti.com
>
> Regards
> Devarsh

-- 
Jani Nikula, Intel