linux-kernel - Re: [PATCH v5 1/4] uapi: Provide DIV_ROUND

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <c9f51c59-c972-420e-89da-f42ab5a843c0@collabora.com>
Date: Thu, 29 Jan 2026 03:14:41 +0200
From: Cristian Ciocaltea <cristian.ciocaltea@...labora.com>
To: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
 Jani Nikula <jani.nikula@...ux.intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
 Maxime Ripard <mripard@...nel.org>, Thomas Zimmermann <tzimmermann@...e.de>,
 David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
 Sandy Huang <hjc@...k-chips.com>, Heiko Stübner
 <heiko@...ech.de>, Andy Yan <andy.yan@...k-chips.com>,
 Louis Chauvet <louis.chauvet@...tlin.com>,
 Haneen Mohammed <hamohammed.sa@...il.com>,
 Melissa Wen <melissa.srw@...il.com>,
 Robert Mader <robert.mader@...labora.com>, kernel@...labora.com,
 dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org,
 linux-arm-kernel@...ts.infradead.org, linux-rockchip@...ts.infradead.org,
 Nícolas F. R. A. Prado <nfraprado@...labora.com>,
 Diederik de Haas <diederik@...ow-tech.com>
Subject: Re: [PATCH v5 1/4] uapi: Provide DIV_ROUND_CLOSEST()

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@...labora.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@...ux.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