Messages in this thread |  | | | Date | Wed, 01 Feb 2012 20:00:38 +0900 | | From | Takuya Yoshikawa <> | | Subject | Re: [RFC][PATCH] srcu: Implement call_srcu() |
| |
(2012/02/01 19:49), Avi Kivity wrote: > On 02/01/2012 12:44 PM, Avi Kivity wrote: >> On 02/01/2012 12:22 PM, Peter Zijlstra wrote: >>> One of the things I was thinking of is adding a sequence counter in the >>> per-cpu data. Using that we could do something like: >>> >>> unsigned int seq1 = 0, seq2 = 0, count = 0; >>> int cpu, idx; >>> >>> idx = ACCESS_ONCE(sp->completions)& 1; >>> >>> for_each_possible_cpu(cpu) >>> seq1 += per_cpu(sp->per_cpu_ref, cpu)->seq; >>> >>> for_each_possible_cpu(cpu) >>> count += per_cpu(sp->per_cpu_ref, cpu)->c[idx]; >>> >>> for_each_possible_cpu(cpu) >>> seq2 += per_cpu(sp->per_cpu_ref, cpu)->seq; >>> >>> /* >>> * there's no active references and no activity, we pass >>> */ >>> if (seq1 == seq2&& count == 0) >>> return; >>> >>> synchronize_srcu_slow(); >>> >>> >>> This would add a fast-path which should catch the case Avi outlined >>> where we call sync_srcu() when there's no other SRCU activity. >> >> Sorry, I was inaccurate. In two of the cases indeed we don't expect >> guest activity, and we're okay with waiting a bit if there is guest >> activity - when we're altering the guest physical memory map. But the >> third case does have concurrent guest activity with >> synchronize_srcu_expedited() and we still need it fast - that's when >> userspace reads the dirty bitmap log of a running guest and replaces it >> with a new bitmap. >> >> There may be a way to convert it to call_srcu() though. Without >> synchronize_srcu_expedited(), kvm sees both the old and the new bitmaps, >> but that's fine, since the dirty bits will go *somewhere*, and we can >> pick them up later in call_srcu(). The only problem is if this is the >> very last call to kvm_vm_ioctl_get_dirty_log(), and the callback >> triggers after it returns - we end up with a bag of bits with not one to >> return them to. Maybe we can detect this conditions (all vcpus ought to >> be stopped), and do something like: >> >> >> if (all vcpus stopped) { >> /* no activity, this should be fast */ >> synchronize_srcu() >> /* collect and return bits */ >> } else { >> call_srcu(collect bits) >> } >> >> still a snag - we can't reliably detect that all vcpus are stopped, they >> may be just resting in userspace, and restart while synchronize_srcu() >> is running. >> >> Marcelo? >> > > Or something completely different - we can remove srcu from the equation > completely in this case. Use just one bitmap (so no
I am already testing various possibilities like this. For VGA, using clear_bit() (+ rmap write protect) works well!
> rcu_assign_pointer), and use atomic operations to copy and clear: > > word = bitmap[i] > put_user(word) > atomic_and(&bitmap[i], ~word) > >
This kind of this was really slow IIRC.
How about just doing:
take a spin_lock copy the entire (or some portions of) bitmap locally clear the bitmap unlock
write protect the dirty pages based on the copied dirty data
copy_to_user
I can show you some performance numbers, this weekend, if you like.
Takuya
|  |