2019-06-05 13:25:22

by Peter Zijlstra

[permalink] [raw]
Subject: [PATCH 10/15] static_call: Add basic static call infrastructure

From: Josh Poimboeuf <[email protected]>

Static calls are a replacement for global function pointers. They use
code patching to allow direct calls to be used instead of indirect
calls. They give the flexibility of function pointers, but with
improved performance. This is especially important for cases where
retpolines would otherwise be used, as retpolines can significantly
impact performance.

The concept and code are an extension of previous work done by Ard
Biesheuvel and Steven Rostedt:

https://lkml.kernel.org/r/[email protected]
https://lkml.kernel.org/r/[email protected]

There are two implementations, depending on arch support:

1) out-of-line: patched trampolines (CONFIG_HAVE_STATIC_CALL)
2) basic function pointers

For more details, see the comments in include/linux/static_call.h.

Cc: [email protected]
Cc: Steven Rostedt <[email protected]>
Cc: Julia Cartwright <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Ard Biesheuvel <[email protected]>
Cc: Jason Baron <[email protected]>
Cc: Rasmus Villemoes <[email protected]>
Cc: Daniel Bristot de Oliveira <[email protected]>
Cc: Linus Torvalds <[email protected]>
Cc: Jiri Kosina <[email protected]>
Cc: Edward Cree <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Masami Hiramatsu <[email protected]>
Cc: Borislav Petkov <[email protected]>
Cc: David Laight <[email protected]>
Cc: Jessica Yu <[email protected]>
Cc: Nadav Amit <[email protected]>
Cc: Andy Lutomirski <[email protected]>
Cc: "H. Peter Anvin" <[email protected]>
Signed-off-by: Josh Poimboeuf <[email protected]>
Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
Link: https://lkml.kernel.org/r/a01f733889ebf4bc447507ab8041a60378eaa89f.1547073843.git.jpoimboe@redhat.com
---
arch/Kconfig | 3
include/linux/static_call.h | 135 ++++++++++++++++++++++++++++++++++++++
include/linux/static_call_types.h | 13 +++
3 files changed, 151 insertions(+)
create mode 100644 include/linux/static_call.h
create mode 100644 include/linux/static_call_types.h

--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -927,6 +927,9 @@ config LOCK_EVENT_COUNTS
the chance of application behavior change because of timing
differences. The counts are reported via debugfs.

+config HAVE_STATIC_CALL
+ bool
+
source "kernel/gcov/Kconfig"

source "scripts/gcc-plugins/Kconfig"
--- /dev/null
+++ b/include/linux/static_call.h
@@ -0,0 +1,135 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_STATIC_CALL_H
+#define _LINUX_STATIC_CALL_H
+
+/*
+ * Static call support
+ *
+ * Static calls use code patching to hard-code function pointers into direct
+ * branch instructions. They give the flexibility of function pointers, but
+ * with improved performance. This is especially important for cases where
+ * retpolines would otherwise be used, as retpolines can significantly impact
+ * performance.
+ *
+ *
+ * API overview:
+ *
+ * DECLARE_STATIC_CALL(key, func);
+ * DEFINE_STATIC_CALL(key, func);
+ * static_call(key, args...);
+ * static_call_update(key, func);
+ *
+ *
+ * Usage example:
+ *
+ * # Start with the following functions (with identical prototypes):
+ * int func_a(int arg1, int arg2);
+ * int func_b(int arg1, int arg2);
+ *
+ * # Define a 'my_key' reference, associated with func_a() by default
+ * DEFINE_STATIC_CALL(my_key, func_a);
+ *
+ * # Call func_a()
+ * static_call(my_key, arg1, arg2);
+ *
+ * # Update 'my_key' to point to func_b()
+ * static_call_update(my_key, func_b);
+ *
+ * # Call func_b()
+ * static_call(my_key, arg1, arg2);
+ *
+ *
+ * Implementation details:
+ *
+ * This requires some arch-specific code (CONFIG_HAVE_STATIC_CALL).
+ * Otherwise basic indirect calls are used (with function pointers).
+ *
+ * Each static_call() site calls into a trampoline associated with the key.
+ * The trampoline has a direct branch to the default function. Updates to a
+ * key will modify the trampoline's branch destination.
+ */
+
+#include <linux/types.h>
+#include <linux/cpu.h>
+#include <linux/static_call_types.h>
+
+#ifdef CONFIG_HAVE_STATIC_CALL
+#include <asm/static_call.h>
+extern void arch_static_call_transform(void *site, void *tramp, void *func);
+#endif
+
+
+#define DECLARE_STATIC_CALL(key, func) \
+ extern struct static_call_key key; \
+ extern typeof(func) STATIC_CALL_TRAMP(key)
+
+
+#if defined(CONFIG_HAVE_STATIC_CALL)
+
+struct static_call_key {
+ void *func, *tramp;
+};
+
+#define DEFINE_STATIC_CALL(key, _func) \
+ DECLARE_STATIC_CALL(key, _func); \
+ struct static_call_key key = { \
+ .func = _func, \
+ .tramp = STATIC_CALL_TRAMP(key), \
+ }; \
+ ARCH_DEFINE_STATIC_CALL_TRAMP(key, _func)
+
+#define static_call(key, args...) STATIC_CALL_TRAMP(key)(args)
+
+static inline void __static_call_update(struct static_call_key *key, void *func)
+{
+ cpus_read_lock();
+ WRITE_ONCE(key->func, func);
+ arch_static_call_transform(NULL, key->tramp, func);
+ cpus_read_unlock();
+}
+
+#define static_call_update(key, func) \
+({ \
+ BUILD_BUG_ON(!__same_type(func, STATIC_CALL_TRAMP(key))); \
+ __static_call_update(&key, func); \
+})
+
+#define EXPORT_STATIC_CALL(key) \
+ EXPORT_SYMBOL(STATIC_CALL_TRAMP(key))
+
+#define EXPORT_STATIC_CALL_GPL(key) \
+ EXPORT_SYMBOL_GPL(STATIC_CALL_TRAMP(key))
+
+
+#else /* Generic implementation */
+
+struct static_call_key {
+ void *func;
+};
+
+#define DEFINE_STATIC_CALL(key, _func) \
+ DECLARE_STATIC_CALL(key, _func); \
+ struct static_call_key key = { \
+ .func = _func, \
+ }
+
+#define static_call(key, args...) \
+ ((typeof(STATIC_CALL_TRAMP(key))*)(key.func))(args)
+
+static inline void __static_call_update(struct static_call_key *key, void *func)
+{
+ WRITE_ONCE(key->func, func);
+}
+
+#define static_call_update(key, func) \
+({ \
+ BUILD_BUG_ON(!__same_type(func, STATIC_CALL_TRAMP(key))); \
+ __static_call_update(&key, func); \
+})
+
+#define EXPORT_STATIC_CALL(key) EXPORT_SYMBOL(key)
+#define EXPORT_STATIC_CALL_GPL(key) EXPORT_SYMBOL_GPL(key)
+
+#endif /* CONFIG_HAVE_STATIC_CALL */
+
+#endif /* _LINUX_STATIC_CALL_H */
--- /dev/null
+++ b/include/linux/static_call_types.h
@@ -0,0 +1,13 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _STATIC_CALL_TYPES_H
+#define _STATIC_CALL_TYPES_H
+
+#include <linux/stringify.h>
+
+#define STATIC_CALL_TRAMP_PREFIX ____static_call_tramp_
+#define STATIC_CALL_TRAMP_PREFIX_STR __stringify(STATIC_CALL_TRAMP_PREFIX)
+
+#define STATIC_CALL_TRAMP(key) __PASTE(STATIC_CALL_TRAMP_PREFIX, key)
+#define STATIC_CALL_TRAMP_STR(key) __stringify(STATIC_CALL_TRAMP(key))
+
+#endif /* _STATIC_CALL_TYPES_H */



2019-06-06 22:48:28

by Nadav Amit

[permalink] [raw]
Subject: Re: [PATCH 10/15] static_call: Add basic static call infrastructure

> On Jun 5, 2019, at 6:08 AM, Peter Zijlstra <[email protected]> wrote:
>
> From: Josh Poimboeuf <[email protected]>
>
> Static calls are a replacement for global function pointers. They use
> code patching to allow direct calls to be used instead of indirect
> calls. They give the flexibility of function pointers, but with
> improved performance. This is especially important for cases where
> retpolines would otherwise be used, as retpolines can significantly
> impact performance.
>
> The concept and code are an extension of previous work done by Ard
> Biesheuvel and Steven Rostedt:
>
> https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Flkml.kernel.org%2Fr%2F20181005081333.15018-1-ard.biesheuvel%40linaro.org&amp;data=02%7C01%7Cnamit%40vmware.com%7C3f2ebbeff15e444d2fa008d6e9b9023f%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C636953378182765229&amp;sdata=WHceTWVt%2BNu1RFBv8jHp2Tw7VZuI5HxvHt%2FrWnjAmm4%3D&amp;reserved=0
> https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Flkml.kernel.org%2Fr%2F20181006015110.653946300%40goodmis.org&amp;data=02%7C01%7Cnamit%40vmware.com%7C3f2ebbeff15e444d2fa008d6e9b9023f%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C636953378182765229&amp;sdata=12JcrUsOh7%2FjRwEV9ANHw5SA2A6D4qNJ6z3h5aMHpnE%3D&amp;reserved=0
>
> There are two implementations, depending on arch support:
>
> 1) out-of-line: patched trampolines (CONFIG_HAVE_STATIC_CALL)
> 2) basic function pointers
>
> For more details, see the comments in include/linux/static_call.h.
>
> Cc: [email protected]
> Cc: Steven Rostedt <[email protected]>
> Cc: Julia Cartwright <[email protected]>
> Cc: Ingo Molnar <[email protected]>
> Cc: Ard Biesheuvel <[email protected]>
> Cc: Jason Baron <[email protected]>
> Cc: Rasmus Villemoes <[email protected]>
> Cc: Daniel Bristot de Oliveira <[email protected]>
> Cc: Linus Torvalds <[email protected]>
> Cc: Jiri Kosina <[email protected]>
> Cc: Edward Cree <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> Cc: Masami Hiramatsu <[email protected]>
> Cc: Borislav Petkov <[email protected]>
> Cc: David Laight <[email protected]>
> Cc: Jessica Yu <[email protected]>
> Cc: Nadav Amit <[email protected]>
> Cc: Andy Lutomirski <[email protected]>
> Cc: "H. Peter Anvin" <[email protected]>
> Signed-off-by: Josh Poimboeuf <[email protected]>
> Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
> Link: https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Flkml.kernel.org%2Fr%2Fa01f733889ebf4bc447507ab8041a60378eaa89f.1547073843.git.jpoimboe%40redhat.com&amp;data=02%7C01%7Cnamit%40vmware.com%7C3f2ebbeff15e444d2fa008d6e9b9023f%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C636953378182765229&amp;sdata=n5wgu%2FxNZiG77ExBcoT2wo7ak9xqyfJH3H8SMyxZj38%3D&amp;reserved=0
> ---
> arch/Kconfig | 3
> include/linux/static_call.h | 135 ++++++++++++++++++++++++++++++++++++++
> include/linux/static_call_types.h | 13 +++
> 3 files changed, 151 insertions(+)
> create mode 100644 include/linux/static_call.h
> create mode 100644 include/linux/static_call_types.h
>
> --- a/arch/Kconfig
> +++ b/arch/Kconfig
> @@ -927,6 +927,9 @@ config LOCK_EVENT_COUNTS
> the chance of application behavior change because of timing
> differences. The counts are reported via debugfs.
>
> +config HAVE_STATIC_CALL
> + bool
> +
> source "kernel/gcov/Kconfig"
>
> source "scripts/gcc-plugins/Kconfig"
> --- /dev/null
> +++ b/include/linux/static_call.h
> @@ -0,0 +1,135 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef _LINUX_STATIC_CALL_H
> +#define _LINUX_STATIC_CALL_H
> +
> +/*
> + * Static call support
> + *
> + * Static calls use code patching to hard-code function pointers into direct
> + * branch instructions. They give the flexibility of function pointers, but
> + * with improved performance. This is especially important for cases where
> + * retpolines would otherwise be used, as retpolines can significantly impact
> + * performance.
> + *
> + *
> + * API overview:
> + *
> + * DECLARE_STATIC_CALL(key, func);
> + * DEFINE_STATIC_CALL(key, func);
> + * static_call(key, args...);
> + * static_call_update(key, func);
> + *
> + *
> + * Usage example:
> + *
> + * # Start with the following functions (with identical prototypes):
> + * int func_a(int arg1, int arg2);
> + * int func_b(int arg1, int arg2);
> + *
> + * # Define a 'my_key' reference, associated with func_a() by default
> + * DEFINE_STATIC_CALL(my_key, func_a);
> + *
> + * # Call func_a()
> + * static_call(my_key, arg1, arg2);
> + *
> + * # Update 'my_key' to point to func_b()
> + * static_call_update(my_key, func_b);
> + *
> + * # Call func_b()
> + * static_call(my_key, arg1, arg2);

I think that this calling interface is not very intuitive. I understand that
the macros/objtool cannot allow the calling interface to be completely
transparent (as compiler plugin could). But, can the macros be used to
paste the key with the “static_call”? I think that having something like:

static_call__func(arg1, arg2)

Is more readable than

static_call(func, arg1, arg2)

> +}
> +
> +#define static_call_update(key, func) \
> +({ \
> + BUILD_BUG_ON(!__same_type(func, STATIC_CALL_TRAMP(key))); \
> + __static_call_update(&key, func); \
> +})

Is this safe against concurrent module removal?

2019-06-07 08:32:33

by Peter Zijlstra

[permalink] [raw]
Subject: Re: [PATCH 10/15] static_call: Add basic static call infrastructure

On Thu, Jun 06, 2019 at 10:44:23PM +0000, Nadav Amit wrote:
> > + * Usage example:
> > + *
> > + * # Start with the following functions (with identical prototypes):
> > + * int func_a(int arg1, int arg2);
> > + * int func_b(int arg1, int arg2);
> > + *
> > + * # Define a 'my_key' reference, associated with func_a() by default
> > + * DEFINE_STATIC_CALL(my_key, func_a);
> > + *
> > + * # Call func_a()
> > + * static_call(my_key, arg1, arg2);
> > + *
> > + * # Update 'my_key' to point to func_b()
> > + * static_call_update(my_key, func_b);
> > + *
> > + * # Call func_b()
> > + * static_call(my_key, arg1, arg2);
>
> I think that this calling interface is not very intuitive.

Yeah, it is somewhat unfortunate..

> I understand that
> the macros/objtool cannot allow the calling interface to be completely
> transparent (as compiler plugin could). But, can the macros be used to
> paste the key with the “static_call”? I think that having something like:
>
> static_call__func(arg1, arg2)
>
> Is more readable than
>
> static_call(func, arg1, arg2)

Doesn't really make it much better for me; I think I'd prefer to switch
to the GCC plugin scheme over this. ISTR there being some propotypes
there, but I couldn't quickly locate them.

> > +}
> > +
> > +#define static_call_update(key, func) \
> > +({ \
> > + BUILD_BUG_ON(!__same_type(func, STATIC_CALL_TRAMP(key))); \
> > + __static_call_update(&key, func); \
> > +})
>
> Is this safe against concurrent module removal?

It is for CONFIG_MODULE=n :-)

2019-06-07 08:51:03

by Ard Biesheuvel

[permalink] [raw]
Subject: Re: [PATCH 10/15] static_call: Add basic static call infrastructure

On Fri, 7 Jun 2019 at 10:29, Peter Zijlstra <[email protected]> wrote:
>
> On Thu, Jun 06, 2019 at 10:44:23PM +0000, Nadav Amit wrote:
> > > + * Usage example:
> > > + *
> > > + * # Start with the following functions (with identical prototypes):
> > > + * int func_a(int arg1, int arg2);
> > > + * int func_b(int arg1, int arg2);
> > > + *
> > > + * # Define a 'my_key' reference, associated with func_a() by default
> > > + * DEFINE_STATIC_CALL(my_key, func_a);
> > > + *
> > > + * # Call func_a()
> > > + * static_call(my_key, arg1, arg2);
> > > + *
> > > + * # Update 'my_key' to point to func_b()
> > > + * static_call_update(my_key, func_b);
> > > + *
> > > + * # Call func_b()
> > > + * static_call(my_key, arg1, arg2);
> >
> > I think that this calling interface is not very intuitive.
>
> Yeah, it is somewhat unfortunate..
>

Another thing I brought up at the time is that it would be useful to
have the ability to 'reset' a static call to its default target. E.g.,
for crypto modules that implement an accelerated version of a library
interface, removing the module should revert those call sites back to
the original target, without putting a disproportionate burden on the
module itself to implement the logic to support this.


> > I understand that
> > the macros/objtool cannot allow the calling interface to be completely
> > transparent (as compiler plugin could). But, can the macros be used to
> > paste the key with the “static_call”? I think that having something like:
> >
> > static_call__func(arg1, arg2)
> >
> > Is more readable than
> >
> > static_call(func, arg1, arg2)
>
> Doesn't really make it much better for me; I think I'd prefer to switch
> to the GCC plugin scheme over this. ISTR there being some propotypes
> there, but I couldn't quickly locate them.
>

I implemented the GCC plugin here

https://git.kernel.org/pub/scm/linux/kernel/git/ardb/linux.git/log/?h=static-calls

but IIRC, all it does is annotate call sites exactly how objtool does it.

> > > +}
> > > +
> > > +#define static_call_update(key, func) \
> > > +({ \
> > > + BUILD_BUG_ON(!__same_type(func, STATIC_CALL_TRAMP(key))); \
> > > + __static_call_update(&key, func); \
> > > +})
> >
> > Is this safe against concurrent module removal?
>
> It is for CONFIG_MODULE=n :-)

2019-06-07 16:35:26

by Andy Lutomirski

[permalink] [raw]
Subject: Re: [PATCH 10/15] static_call: Add basic static call infrastructure



> On Jun 7, 2019, at 1:49 AM, Ard Biesheuvel <[email protected]> wrote:
>
>> On Fri, 7 Jun 2019 at 10:29, Peter Zijlstra <[email protected]> wrote:
>>
>> On Thu, Jun 06, 2019 at 10:44:23PM +0000, Nadav Amit wrote:
>>>> + * Usage example:
>>>> + *
>>>> + * # Start with the following functions (with identical prototypes):
>>>> + * int func_a(int arg1, int arg2);
>>>> + * int func_b(int arg1, int arg2);
>>>> + *
>>>> + * # Define a 'my_key' reference, associated with func_a() by default
>>>> + * DEFINE_STATIC_CALL(my_key, func_a);
>>>> + *
>>>> + * # Call func_a()
>>>> + * static_call(my_key, arg1, arg2);
>>>> + *
>>>> + * # Update 'my_key' to point to func_b()
>>>> + * static_call_update(my_key, func_b);
>>>> + *
>>>> + * # Call func_b()
>>>> + * static_call(my_key, arg1, arg2);
>>>
>>> I think that this calling interface is not very intuitive.
>>
>> Yeah, it is somewhat unfortunate..
>>
>
> Another thing I brought up at the time is that it would be useful to
> have the ability to 'reset' a static call to its default target. E.g.,
> for crypto modules that implement an accelerated version of a library
> interface, removing the module should revert those call sites back to
> the original target, without putting a disproportionate burden on the
> module itself to implement the logic to support this.

I was thinking this could be a layer on top. We could have a way to register a static call with the module core so that, when a GPL module with an appropriate symbol is loaded, the static call gets replaced.

KVM could use this too. Or we could just require KVM to be built in some day.

2019-06-07 19:52:34

by Nadav Amit

[permalink] [raw]
Subject: Re: [PATCH 10/15] static_call: Add basic static call infrastructure

> On Jun 7, 2019, at 1:49 AM, Ard Biesheuvel <[email protected]> wrote:
>
> On Fri, 7 Jun 2019 at 10:29, Peter Zijlstra <[email protected]> wrote:
>> On Thu, Jun 06, 2019 at 10:44:23PM +0000, Nadav Amit wrote:
>>>> + * Usage example:
>>>> + *
>>>> + * # Start with the following functions (with identical prototypes):
>>>> + * int func_a(int arg1, int arg2);
>>>> + * int func_b(int arg1, int arg2);
>>>> + *
>>>> + * # Define a 'my_key' reference, associated with func_a() by default
>>>> + * DEFINE_STATIC_CALL(my_key, func_a);
>>>> + *
>>>> + * # Call func_a()
>>>> + * static_call(my_key, arg1, arg2);
>>>> + *
>>>> + * # Update 'my_key' to point to func_b()
>>>> + * static_call_update(my_key, func_b);
>>>> + *
>>>> + * # Call func_b()
>>>> + * static_call(my_key, arg1, arg2);
>>>
>>> I think that this calling interface is not very intuitive.
>>
>> Yeah, it is somewhat unfortunate..
>
> Another thing I brought up at the time is that it would be useful to
> have the ability to 'reset' a static call to its default target. E.g.,
> for crypto modules that implement an accelerated version of a library
> interface, removing the module should revert those call sites back to
> the original target, without putting a disproportionate burden on the
> module itself to implement the logic to support this.
>
>
>>> I understand that
>>> the macros/objtool cannot allow the calling interface to be completely
>>> transparent (as compiler plugin could). But, can the macros be used to
>>> paste the key with the “static_call”? I think that having something like:
>>>
>>> static_call__func(arg1, arg2)
>>>
>>> Is more readable than
>>>
>>> static_call(func, arg1, arg2)
>>
>> Doesn't really make it much better for me; I think I'd prefer to switch
>> to the GCC plugin scheme over this. ISTR there being some propotypes
>> there, but I couldn't quickly locate them.
>
> I implemented the GCC plugin here
>
> https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.kernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Fardb%2Flinux.git%2Flog%2F%3Fh%3Dstatic-calls&amp;data=02%7C01%7Cnamit%40vmware.com%7Cd31c4713640c44a651bf08d6eb250faa%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C636954941771964758&amp;sdata=h7RtT33E9FMapLZbAu9aTfjREP5kXrM0o2QQ1WpbDCM%3D&amp;reserved=0
>
> but IIRC, all it does is annotate call sites exactly how objtool does it.

I did not see your version before I made mine for a similar (but slightly
different) purpose:

https://lore.kernel.org/lkml/[email protected]/

My version, I think, is more generic (I don’t think yours consider calls
that have a return value). Anyhow, I am sure you know more about GCC plugins
than I do.

I do have a version that can take annotations to say which call should be
static and to get the symbol it uses.

I also think that this implementation would disallow keys that reside within
structs. This would mean that paravirt, for instance, would need to go
through many changes to use this infrastructure.

2019-10-02 13:55:58

by Peter Zijlstra

[permalink] [raw]
Subject: Re: [PATCH 10/15] static_call: Add basic static call infrastructure

On Fri, Jun 07, 2019 at 10:28:51AM +0200, Peter Zijlstra wrote:
> On Thu, Jun 06, 2019 at 10:44:23PM +0000, Nadav Amit wrote:
> > > + * Usage example:
> > > + *
> > > + * # Start with the following functions (with identical prototypes):
> > > + * int func_a(int arg1, int arg2);
> > > + * int func_b(int arg1, int arg2);
> > > + *
> > > + * # Define a 'my_key' reference, associated with func_a() by default
> > > + * DEFINE_STATIC_CALL(my_key, func_a);
> > > + *
> > > + * # Call func_a()
> > > + * static_call(my_key, arg1, arg2);
> > > + *
> > > + * # Update 'my_key' to point to func_b()
> > > + * static_call_update(my_key, func_b);
> > > + *
> > > + * # Call func_b()
> > > + * static_call(my_key, arg1, arg2);
> >
> > I think that this calling interface is not very intuitive.
>
> Yeah, it is somewhat unfortunate..
>
> > I understand that
> > the macros/objtool cannot allow the calling interface to be completely
> > transparent (as compiler plugin could). But, can the macros be used to
> > paste the key with the “static_call”? I think that having something like:
> >
> > static_call__func(arg1, arg2)
> >
> > Is more readable than
> >
> > static_call(func, arg1, arg2)
>
> Doesn't really make it much better for me; I think I'd prefer to switch
> to the GCC plugin scheme over this. ISTR there being some propotypes
> there, but I couldn't quickly locate them.

How about something like:

static_call(key)(arg1, arg2);

which is very close to the regular indirect call syntax. Furthermore,
how about we put the trampolines in .static_call.text instead of relying
on prefixes?

Also, I think I can shrink static_call_key by half:

- we can do away with static_call_key::tramp; there are only two usage
sites:

o __static_call_update, the static_call() macro can provide the
address of STATIC_CALL_TRAMP(key) directly

o static_call_add_module(), has two cases:

* the trampoline is from outside the module; in this case
it will already have been updated by a previous call to
__static_call_update.
* the trampoline is from inside the module; in this case
it will have the default value and it doesn't need an
update.

so in no case does static_call_add_module() need to modify a
trampoline.

- we can change static_call_key::site_mods into a single next pointer,
just like jump_label's static_key.

But so far all the schemes I've come up with require 'key' to be a name,
it cannot be an actual 'struct static_call_key *' value. And therefore
usage from within structures isn't allowed.


2019-10-02 21:54:04

by Josh Poimboeuf

[permalink] [raw]
Subject: Re: [PATCH 10/15] static_call: Add basic static call infrastructure

On Wed, Oct 02, 2019 at 03:54:17PM +0200, Peter Zijlstra wrote:
> On Fri, Jun 07, 2019 at 10:28:51AM +0200, Peter Zijlstra wrote:
> > On Thu, Jun 06, 2019 at 10:44:23PM +0000, Nadav Amit wrote:
> > > > + * Usage example:
> > > > + *
> > > > + * # Start with the following functions (with identical prototypes):
> > > > + * int func_a(int arg1, int arg2);
> > > > + * int func_b(int arg1, int arg2);
> > > > + *
> > > > + * # Define a 'my_key' reference, associated with func_a() by default
> > > > + * DEFINE_STATIC_CALL(my_key, func_a);
> > > > + *
> > > > + * # Call func_a()
> > > > + * static_call(my_key, arg1, arg2);
> > > > + *
> > > > + * # Update 'my_key' to point to func_b()
> > > > + * static_call_update(my_key, func_b);
> > > > + *
> > > > + * # Call func_b()
> > > > + * static_call(my_key, arg1, arg2);
> > >
> > > I think that this calling interface is not very intuitive.
> >
> > Yeah, it is somewhat unfortunate..
> >
> > > I understand that
> > > the macros/objtool cannot allow the calling interface to be completely
> > > transparent (as compiler plugin could). But, can the macros be used to
> > > paste the key with the “static_call”? I think that having something like:
> > >
> > > static_call__func(arg1, arg2)
> > >
> > > Is more readable than
> > >
> > > static_call(func, arg1, arg2)
> >
> > Doesn't really make it much better for me; I think I'd prefer to switch
> > to the GCC plugin scheme over this. ISTR there being some propotypes
> > there, but I couldn't quickly locate them.
>
> How about something like:
>
> static_call(key)(arg1, arg2);
>
> which is very close to the regular indirect call syntax.

Looks ok to me.

> Furthermore, how about we put the trampolines in .static_call.text
> instead of relying on prefixes?

Yeah, that would probably be better.

> Also, I think I can shrink static_call_key by half:
>
> - we can do away with static_call_key::tramp; there are only two usage
> sites:
>
> o __static_call_update, the static_call() macro can provide the
> address of STATIC_CALL_TRAMP(key) directly
>
> o static_call_add_module(), has two cases:
>
> * the trampoline is from outside the module; in this case
> it will already have been updated by a previous call to
> __static_call_update.
> * the trampoline is from inside the module; in this case
> it will have the default value and it doesn't need an
> update.
>
> so in no case does static_call_add_module() need to modify a
> trampoline.

Sounds plausible.

> - we can change static_call_key::site_mods into a single next pointer,
> just like jump_label's static_key.

Yep.

> But so far all the schemes I've come up with require 'key' to be a name,
> it cannot be an actual 'struct static_call_key *' value. And therefore
> usage from within structures isn't allowed.

Is that something we need? At least we were able to work around this
limitation with tracepoints' usage of static calls. But I could see how
it could be useful.

One way to solve that would be a completely different implementation:
have a global trampoline which detects the call site of the caller,
associates it with the given key, schedules some work to patch the call
site later, and then jumps to key->func. So the first call would
trigger the patching.

Then we might not even need objtool :-) But it might be tricky to pull
off.

--
Josh