* [PATCH hotfix 6.11 v2 1/3] minmax: reduce min/max macro expansion in mvpp2 driver [not found] <cover.1726074904.git.lorenzo.stoakes@oracle.com> @ 2024-09-11 17:51 ` Lorenzo Stoakes 2024-09-11 17:51 ` [PATCH hotfix 6.11 v2 2/3] minmax: reduce min/max macro expansion in skbuff Lorenzo Stoakes 2024-09-11 17:51 ` [PATCH hotfix 6.11 v2 3/3] minmax: reduce min/max macro expansion in atomisp driver Lorenzo Stoakes 2 siblings, 0 replies; 6+ messages in thread From: Lorenzo Stoakes @ 2024-09-11 17:51 UTC (permalink / raw) To: Andrew Morton Cc: Richard Narron, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Hans de Goede, Mauro Carvalho Chehab, Sakari Ailus, Greg Kroah-Hartman, Marcin Wojtas, Russell King, David S . Miller, Arnd Bergmann, Linus Torvalds, netdev, linux-kernel, linux-media, linux-staging, linux-mm, Andrew Lunn, Dan Carpenter, stable Avoid unnecessary nested min()/max() which reults in egregious macro expansion. Use clamp_t() as this introduces the least possible expansion. Not doing so results in an impact on build times. Cc: stable@vger.kernel.org Fixes: 867046cc7027 ("minmax: relax check to allow comparison between unsigned arguments and signed constants") Reviewed-by: Andrew Lunn <andrew@lunn.ch> Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com> --- drivers/net/ethernet/marvell/mvpp2/mvpp2.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2.h b/drivers/net/ethernet/marvell/mvpp2/mvpp2.h index e809f91c08fb..8b431f90efc3 100644 --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2.h +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2.h @@ -23,7 +23,7 @@ /* The PacketOffset field is measured in units of 32 bytes and is 3 bits wide, * so the maximum offset is 7 * 32 = 224 */ -#define MVPP2_SKB_HEADROOM min(max(XDP_PACKET_HEADROOM, NET_SKB_PAD), 224) +#define MVPP2_SKB_HEADROOM clamp_t(int, XDP_PACKET_HEADROOM, NET_SKB_PAD, 224) #define MVPP2_XDP_PASS 0 #define MVPP2_XDP_DROPPED BIT(0) -- 2.46.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH hotfix 6.11 v2 2/3] minmax: reduce min/max macro expansion in skbuff [not found] <cover.1726074904.git.lorenzo.stoakes@oracle.com> 2024-09-11 17:51 ` [PATCH hotfix 6.11 v2 1/3] minmax: reduce min/max macro expansion in mvpp2 driver Lorenzo Stoakes @ 2024-09-11 17:51 ` Lorenzo Stoakes 2024-09-11 17:52 ` kernel test robot 2024-09-11 17:51 ` [PATCH hotfix 6.11 v2 3/3] minmax: reduce min/max macro expansion in atomisp driver Lorenzo Stoakes 2 siblings, 1 reply; 6+ messages in thread From: Lorenzo Stoakes @ 2024-09-11 17:51 UTC (permalink / raw) To: Andrew Morton Cc: Richard Narron, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Hans de Goede, Mauro Carvalho Chehab, Sakari Ailus, Greg Kroah-Hartman, Marcin Wojtas, Russell King, David S . Miller, Arnd Bergmann, Linus Torvalds, netdev, linux-kernel, linux-media, linux-staging, linux-mm, Andrew Lunn, Dan Carpenter, stable Avoid unnecessary nested min()/max() which reults in egregious macro expansion. The nesting occurs when NET_SKB_PAD is used in a min() or max() invocation, for instance, various ethernet drivers wrap it in a max(). Not doing so results in an impact on build times. Cc: stable@vger.kernel.org Fixes: 867046cc7027 ("minmax: relax check to allow comparison between unsigned arguments and signed constants") Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com> --- include/linux/skbuff.h | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h index 29c3ea5b6e93..d53b296df504 100644 --- a/include/linux/skbuff.h +++ b/include/linux/skbuff.h @@ -3164,7 +3164,11 @@ static inline int pskb_network_may_pull(struct sk_buff *skb, unsigned int len) * NET_IP_ALIGN(2) + ethernet_header(14) + IP_header(20/40) + ports(8) */ #ifndef NET_SKB_PAD -#define NET_SKB_PAD max(32, L1_CACHE_BYTES) +#if L1_CACHE_BYTES < 32 +#define NET_SKB_PAD 32 +#else +#define NET_SKB_PAD L1_CACHE_BYTES +#endif #endif int ___pskb_trim(struct sk_buff *skb, unsigned int len); -- 2.46.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH hotfix 6.11 v2 2/3] minmax: reduce min/max macro expansion in skbuff 2024-09-11 17:51 ` [PATCH hotfix 6.11 v2 2/3] minmax: reduce min/max macro expansion in skbuff Lorenzo Stoakes @ 2024-09-11 17:52 ` kernel test robot 0 siblings, 0 replies; 6+ messages in thread From: kernel test robot @ 2024-09-11 17:52 UTC (permalink / raw) To: Lorenzo Stoakes; +Cc: stable, oe-kbuild-all Hi, Thanks for your patch. FYI: kernel test robot notices the stable kernel rule is not satisfied. The check is based on https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html#option-3 Rule: The upstream commit ID must be specified with a separate line above the commit text. Subject: [PATCH hotfix 6.11 v2 2/3] minmax: reduce min/max macro expansion in skbuff Link: https://lore.kernel.org/stable/a32131084038312529243bc22b06f0ee64c95fb6.1726074904.git.lorenzo.stoakes%40oracle.com Please ignore this mail if the patch is not relevant for upstream. -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH hotfix 6.11 v2 3/3] minmax: reduce min/max macro expansion in atomisp driver [not found] <cover.1726074904.git.lorenzo.stoakes@oracle.com> 2024-09-11 17:51 ` [PATCH hotfix 6.11 v2 1/3] minmax: reduce min/max macro expansion in mvpp2 driver Lorenzo Stoakes 2024-09-11 17:51 ` [PATCH hotfix 6.11 v2 2/3] minmax: reduce min/max macro expansion in skbuff Lorenzo Stoakes @ 2024-09-11 17:51 ` Lorenzo Stoakes 2024-09-11 18:03 ` Hans de Goede 2024-09-11 18:11 ` Linus Torvalds 2 siblings, 2 replies; 6+ messages in thread From: Lorenzo Stoakes @ 2024-09-11 17:51 UTC (permalink / raw) To: Andrew Morton Cc: Richard Narron, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Hans de Goede, Mauro Carvalho Chehab, Sakari Ailus, Greg Kroah-Hartman, Marcin Wojtas, Russell King, David S . Miller, Arnd Bergmann, Linus Torvalds, netdev, linux-kernel, linux-media, linux-staging, linux-mm, Andrew Lunn, Dan Carpenter, stable Avoid unnecessary nested min()/max() which results in egregious macro expansion. Use clamp_t() as this introduces the least possible expansion. Not doing so results in an impact on build times. 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 patch should be easier and fix's Richard's problem in 5.15. Reported-by: Richard Narron <richard@aaazen.com> Closes: https://lore.kernel.org/all/4a5321bd-b1f-1832-f0c-cea8694dc5aa@aaazen.com/ Fixes: 867046cc7027 ("minmax: relax check to allow comparison between unsigned arguments and signed constants") Cc: stable@vger.kernel.org Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com> --- .../staging/media/atomisp/pci/sh_css_frac.h | 26 ++++++++++++++----- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/drivers/staging/media/atomisp/pci/sh_css_frac.h b/drivers/staging/media/atomisp/pci/sh_css_frac.h index b90b5b330dfa..8ba65161f7a9 100644 --- a/drivers/staging/media/atomisp/pci/sh_css_frac.h +++ b/drivers/staging/media/atomisp/pci/sh_css_frac.h @@ -32,12 +32,24 @@ #define uISP_VAL_MAX ((unsigned int)((1 << uISP_REG_BIT) - 1)) /* a:fraction bits for 16bit precision, b:fraction bits for ISP precision */ -#define sDIGIT_FITTING(v, a, b) \ - min_t(int, max_t(int, (((v) >> sSHIFT) >> max(sFRACTION_BITS_FITTING(a) - (b), 0)), \ - sISP_VAL_MIN), sISP_VAL_MAX) -#define uDIGIT_FITTING(v, a, b) \ - min((unsigned int)max((unsigned)(((v) >> uSHIFT) \ - >> max((int)(uFRACTION_BITS_FITTING(a) - (b)), 0)), \ - uISP_VAL_MIN), uISP_VAL_MAX) +static inline int sDIGIT_FITTING(int v, int a, int b) +{ + int fit_shift = sFRACTION_BITS_FITTING(a) - b; + + v >>= sSHIFT; + v >>= fit_shift > 0 ? fit_shift : 0; + + return clamp_t(int, v, sISP_VAL_MIN, sISP_VAL_MAX); +} + +static inline unsigned int uDIGIT_FITTING(unsigned int v, int a, int b) +{ + int fit_shift = uFRACTION_BITS_FITTING(a) - b; + + v >>= uSHIFT; + v >>= fit_shift > 0 ? fit_shift : 0; + + return clamp_t(unsigned int, v, uISP_VAL_MIN, uISP_VAL_MAX); +} #endif /* __SH_CSS_FRAC_H */ -- 2.46.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH hotfix 6.11 v2 3/3] minmax: reduce min/max macro expansion in atomisp driver 2024-09-11 17:51 ` [PATCH hotfix 6.11 v2 3/3] minmax: reduce min/max macro expansion in atomisp driver Lorenzo Stoakes @ 2024-09-11 18:03 ` Hans de Goede 2024-09-11 18:11 ` Linus Torvalds 1 sibling, 0 replies; 6+ messages in thread From: Hans de Goede @ 2024-09-11 18:03 UTC (permalink / raw) To: Lorenzo Stoakes, Andrew Morton Cc: Richard Narron, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Mauro Carvalho Chehab, Sakari Ailus, Greg Kroah-Hartman, Marcin Wojtas, Russell King, David S . Miller, Arnd Bergmann, Linus Torvalds, netdev, linux-kernel, linux-media, linux-staging, linux-mm, Andrew Lunn, Dan Carpenter, stable Hi, On 9/11/24 7:51 PM, Lorenzo Stoakes wrote: > Avoid unnecessary nested min()/max() which results in egregious macro > expansion. Use clamp_t() as this introduces the least possible expansion. > > Not doing so results in an impact on build times. > > 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 patch > should be easier and fix's Richard's problem in 5.15. > > Reported-by: Richard Narron <richard@aaazen.com> > Closes: https://lore.kernel.org/all/4a5321bd-b1f-1832-f0c-cea8694dc5aa@aaazen.com/ > Fixes: 867046cc7027 ("minmax: relax check to allow comparison between unsigned arguments and signed constants") > Cc: stable@vger.kernel.org > Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com> Thanks, patch looks good to me: Reviewed-by: Hans de Goede <hdegoede@redhat.com> Regards, Hans > --- > .../staging/media/atomisp/pci/sh_css_frac.h | 26 ++++++++++++++----- > 1 file changed, 19 insertions(+), 7 deletions(-) > > diff --git a/drivers/staging/media/atomisp/pci/sh_css_frac.h b/drivers/staging/media/atomisp/pci/sh_css_frac.h > index b90b5b330dfa..8ba65161f7a9 100644 > --- a/drivers/staging/media/atomisp/pci/sh_css_frac.h > +++ b/drivers/staging/media/atomisp/pci/sh_css_frac.h > @@ -32,12 +32,24 @@ > #define uISP_VAL_MAX ((unsigned int)((1 << uISP_REG_BIT) - 1)) > > /* a:fraction bits for 16bit precision, b:fraction bits for ISP precision */ > -#define sDIGIT_FITTING(v, a, b) \ > - min_t(int, max_t(int, (((v) >> sSHIFT) >> max(sFRACTION_BITS_FITTING(a) - (b), 0)), \ > - sISP_VAL_MIN), sISP_VAL_MAX) > -#define uDIGIT_FITTING(v, a, b) \ > - min((unsigned int)max((unsigned)(((v) >> uSHIFT) \ > - >> max((int)(uFRACTION_BITS_FITTING(a) - (b)), 0)), \ > - uISP_VAL_MIN), uISP_VAL_MAX) > +static inline int sDIGIT_FITTING(int v, int a, int b) > +{ > + int fit_shift = sFRACTION_BITS_FITTING(a) - b; > + > + v >>= sSHIFT; > + v >>= fit_shift > 0 ? fit_shift : 0; > + > + return clamp_t(int, v, sISP_VAL_MIN, sISP_VAL_MAX); > +} > + > +static inline unsigned int uDIGIT_FITTING(unsigned int v, int a, int b) > +{ > + int fit_shift = uFRACTION_BITS_FITTING(a) - b; > + > + v >>= uSHIFT; > + v >>= fit_shift > 0 ? fit_shift : 0; > + > + return clamp_t(unsigned int, v, uISP_VAL_MIN, uISP_VAL_MAX); > +} > > #endif /* __SH_CSS_FRAC_H */ > -- > 2.46.0 > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH hotfix 6.11 v2 3/3] minmax: reduce min/max macro expansion in atomisp driver 2024-09-11 17:51 ` [PATCH hotfix 6.11 v2 3/3] minmax: reduce min/max macro expansion in atomisp driver Lorenzo Stoakes 2024-09-11 18:03 ` Hans de Goede @ 2024-09-11 18:11 ` Linus Torvalds 1 sibling, 0 replies; 6+ messages in thread From: Linus Torvalds @ 2024-09-11 18:11 UTC (permalink / raw) To: Lorenzo Stoakes Cc: Andrew Morton, Richard Narron, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Hans de Goede, Mauro Carvalho Chehab, Sakari Ailus, Greg Kroah-Hartman, Marcin Wojtas, Russell King, David S . Miller, Arnd Bergmann, netdev, linux-kernel, linux-media, linux-staging, linux-mm, Andrew Lunn, Dan Carpenter, stable 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-09-11 18:11 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <cover.1726074904.git.lorenzo.stoakes@oracle.com>
2024-09-11 17:51 ` [PATCH hotfix 6.11 v2 1/3] minmax: reduce min/max macro expansion in mvpp2 driver Lorenzo Stoakes
2024-09-11 17:51 ` [PATCH hotfix 6.11 v2 2/3] minmax: reduce min/max macro expansion in skbuff Lorenzo Stoakes
2024-09-11 17:52 ` kernel test robot
2024-09-11 17:51 ` [PATCH hotfix 6.11 v2 3/3] minmax: reduce min/max macro expansion in atomisp driver Lorenzo Stoakes
2024-09-11 18:03 ` Hans de Goede
2024-09-11 18:11 ` Linus Torvalds
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox