Received-SPF: pass (google.com: domain of linux-kernel+bounces-208707-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Precedence: bulk
MIME-Version: 1.0
References: <20240608193504.429644-2-torvalds@linux-foundation.org> <20240610104352.GT8774@noisy.programming.kicks-ass.net>
In-Reply-To: <20240610104352.GT8774@noisy.programming.kicks-ass.net>
From: Linus Torvalds <torvalds@linux-foundation.org>
Date: Mon, 10 Jun 2024 11:06:44 -0700
Message-ID: <CAHk-=wh5DAR=a12cbKbxDy875hTOtPmNUDEn+dU2VS47h9MgcQ@mail.gmail.com>
Subject: Re: [PATCH] x86: add 'runtime constant' infrastructure
To: Peter Zijlstra <peterz@infradead.org>
Cc: Peter Anvin <hpa@zytor.com>, Ingo Molnar <mingo@kernel.org>, Borislav Petkov <bp@alien8.de>, 
	Thomas Gleixner <tglx@linutronix.de>, Rasmus Villemoes <linux@rasmusvillemoes.dk>, 
	Josh Poimboeuf <jpoimboe@kernel.org>, 
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>, "the arch/x86 maintainers" <x86@kernel.org>, 
	linux-arch <linux-arch@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"

On Mon, 10 Jun 2024 at 03:43, Peter Zijlstra <peterz@infradead.org> wrote:
>
> --- linux-2.6.orig/arch/Kconfig
> +++ linux-2.6/arch/Kconfig
> @@ -1492,6 +1492,9 @@ config HAVE_SPARSE_SYSCALL_NR
>  config ARCH_HAS_VDSO_DATA
>         bool
>
> +config HAVE_RUNTIME_CONST
> +       bool

No. We're not adding a stupid config variable, when nothing actually wants it.

> +#define __runtime_const(sym, op, type)                 \
> +({                                                     \
> +       typeof(sym) __ret;                              \
> +       asm(op " %1, %0\n1:\n"                          \
> +           ".pushsection __runtime_const, \"aw\"\n\t"  \
> +           ".long %c3 - .      # sym \n\t"             \
> +           ".long %c2          # size \n\t"            \
> +           ".long 1b - %c2 - . # addr \n\t"            \
> +           ".popsection\n\t"                           \
> +           : "=r" (__ret)                              \
> +           : "i" ((type)0x0123456789abcdefull),        \
> +             "i" (sizeof(type)),                       \
> +             "i" ((void *)&sym));                      \
> +       __ret;                                          \
> +})
> +
> +#define runtime_const(sym)                                             \
> +({                                                                     \
> +       typeof(sym) __ret;                                              \
> +       switch(sizeof(sym)) {                                           \
> +       case 1: __ret = __runtime_const(sym, "movb", u8); break;        \
> +       case 2: __ret = __runtime_const(sym, "movs", u16); break;       \
> +       case 4: __ret = __runtime_const(sym, "movl", u32); break;       \
> +       case 8: __ret = __runtime_const(sym, "movq", u64); break;       \
> +       default: BUG();                                                 \
> +       }                                                               \
> +       __ret;                                                          \
> +})

And no. We're not adding magic "generic" helpers that have zero use
and just make the code harder to read, and don't even work on 32-bit
x86 anyway.

Because I didn't test, but I am pretty sure that clang will not
compile the above on x86-32, because clang verifies the inline asm,
and "movq" isn't a valid instruction.

We had exactly that for the uaccess macros, and needed to special-case
the 64-bit case for that reason.

And we don't *need* to. All of the above is garbage waiting for a use
that shouldn't exist.

> +++ linux-2.6/kernel/runtime_const.c
> @@ -0,0 +1,119 @@

And here you basically tripled the size of the patch in order to have
just one section, when I had per-symbol sections.

So no.

I envision a *couple* of runtime constants. The absolutely only reason
to use a runtime constant is that it is *so* hot that the difference
between "load a variable" and "write code with a constant" is
noticeable.

I can point to exactly one such case in the kernel right now.

I'm sure there are others, but I'd expect that "others" to be mainly a handful.

This needs to be simple, obvious, and literally designed for very targeted use.

This is a *very* special case for a *very* special code. Not for generic use.

I do not ever expect to see this used by modules, for example. There
is no way in hell I will expose the instruction rewriting to a module.
The *use* of a constant is a _maybe_, but it's questionable too. By
definition, module code cannot be all that core.

             Linus