Re: [PATCH 0/7] minmax: reduce compilation time

[Linus Torvalds]

On Fri, 26 Jul 2024 at 12:21, Lorenzo Stoakes
<lorenzo.stoakes@xxxxxxxxxx> wrote:
>
> A simple comparison pre-revert vs. post-revert gives some ideas for other
> low-hanging fruit:
>
>   1334256./mm/compaction.o.pre

LOL. At least some of that is because of 'pageblock_order', which
expands to 2.5kB of text just because of this:

  /*
   * Huge pages are a constant size, but don't exceed the maximum allocation
   * granularity.
   */
  #define pageblock_order    min_t(unsigned int, HUGETLB_PAGE_ORDER,
MAX_PAGE_ORDER)

I think the two arguments to "min_t" are literally "(21 - 12)" and
"10", and it expands to 2.5kB.

So it _looks_ like "pageblock_order", and it *acts* like a simple
compile-time constant, but our complex type-checking min() macro ends
up making it horrible.

But no, that's not nearly the longest expansion. Writing a little
script, and I get

   Longest line is 85061 (253kB)

so we have a single expansion that is 253kB in size. And it comes from this:

                case ISOLATE_SUCCESS:
                        update_cached = false;
                        last_migrated_pfn = max(cc->zone->zone_start_pfn,
                                pageblock_start_pfn(cc->migrate_pfn - 1));

where that "max()" ends up interacting with "pageblock_start_pfn()",
and that pageblock_start_pfn() thing is

  #define pageblock_nr_pages      (1UL << pageblock_order)
  #define pageblock_start_pfn(pfn)  ALIGN_DOWN((pfn), pageblock_nr_pages)

so once again it's "pageblock_order", it's just that it's now mixed in
with "max()".

Now, fixing that, and you end up with

  Longest line is 61861 (82kB)

so it's now "only" 82kB in size, and that actually comes from
<linux/bio.h>, which has this:

   static inline unsigned bio_segments(struct bio *bio)
   {
   ...
        bio_for_each_segment(bv, bio, iter)
                segs++;

which looks very tame indeed, but it turns out that
"bio_for_each_segment()" expands to 82kB of code.

Jens? Maybe time to look into this?

               Linus
 arch/x86/xen/setup.c            | 5 +++--
 include/linux/minmax.h          | 7 +++++++
 include/linux/pageblock-flags.h | 4 ++--
 3 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/arch/x86/xen/setup.c b/arch/x86/xen/setup.c
index a0c3e77e3d5b..806ddb2391d9 100644
--- a/arch/x86/xen/setup.c
+++ b/arch/x86/xen/setup.c
@@ -690,6 +690,7 @@ char * __init xen_memory_setup(void)
 	struct xen_memory_map memmap;
 	unsigned long max_pages;
 	unsigned long extra_pages = 0;
+	unsigned long maxmem_pages;
 	int i;
 	int op;
 
@@ -761,8 +762,8 @@ char * __init xen_memory_setup(void)
 	 * Make sure we have no memory above max_pages, as this area
 	 * isn't handled by the p2m management.
 	 */
-	extra_pages = min3(EXTRA_MEM_RATIO * min(max_pfn, PFN_DOWN(MAXMEM)),
-			   extra_pages, max_pages - max_pfn);
+	maxmem_pages = EXTRA_MEM_RATIO * min(max_pfn, PFN_DOWN(MAXMEM));
+	extra_pages = min3(maxmem_pages, extra_pages, max_pages - max_pfn);
 	i = 0;
 	addr = xen_e820_table.entries[0].addr;
 	size = xen_e820_table.entries[0].size;
diff --git a/include/linux/minmax.h b/include/linux/minmax.h
index 2ec559284a9f..5e0c02a87d08 100644
--- a/include/linux/minmax.h
+++ b/include/linux/minmax.h
@@ -7,6 +7,13 @@
 #include <linux/const.h>
 #include <linux/types.h>
 
+/*
+ * Use these carefully: no type checking, and uses the arguments
+ * multiple times. Use for obvious constants only.
+ */
+#define CONST_MIN(a,b) ((a)<(b)?(a):(b))
+#define CONST_MAX(a,b) ((a)>(b)?(a):(b))
+
 /*
  * min()/max()/clamp() macros must accomplish three things:
  *
diff --git a/include/linux/pageblock-flags.h b/include/linux/pageblock-flags.h
index 547e82cdc89a..ce1b7c4d57e2 100644
--- a/include/linux/pageblock-flags.h
+++ b/include/linux/pageblock-flags.h
@@ -41,13 +41,13 @@ extern unsigned int pageblock_order;
  * Huge pages are a constant size, but don't exceed the maximum allocation
  * granularity.
  */
-#define pageblock_order		min_t(unsigned int, HUGETLB_PAGE_ORDER, MAX_PAGE_ORDER)
+#define pageblock_order	((unsigned int)CONST_MIN(HUGETLB_PAGE_ORDER, MAX_PAGE_ORDER))
 
 #endif /* CONFIG_HUGETLB_PAGE_SIZE_VARIABLE */
 
 #elif defined(CONFIG_TRANSPARENT_HUGEPAGE)
 
-#define pageblock_order		min_t(unsigned int, HPAGE_PMD_ORDER, MAX_PAGE_ORDER)
+#define pageblock_order ((unsigned int)CONST_MIN(HPAGE_PMD_ORDER, MAX_PAGE_ORDER))
 
 #else /* CONFIG_TRANSPARENT_HUGEPAGE */
 
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <sys/stat.h>

static void die(const char *reason)
{
	fprintf(stderr, "Fatal: %s\n", reason);
	exit(1);
}

static unsigned long find_line(const char *line, unsigned long len)
{
	unsigned long res = 0;
	do {
		res++;
		if (*line == '\n')
			break;
		line++;
	} while (--len);
	return res;
}

int main(int argc, char **argv)
{
	struct stat st;
	char *buf;
	unsigned long len;
	long longest = 0;
	char *line = "";
	int lineno = 0, i;

	if (fstat(0, &st)) die("stat");
	if (!S_ISREG(st.st_mode)) die("Not a regularfile");
	len = st.st_size;
	buf = malloc(len);
	if (!buf) die("malloc failed");
	if (read(0, buf, len) != len) die("read failed");

	for (i = 1; len; i++) {
		unsigned long linelen;
		linelen = find_line(buf, len);
		if (linelen > longest) {
			longest = linelen;
			line = buf;
			lineno = i;
		}
		buf += linelen;
		len -= linelen;
	}

	printf("Longest line is %d (%lukB)\n", lineno, (longest+512) / 1024);
	printf("   '%.*s'\n", (int) longest-1, line);
	return 0;
}