Re: [PATCH RFC V5 00/11] Paravirtualized ticketlocks

[Jeremy Fitzhardinge <jeremy@goop.org>]

On 10/14/2011 11:35 AM, Jason Baron wrote:
> On Fri, Oct 14, 2011 at 10:02:35AM -0700, Jeremy Fitzhardinge wrote:
>> On 10/14/2011 07:17 AM, Jason Baron wrote:
>>> On Thu, Oct 13, 2011 at 09:44:48AM -0700, Jeremy Fitzhardinge wrote:
>>>> pvops is basically a collection of ordinary _ops structures full of
>>>> function pointers, but it has a layer of patching to help optimise it. 
>>>> In the common case, this just replaces an indirect call with a direct
>>>> one, but in some special cases it can inline code.  This is used for
>>>> small, extremely performance-critical things like cli/sti, but it
>>>> awkward to use in general because you have to specify the inlined code
>>>> as a parameterless asm.
>>>>
>>> I haven't look at the pvops patching (probably should), but I was
>>> wondering if jump labels could be used for it? Or is there something
>>> that the pvops patching is doing that jump labels can't handle?
>> Jump labels are essentially binary: you can use path A or path B.  pvops
>> are multiway: there's no limit to the number of potential number of
>> paravirtualized hypervisor implementations.  At the moment we have 4:
>> native, Xen, KVM and lguest.
>>
> Yes, they are binary using the static_branch() interface. But in
> general, the asm goto() construct, allows branching to any number of
> labels. I have implemented the boolean static_branch() b/c it seems like
> the most common interface for jump labels, but I imagine we will
> introduce new interfaces as time goes on. You could of course nest
> static_branch() calls, although I can't say I've tried it.

At the moment we're using pvops to optimise things like:

	(*pv_mmu_ops.set_pte)(...);

To do that with some kind of multiway jump label thing, then that would
need to expand out to something akin to:

	if (static_branch(is_xen))
		xen_set_pte(...);
	else if (static_branch(is_kvm))
		kvm_set_pte(...);
	else if (static_branch(is_lguest))
		lguest_set_pte(...);
	else
		native_set_pte(...);

or something similar with an actual jump table.  But I don't see how it
offers much scope for improvement.

If there were something like:

	STATIC_INDIRECT_CALL(&pv_mmu_ops.set_pte)(...);

where the apparently indirect call is actually patched to be a direct
call, then that would offer a large subset of what we do with pvops.

However, to completely replace pvops patching, the static branch / jump
label mechanism would also need to work in assembler code, and be
capable of actually patching callsites with instructions rather than
just calls (sti/cli/pushf/popf being the most important).

We also keep track of the live registers at the callsite, and compare
that to what registers the target functions will clobber in order to
optimise the amount of register save/restore is needed.  And as a result
we have some pvops functions with non-standard calling conventions to
minimise save/restores on critical paths.

> We could have an interface, that allowed static branch(), to specifiy an
> arbitrary number of no-ops such that call-site itself could look anyway
> we want, if we don't know the bias at compile time. This, of course
> means potentially greater than 1 no-op in the fast path. I assume the
> pvops can have greater than 1 no-op in the fast path. Or is there a
> better solution here?

See above.  But pvops patching is pretty well tuned for its job.

However, I definitely think its worth investigating some way to reduce
the number of patching mechanisms, and if pvops patching doesn't stretch
static jumps in unnatural ways, then perhaps that's the way to go.

Thanks,
    J