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 rcu_assign_pointer), and use atomic operations to copy and clear: word = bitmap[i] put_user(word) atomic_and(&bitmap[i], ~word) -- error compiling committee.c: too many arguments to function -- 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