[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <eea5d9a4-fcb4-4076-8222-fcff668a2bfa@lucifer.local>
Date: Sat, 27 Jul 2024 19:58:52 +0100
From: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
To: David Laight <David.Laight@...lab.com>
Cc: 'Linus Torvalds' <torvalds@...uxfoundation.org>,
Jens Axboe <axboe@...nel.dk>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"Matthew Wilcox (Oracle)" <willy@...radead.org>,
Christoph Hellwig <hch@...radead.org>,
Andrew Morton <akpm@...ux-foundation.org>,
Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
Dan Carpenter <dan.carpenter@...aro.org>,
Arnd Bergmann <arnd@...nel.org>, "Jason@...c4.com" <Jason@...c4.com>,
"pedro.falcato@...il.com" <pedro.falcato@...il.com>,
Mateusz Guzik <mjguzik@...il.com>,
"linux-mm@...ck.org" <linux-mm@...ck.org>
Subject: Re: [PATCH 0/7] minmax: reduce compilation time
On Sat, Jul 27, 2024 at 08:08:39AM GMT, David Laight wrote:
> ...
> > and it will spit out
> >
> > Longest line is drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c:1136 (2346kB)
> > ' ((((((pkt_size) + __builtin_choose_expr((sizeof(int) ==
> > sizeof(*(8 ? ((void *)((long)((__builtin_...'
> >
> > to tell me that we have that insane 2.2 *megabyte* line due to the
> > MVPP2_SKB_HEADROOM thing, and I should apply this patch:
> >
> > -#define MVPP2_SKB_HEADROOM min(max(XDP_PACKET_HEADROOM, NET_SKB_PAD), 224)
> > +#define MVPP2_SKB_HEADROOM
> > MIN_T(int,MAX_T(int,XDP_PACKET_HEADROOM, NET_SKB_PAD), 224)
> >
> > to fix it.
Yeah sorry just saw you had already addresed this Linus... I just went with a
clamp()_t in my patch.
>
> Or (if I've got is right):
> #define MVPP2_SKB_HEADROOM clamp(XDP_PACKET_HEADROOM, NET_SKB_PAD, 224)
>
I'm pretty sure you can clamp_t(int, ...) here safely based on usage.
Part of the expansion is because of NET_SKB_PAD as you mention below...
> Hmmm...
> I've found:
> #define XDP_PACKET_HEADROOM 256
> #define NET_SKB_PAD max(32, L1_CACHE_BYTES)
> I'd bet that some architecture even has a non-constant L1_CACHE_BYTES.
I checked and unless I missed something, nope.
You'll see in my proposed patch that I jut replace this with a dumb #if as a
result.
>
> But the 256 means the headroom is always 224 (whatever that limit is related to).
>
> It is definitely worth freeing up MIN() and MAX() for:
> #define MIN(x, y) ( \
> BUILD_BUG_ON_ZERO(__is_constexpr((x) + (y)) + ((x) < (y) ? (x) : (y)))
> which is then usable for static initialisers.
> Just assuming that no one is silly enough to get a negative constant
> compared to an unsigned value.
>
> David
>
> -
> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
> Registration No: 1397386 (Wales)
Powered by blists - more mailing lists