On Tue, May 26, 2009 at 10:02:59AM +0200, Christian Ehrhardt wrote: > Marcelo Tosatti wrote: >> On Mon, May 25, 2009 at 01:40:49PM +0200, ehrhardt@xxxxxxxxxxxxxxxxxx wrote: >> >>> From: Christian Ehrhardt <ehrhardt@xxxxxxxxxxxxxxxxxx> >>> >>> To ensure vcpu's come out of guest context in certain cases this patch adds a >>> s390 specific way to kick them out of guest context. Currently it kicks them >>> out to rerun the vcpu_run path in the s390 code, but the mechanism itself is >>> expandable and with a new flag we could also add e.g. kicks to userspace etc. >>> >>> Signed-off-by: Christian Ehrhardt <ehrhardt@xxxxxxxxxxxxxxxxxx> >>> >> >> "For now I added the optimization to skip kicking vcpus out of guest >> that had the request bit already set to the s390 specific loop (sent as >> v2 in a few minutes). >> >> We might one day consider standardizing some generic kickout levels e.g. >> kick to "inner loop", "arch vcpu run", "generic vcpu run", "userspace", >> ... whatever levels fit *all* our use cases. And then let that kicks be >> implemented in an kvm_arch_* backend as it might be very different how >> they behave on different architectures." >> >> That would be ideal, yes. Two things make_all_requests handles: >> >> 1) It disables preemption with get_cpu(), so it can reliably check for >> cpu id. Somehow you don't need that for s390 when kicking multiple >> vcpus? >> > I don't even need the cpuid as make_all_requests does, I just insert a > special bit in the vcpu arch part and the vcpu will "come out to me > (host)". > Fortunateley the kick is rare and fast so I can just insert it > unconditionally (it's even ok to insert it if the vcpu is not in guest > state). That prevents us from needing vcpu lock or detailed checks which > would end up where we started (no guarantee that vcpu's come out of > guest context while trying to aquire all vcpu locks) Let me see if I get this right: you kick the vcpus out of guest mode by using a special bit in the vcpu arch part. OK. What I don't understand is this: "would end up where we started (no guarantee that vcpu's come out of guest context while trying to aquire all vcpu locks)" So you _need_ a mechanism to kick all vcpus out of guest mode? >> 2) It uses smp_call_function_many(wait=1), which guarantees that by the >> time make_all_requests returns no vcpus will be using stale data (the >> remote vcpus will have executed ack_flush). >> > yes this is really a part my s390 implementation doesn't fulfill yet. > Currently on return vcpus might still use the old memslot information. > As mentioned before letting all interrupts come "too far" out of the hot > loop would be a performance issue, therefore I think I will need some > request&confirm mechanism. I'm not sure yet but maybe it could be as > easy as this pseudo code example: > > # in make_all_requests > # remember we have slots_lock write here and the reentry that updates > the vcpu specific data aquires slots_lock for read. > loop vcpus > set_bit in vcpu requests > kick vcpu #arch function > endloop > > loop vcpus > until the requested bit is disappeared #as the reentry path uses > test_and_clear it will disappear > endloop > > That would be a implicit synchronization and should work, as I wrote > before setting memslots while the guest is running is rare if ever > existant for s390. On x86 smp_call_many could then work without the wait > flag being set. I see, yes. > But I assume that this synchronization approach is slower as it > serializes all vcpus on reentry (they wait for the slots_lock to get > dropped), therefore I wanted to ask how often setting memslots on > runtime will occur on x86 ? Would this approach be acceptable ? For x86 we need slots_lock for two things: 1) to protect the memslot structures from changing (very rare), ie: kvm_set_memory. 2) to protect updates to the dirty bitmap (operations on behalf of guest) which take slots_lock for read versus updates to that dirty bitmap (an ioctl that retrieves what pages have been dirtied in the memslots, and clears the dirtyness info). All you need for S390 is 1), AFAICS. For 1), we can drop the slots_lock usage, but instead create an explicit synchronization point, where all vcpus are forced to (say kvm_vcpu_block) "paused" state. qemu-kvm has such notion. Same language? > If it is too adventurous for now I can implement it that way in the s390 > code and we split the long term discussion (synchronization + generic > kickout levels + who knows what comes up). >> If smp_call_function_many is hidden behind kvm_arch_kick_vcpus, can you >> make use of make_all_requests for S390 (without the smp_call_function >> performance impact you mentioned) ? >> > In combination with the request&confirm mechanism desribed above it > should work if smp_call function and all the cpuid gathering which > belongs to it is hidden behind kvm_arch_kick_vcpus. >> For x86 we can further optimize make_all_requests by checking REQ_KICK, >> and kvm_arch_kick_vcpus would be a good place for that. >> >> And the kickout levels idea you mentioned can come later, as an >> optimization? > yes I agree splitting that to a later optimization is a good idea. > >> -- >> 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 >> > > > -- > > Grüsse / regards, Christian Ehrhardt > IBM Linux Technology Center, Open Virtualization -- 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
