On Tue, 27 Aug 2024, Devarsh Thakkar <devarsht@xxxxxx> 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@xxxxxx > > Regards > Devarsh -- Jani Nikula, Intel
