On Fri, Oct 14, 2011 at 12:02:43PM -0700, Jeremy Fitzhardinge wrote: > 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 ok, as things are now, I don't think jump labels are well suited for replacing indirect calls. They could be used to have a single no-op that is replaced with a jmp to the proper direct call...but at that point you've taken an extra jump. That doesn't make sense to me. Jump labels are well suited as mentioned for if/else type control flow, while the indirect call table, at least to me, seems like a bit of a different use-case... Thanks, -Jason -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
