On 08/28/2009 01:59 AM, Marcelo Tosatti wrote:
On Thu, Aug 27, 2009 at 07:27:48PM +0300, Avi Kivity wrote:
On 08/27/2009 06:54 PM, Marcelo Tosatti wrote:
perf report shows heavy overhead from down/up of slots_lock.
Attempted to remove slots_lock by having vcpus stop on a synchronization
point, but this introduced further complexity (a vcpu can be scheduled
out before reaching the synchronization point, and can sched back in at
points which are slots_lock protected, etc).
This patch changes vcpu_enter_guest to conditionally release/acquire
slots_lock in case a vcpu state bit is set.
vmexit performance improves by 5-10% on UP guest.
Sorry, it looks pretty complex.
Why?
There's a new locking protocol in there. I prefer sticking with the
existing kernel plumbing, or it gets more and more difficult knowing who
protects what and in what order you can do things.
Have you considered using srcu? It seems to me down/up_read() can
be replaced by srcu_read_lock/unlock(), and after proper conversion
of memslots and io_bus to rcu_assign_pointer(), we can just add
synchronize_srcu() immediately after changing stuff (of course
mmu_lock still needs to be held when updating slots).
I don't see RCU as being suitable because in certain operations you
want to stop writers (on behalf of vcpus), do something, and let them
continue afterwards. The dirty log, for example. Or any operation that
wants to modify lockless vcpu specific data.
kvm_flush_remote_tlbs() (which you'd call after mmu operations), will
get cpus out of guest mode, and synchronize_srcu() will wait for them to
drop the srcu "read lock". So it really happens naturally: do an RCU
update, send some request to all vcpus, synchronize_srcu(), done.
Also, synchronize_srcu() is limited to preemptible sections.
io_bus could use RCU, but I think being able to stop vcpus is also a
different requirement. Do you have any suggestion on how to do it in a
better way?
We don't need to stop vcpus, just kick them out of guest mode to let
them notice the new data. SRCU does that well.
--
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.
--
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
