Re: [PATCH v2 03/13 6.1.y] minmax: simplify min()/max()/clamp() implementation

[Greg KH <gregkh@linuxfoundation.org>]

On Mon, Sep 29, 2025 at 06:33:48PM +0000, Eliav Farber wrote:
> From: Linus Torvalds <torvalds@linux-foundation.org>
> 
> [ Upstream commit dc1c8034e31b14a2e5e212104ec508aec44ce1b9 ]
> 
> Now that we no longer have any C constant expression contexts (ie array
> size declarations or static initializers) that use min() or max(), we
> can simpify the implementation by not having to worry about the result
> staying as a C constant expression.
> 
> So now we can unconditionally just use temporary variables of the right
> type, and get rid of the excessive expansion that used to come from the
> use of
> 
>    __builtin_choose_expr(__is_constexpr(...), ..
> 
> to pick the specialized code for constant expressions.
> 
> Another expansion simplification is to pass the temporary variables (in
> addition to the original expression) to our __types_ok() macro.  That
> may superficially look like it complicates the macro, but when we only
> want the type of the expression, expanding the temporary variable names
> is much simpler and smaller than expanding the potentially complicated
> original expression.
> 
> As a result, on my machine, doing a
> 
>   $ time make drivers/staging/media/atomisp/pci/isp/kernels/ynr/ynr_1.0/ia_css_ynr.host.i
> 
> goes from
> 
> 	real	0m16.621s
> 	user	0m15.360s
> 	sys	0m1.221s
> 
> to
> 
> 	real	0m2.532s
> 	user	0m2.091s
> 	sys	0m0.452s
> 
> because the token expansion goes down dramatically.
> 
> In particular, the longest line expansion (which was line 71 of that
> 'ia_css_ynr.host.c' file) shrinks from 23,338kB (yes, 23MB for one
> single line) to "just" 1,444kB (now "only" 1.4MB).
> 
> And yes, that line is still the line from hell, because it's doing
> multiple levels of "min()/max()" expansion thanks to some of them being
> hidden inside the uDIGIT_FITTING() macro.
> 
> Lorenzo has a nice cleanup patch that makes that driver use inline
> functions instead of macros for sDIGIT_FITTING() and uDIGIT_FITTING(),
> which will fix that line once and for all, but the 16-fold reduction in
> this case does show why we need to simplify these helpers.
> 
> Cc: David Laight <David.Laight@aculab.com>
> Cc: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
> Signed-off-by: Eliav Farber <farbere@amazon.com>
> ---
>  include/linux/minmax.h | 43 ++++++++++++++++++++----------------------
>  1 file changed, 20 insertions(+), 23 deletions(-)

This change breaks the build in drivers/md/ :

In file included from ./include/linux/container_of.h:5,
                 from ./include/linux/list.h:5,
                 from ./include/linux/wait.h:7,
                 from ./include/linux/mempool.h:8,
                 from ./include/linux/bio.h:8,
                 from drivers/md/dm-bio-record.h:10,
                 from drivers/md/dm-integrity.c:9:
drivers/md/dm-integrity.c: In function ‘integrity_metadata’:
drivers/md/dm-integrity.c:131:105: error: ISO C90 forbids variable length array ‘checksums_onstack’ [-Werror=vla]
  131 | #define MAX_TAG_SIZE                    (JOURNAL_SECTOR_DATA - JOURNAL_MAC_PER_SECTOR - offsetof(struct journal_entry, last_bytes[MAX_SECTORS_PER_BLOCK]))
      |                                                                                                         ^~~~~~~~~~~~~
./include/linux/build_bug.h:78:56: note: in definition of macro ‘__static_assert’
   78 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
      |                                                        ^~~~
./include/linux/minmax.h:56:9: note: in expansion of macro ‘static_assert’
   56 |         static_assert(__types_ok(x, y, ux, uy),         \
      |         ^~~~~~~~~~~~~
./include/linux/minmax.h:41:31: note: in expansion of macro ‘__is_noneg_int’
   41 |          __is_noneg_int(x) || __is_noneg_int(y))
      |                               ^~~~~~~~~~~~~~
./include/linux/minmax.h:56:23: note: in expansion of macro ‘__types_ok’
   56 |         static_assert(__types_ok(x, y, ux, uy),         \
      |                       ^~~~~~~~~~
./include/linux/minmax.h:61:9: note: in expansion of macro ‘__careful_cmp_once’
   61 |         __careful_cmp_once(op, x, y, __UNIQUE_ID(x_), __UNIQUE_ID(y_))
      |         ^~~~~~~~~~~~~~~~~~
./include/linux/minmax.h:92:25: note: in expansion of macro ‘__careful_cmp’
   92 | #define max(x, y)       __careful_cmp(max, x, y)
      |                         ^~~~~~~~~~~~~
drivers/md/dm-integrity.c:1797:40: note: in expansion of macro ‘max’
 1797 |                 char checksums_onstack[max((size_t)HASH_MAX_DIGESTSIZE, MAX_TAG_SIZE)];
      |                                        ^~~
drivers/md/dm-integrity.c:131:89: note: in expansion of macro ‘offsetof’
  131 | #define MAX_TAG_SIZE                    (JOURNAL_SECTOR_DATA - JOURNAL_MAC_PER_SECTOR - offsetof(struct journal_entry, last_bytes[MAX_SECTORS_PER_BLOCK]))
      |                                                                                         ^~~~~~~~
drivers/md/dm-integrity.c:1797:73: note: in expansion of macro ‘MAX_TAG_SIZE’
 1797 |                 char checksums_onstack[max((size_t)HASH_MAX_DIGESTSIZE, MAX_TAG_SIZE)];
      |                                                                         ^~~~~~~~~~~~


So I'll stop here on this series.

After the next release, can you rebase the series and resend the remaining ones after they are fixed up to build properly?

thanks,

greg k-h