Re: [PATCH v2 0/4] Static calls

[Edward Cree]

On 12/12/18 18:14, Nadav Amit wrote:
> Second, (2i) is not very intuitive for me. Using the out-of-line static
> calls seems to me as less performant than the inline (potentially, I didnât
> check).
>
> Anyhow, the use of out-of-line static calls seems to me as
> counter-intuitive. I think (didnât measure) that it may add more overhead
> than it saves due to the additional call, ret, and so on
AIUI the outline version uses a tail-call (i.e. jmpq *target) rather than an
Âadditional call and ret. So I wouldn't expect it to be too expensive.
More to the point, it seems like it's easier to get right than the inline
Âversion, and if we get the inline version working later we can introduce it
Âwithout any API change, much as Josh's existing patches have both versions
Âbehind a Kconfig switch.

> I tried to avoid reading to
> compared target from memory and therefore used an immediate. This should
> prevent data cache misses and even when the data is available is faster by
> one cycle. But it requires the patching of both the âcmp %target-reg, immâ
> and âcall rel-targetâ to be patched âatomicallyâ. So the static-calls
> mechanism wouldnât be sufficient.
The approach I took to deal with that (since though I'm doing a read from
Âmemory, it's key->func in .data rather than the jmp immediate in .text) was
Âto have another static_call (though a plain static_key could also be used)
Âto 'skip' the fast-path while it's actually being patched. Then, since all
Âmy callers were under the rcu_read_lock, I just needed to synchronize_rcu()
Âafter switching off the fast-path to make sure no threads were still in it.
I'm not sure how that would be generalised to all cases, though; we don't
Âwant to force every indirect call to take the rcu_read_lock as that means
Âno callee can ever synchronize_rcu(). I guess we could have our own
Âseparate RCU read lock just for indirect call patching? (What does kgraft
Âdo?)

> Based on Joshâs previous feedback, I thought of improving the learning using
> some hysteresis. Anyhow, note that there are quite a few cases in which you
> wouldnât want optpolines. The question is whether in general it would be an
> opt-in or opt-out mechanism.
I was working on the assumption that it would be opt-in, wrapping a macro
Âaround indirect calls that are known to have a fairly small number of hot
Âtargets. There are plenty of indirect calls in the kernel that are only
Âcalled once in a blue moon, e.g. in control-plane operations like ethtool;
Âwe don't really need to bulk up .text with trampolines for all of them.

-Ed