[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v4 01/11] xen: Implement xen/alternative-call.h for use in common code
On 9/7/21 2:00 AM, Jan Beulich wrote: On 06.09.2021 18:22, Andrew Cooper wrote:On 06/09/2021 16:52, Jan Beulich wrote:On 03.09.2021 21:06, Daniel P. Smith wrote:--- /dev/null +++ b/xen/include/xen/alternative-call.h @@ -0,0 +1,63 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#ifndef XEN_ALTERNATIVE_CALL +#define XEN_ALTERNATIVE_CALL + +/* + * Some subsystems in Xen may have multiple implementions, which can be + * resolved to a single implementation at boot time. By default, this will + * result in the use of function pointers. + * + * Some architectures may have mechanisms for dynamically modifying .text. + * Using this mechnaism, function pointers can be converted to direct calls + * which are typically more efficient at runtime. + * + * For architectures to support: + * + * - Implement alternative_{,v}call() in asm/alternative.h. Code generation + * requirements are to emit a function pointer call at build time, and stash + * enough metadata to simplify the call at boot once the implementation has + * been resolved. + * - Select ALTERNATIVE_CALL in Kconfig. + * + * To use: + * + * Consider the following simplified example. + * + * 1) struct foo_ops __alt_call_maybe_initdata ops; + * + * 2) const struct foo_ops __initconst foo_a_ops = { ... }; + * const struct foo_ops __initconst foo_b_ops = { ... }; + * + * void foo_init(void) + * { + * ... + * if ( use_impl_a ) + * ops = *foo_a_ops; + * else if ( use_impl_b ) + * ops = *foo_b_ops; + * ... + * } + * + * 3) alternative_call(ops.bar, ...); + * + * There needs to a single ops object (1) which will eventually contain the + * function pointers. This should be populated in foo's init() function (2) + * by one of the available implementations. To call functions, use + * alternative_{,v}call() referencing the main ops object (3). + */ + +#ifdef CONFIG_ALTERNATIVE_CALL + +#include <asm/alternative.h> + +#define __alt_call_maybe_initdata __initdataMy v3 comment here was: "I think it wants (needs) clarifying that this may only be used if the ops object is used exclusively in alternative_{,v}call() instances (besides the original assignments to it, of course)." I realize this was slightly too strict, as other uses from .init.* are of course also okay, but I continue to think that - in particular with the example using it - there should be a warning about this possible pitfall. Or am I merely unable to spot the wording change somewhere in the comment?Such a comment is utterly pointless. initdata has a well known meaning, and a comment warning about the effects of it is just teaching developers to suck eggs[1]Well, okay then - at least the definition of __alt_call_maybe_initdata isn't far away from the comment. (What I'm not convinced of is that people knowing __initdata's meaning necessarily need to correctly infer __alt_call_maybe_initdata's.) Two other observations about the comment though, which I'd like to be taken care of (perhaps while committing): - __initconst wants to become __initconstrel. - foo_init(), seeing that there are section annotations elsewhere, wants to be marked __init. Then Acked-by: Jan Beulich <jbeulich@xxxxxxxx> Daniel, you having made changes (even if just minor ones) imo requires you S-o-b on the patch alongside Andrew's. Ack, I realized after sending I didn't SoB it, my apologies on that. v/r, dps
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |