mbox series

[hotfix,6.11,v2,0/3] minmax: reduce egregious min/max macro expansion

Message ID cover.1726074904.git.lorenzo.stoakes@oracle.com
Headers show
Series minmax: reduce egregious min/max macro expansion | expand

Message

Lorenzo Stoakes Sept. 11, 2024, 5:51 p.m. UTC 
Avoid nested min()/max() which results in egregious macro expansion.

This issue was introduced by commit 867046cc7027 ("minmax: relax check to
allow comparison between unsigned arguments and signed constants") [0].

Work has been done to address the issue of egregious min()/max() macro
expansion in commit 22f546873149 ("minmax: improve macro expansion and type
checking") and related, however it appears that some issues remain on more
tightly constrained systems.

Adjust a few known-bad cases of deeply nested macros to avoid doing so to
mitigate this. Porting the patch first proposed in [1] to Linus's tree.

Note that we use clamp_t() rather than clamp() when we otherwise could use
the latter because it isn't certain than the clamp() won't expand into a
more egregiously huge form, certainly prior to the recent fixes.

Running an allmodconfig build using the methodology described in [2] we
observe a 35 MiB reduction in generated code.

The difference is much more significant prior to recent minmax fixes which
were not backported. As per [1] prior these the reduction is more like 200
MiB.

This resolves an issue with slackware 15.0 32-bit compilation as reported
by Richard Narron.

Presumably the min/max fixups would be difficult to backport, this series
should be easier and fix's Richard's problem in 5.15.

[0]:https://lore.kernel.org/all/b97faef60ad24922b530241c5d7c933c@AcuMS.aculab.com/
[1]:https://lore.kernel.org/lkml/5882b96e-1287-4390-8174-3316d39038ef@lucifer.local/
[2]:https://lore.kernel.org/linux-mm/36aa2cad-1db1-4abf-8dd2-fb20484aabc3@lucifer.local/

v2:
* Split into separate patches to make backporting easier.
* Corrected type in sDIGIT_FITTING() argument.
* Added reviewed-by tags.

v1:
https://lore.kernel.org/all/20240911153457.1005227-1-lorenzo.stoakes@oracle.com/

Lorenzo Stoakes (3):
  minmax: reduce min/max macro expansion in mvpp2 driver
  minmax: reduce min/max macro expansion in skbuff
  minmax: reduce min/max macro expansion in atomisp driver

 drivers/net/ethernet/marvell/mvpp2/mvpp2.h    |  2 +-
 .../staging/media/atomisp/pci/sh_css_frac.h   | 26 ++++++++++++++-----
 include/linux/skbuff.h                        |  6 ++++-
 3 files changed, 25 insertions(+), 9 deletions(-)

--
2.46.0

Comments

Linus Torvalds Sept. 11, 2024, 6:11 p.m. UTC | #1 
On Wed, 11 Sept 2024 at 10:51, Lorenzo Stoakes
<lorenzo.stoakes@oracle.com> wrote:
>
> Avoid unnecessary nested min()/max() which results in egregious macro
> expansion. Use clamp_t() as this introduces the least possible expansion.

I took this (single) patch directly, since that's the one that
actually causes build problems in limited environments (admittedly not
in current git with the more invasive min/max cleanups, but in order
to be back-ported).

Plus it cleans up the code with more legible inline functions, rather
than just doing some minimal syntactic changes. I expanded on the
commit message to say that.

The two others I'll leave for now and see what maintainers of their
respective areas think.

            Linus