On Wed, May 06, 2020 at 12:24:55PM -0500, Josh Poimboeuf wrote:
> On that note, what do you think about tweaking the naming from
>
> DEFINE_STATIC_COND_CALL(name, typename);
> static_cond_call(name)(args...);
>
> to
>
> DEFINE_STATIC_CALL_NO_FUNC(name, typename);
> static_call_if_func(name)(args...);
>
> ?
>
> Seems clearer to me. They're still STATIC_CALLs, so it seems logical to
> keep those two words together. And NO_FUNC clarifies the initialized
> value.
>
> Similarly RETTRAMP could be ARCH_DEFINE_STATIC_CALL_NO_FUNC.
So I dislike static_call_if_func(), that's so much typing. Also, I
prefer ARCH_*_RETTRAMP as it clearly describes what the thing is.
How is something like this?
---
--- a/include/linux/static_call.h
+++ b/include/linux/static_call.h
@@ -16,7 +16,7 @@
*
* DECLARE_STATIC_CALL(name, func);
* DEFINE_STATIC_CALL(name, func);
- * DEFINE_STATIC_COND_CALL(name, typename);
+ * DEFINE_STATIC_CALL_NULL(name, typename);
* static_call(name)(args...);
* static_cond_call(name)(args...);
* static_call_update(name, func);
@@ -54,6 +54,43 @@
* rather than calling through the trampoline. This requires objtool or a
* compiler plugin to detect all the static_call() sites and annotate them
* in the .static_call_sites section.
+ *
+ *
+ * Notes on NULL function pointers:
+ *
+ * Static_call()s support NULL functions, with many of the caveats that
+ * regular function pointers have.
+ *
+ * Clearly calling a NULL function pointer is 'BAD', so too for
+ * static_call()s (although when HAVE_STATIC_CALL it might not be immediately
+ * fatal). A NULL static_call can be the result of:
+ *
+ * DECLARE_STATIC_CALL_NULL(my_static_call, void (*)(int));
+ *
+ * which is equivalent to declaring a NULL function pointer with just a
+ * typename:
+ *
+ * void (*my_func_ptr)(int arg1) = NULL;
+ *
+ * or using static_call_update() with a NULL function. In both cases the
+ * HAVE_STATIC_CALL implementation will patch the trampoline with a RET
+ * instruction, instead of an immediate tail-call JMP.
HAVE_STATIC_CALL_INLINE
+ * architectures can patch the trampoline call to a NOP.
+ *
+ * In all cases, any argument evaluation is unconditional. Unlike a regular
+ * conditional function pointer call:
+ *
+ * if (my_func_ptr)
+ * my_func_ptr(arg1)
+ *
+ * where the argument evaludation also depends on the pointer value.
+ *
+ * When calling a static_call that can be NULL, use:
+ *
+ * static_cond_call(name)(arg1);
+ *
+ * which will include the required value tests to avoid NULL-pointer
+ * dereferences.
*/
#include <linux/types.h>
@@ -122,7 +159,7 @@ extern int static_call_text_reserved(voi
}; \
ARCH_DEFINE_STATIC_CALL_TRAMP(name, _func)
-#define DEFINE_STATIC_COND_CALL(name, _func) \
+#define DEFINE_STATIC_CALL_NULL(name, _func) \
DECLARE_STATIC_CALL(name, _func); \
struct static_call_key STATIC_CALL_KEY(name) = { \
.func = NULL, \
@@ -154,7 +191,7 @@ struct static_call_key {
}; \
ARCH_DEFINE_STATIC_CALL_TRAMP(name, _func)
-#define DEFINE_STATIC_COND_CALL(name, _func) \
+#define DEFINE_STATIC_CALL_NULL(name, _func) \
DECLARE_STATIC_CALL(name, _func); \
struct static_call_key STATIC_CALL_KEY(name) = { \
.func = NULL, \
@@ -198,7 +235,7 @@ struct static_call_key {
.func = _func, \
}
-#define DEFINE_STATIC_COND_CALL(name, _func) \
+#define DEFINE_STATIC_CALL_NULL(name, _func) \
DECLARE_STATIC_CALL(name, _func); \
struct static_call_key STATIC_CALL_KEY(name) = { \
.func = NULL, \
