[PATCH v5 1/4] uapi: Provide DIV_ROUND_CLOSEST()

[Cristian Ciocaltea]

Hi Andy,

On 1/27/26 4:38 PM, Andy Shevchenko wrote:
> On Tue, Jan 27, 2026 at 03:58:13PM +0200, Jani Nikula wrote:
>> On Tue, 27 Jan 2026, Cristian Ciocaltea <cristian.ciocaltea at collabora.com> wrote:
>>> Currently DIV_ROUND_CLOSEST() is only available for the kernel via
>>> include/linux/math.h.
>>>
>>> Expose it to userland as well by adding __KERNEL_DIV_ROUND_CLOSEST() as
>>> a common definition in uapi.
>>>
>>> Additionally, ensure it allows building ISO C applications by switching
>>> from the 'typeof' GNU extension to the ISO-friendly __typeof__.
>>
>> I am not convinced that it's a good idea to make the implementation of
>> kernel DIV_ROUND_CLOSEST() part of the kernel UAPI, which is what this
>> change effectively does.
>>
>> I'd at least like to get an ack from Andy Shevchenko first (Cc'd).
> 
> Thanks for Cc'ing me!
> 
> So, the history of the DIV_ROUND_UP() to appear in UAPI is a response to
> the ethtool change that missed the fact that this was a kernel internal macro.
> Giving a precedent there is no technical issues to add DIV_ROUND_CLOSEST()
> to UAPI as proposed. Main question here is: Does DRM headers in question
> (that are going to use it) really need this?
> 
> Interestingly that DRM also started using __KERNEL_DIV_ROUND_UP() in UAPI
> at some point, which kinda makes an argument for allowing the other one.
> 
> Also fun fact: this series plead for a new macro for division while ignoring
> existing (UAPI) macros for masks and bits.

Yeah, I initially planned to make use of those, but gave up after realizing the
FIELD_{PREP|GET} were not exposed. I reconsidered my position now that you
mentioned it. :-)

> 
> 0xffffU is effectively __GENMASK(15, 0). (And if you change the code, avoid
> using variables inside GENMASK() macros, it may generate an awful code, the
> GENMASK($HI, $LO) << foo is preferred over GENMASK(foo + $DELTA, foo) case.
> GENMASK(foo - 1, 0) OTOH is fine, however be always careful against overflows
> with left shifts, as BIT(foo) - 1 may not work for foo == 32, while GENMASK()
> may not work for foo == 0).

Thanks for the heads up!

> 
> So, I have no objections for either choice
> Acked-by: Andy Shevchenko <andriy.shevchenko at linux.intel.com>
> 
> ...
> 
> But if you go that direction, please, fix up the style.
> 
>>> +/*
>>> + * Divide positive or negative dividend by positive or negative divisor
>>> + * and round to closest integer. Result is undefined for negative
>>> + * divisors if the dividend variable type is unsigned and for negative
>>> + * dividends if the divisor variable type is unsigned.
>>> + */
>>> +#define __KERNEL_DIV_ROUND_CLOSEST(x, divisor)(		\
>>> +{							\
> 
> Use ({ on this line together...
> 
>>> +	__typeof__(x) __x = x;				\
>>> +	__typeof__(divisor) __d = divisor;		\
> 
> + blank line here.
> 
>>> +	(((__typeof__(x))-1) > 0 ||			\
>>> +	 ((__typeof__(divisor))-1) > 0 ||		\
>>> +	 (((__x) > 0) == ((__d) > 0))) ?		\
>>> +		(((__x) + ((__d) / 2)) / (__d)) :	\
>>> +		(((__x) - ((__d) / 2)) / (__d));	\
>>> +}							\
>>> +)
> 
> ...as here join }) to be a single line.
> 
> + blank line.
> 
>>>  #endif /* _UAPI_LINUX_CONST_H */


Thanks for the review! I've addressed all your comments (hopefully) in v6:

https://lore.kernel.org/all/20260129-rk3588-bgcolor-v6-0-c15f755a4055@collabora.com/

Regards,
Cristian