在 2022/11/30 6:42, Peter Xu 写道:
> Hi, Yong,
>
> On Mon, Nov 21, 2022 at 11:26:35AM -0500, huangy81@chinatelecom.cn wrote:
>> From: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
>>
>> When tested large vcpu size vm with dirtylimit feature, Qemu crashed
>> due to the assertion in kvm_dirty_ring_reap_one, which assert that
>> vcpu's kvm_dirty_gfns has been allocated and not NULL.
>>
>> Because dirty ring reaper thread races with Qemu main thread, reaper
>> may reap vcpu's dirty ring buffer when main thread doesn't complete
>> vcpu instantiation. So add the waiting logic in reaper thread and
>> start to reap until vcpu instantiation is completed.
>>
>> Signed-off-by: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
>> ---
>> accel/kvm/kvm-all.c | 36 ++++++++++++++++++++++++++++++++++++
>> 1 file changed, 36 insertions(+)
>>
>> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
>> index f99b0be..9457715 100644
>> --- a/accel/kvm/kvm-all.c
>> +++ b/accel/kvm/kvm-all.c
>> @@ -1401,6 +1401,35 @@ out:
>> kvm_slots_unlock();
>> }
>>
>> +/*
>> + * test if dirty ring has been initialized by checking if vcpu
>> + * has been initialized and gfns was allocated correspondlingly.
>> + * return true if dirty ring has been initialized, false otherwise.
>> + */
>> +static bool kvm_vcpu_dirty_ring_initialized(void)
>> +{
>> + CPUState *cpu;
>> + MachineState *ms = MACHINE(qdev_get_machine());
>> + int ncpus = ms->smp.cpus;
>> +
>> + /*
>> + * assume vcpu has not been initilaized if generation
>> + * id less than number of vcpu
>> + */
>> + if (ncpus > cpu_list_generation_id_get()) {
>> + return false;
>> + }
>> +
>> + CPU_FOREACH(cpu) {
>> + if (!cpu->kvm_dirty_gfns) {
>> + return false;
>> + }
>> + }
>> +
>> + return true;
>> +}
>> +
>> +
>> static void *kvm_dirty_ring_reaper_thread(void *data)
>> {
>> KVMState *s = data;
>> @@ -1410,6 +1439,13 @@ static void *kvm_dirty_ring_reaper_thread(void *data)
>>
>> trace_kvm_dirty_ring_reaper("init");
>>
>> +retry:
>> + /* don't allow reaping dirty ring if ring buffer hasn't been mapped */
>> + if (!kvm_vcpu_dirty_ring_initialized()) {
>> + sleep(1);
>
> The sleep here is probably not necessary. Could you instead have a look at
> the other much simpler patch?
Of course yes, this patch is much more graceful, i'll cherry pick it
next version.
Here:
>
> https://lore.kernel.org/qemu-devel/20220927154653.77296-1-peterx@redhat.com/
>
>> + goto retry;
>> + }
>> +
>> while (true) {
>> r->reaper_state = KVM_DIRTY_RING_REAPER_WAIT;
>> trace_kvm_dirty_ring_reaper("wait");
>> --
>> 1.8.3.1
>>
>>
>
--
Best regard
Hyman Huang(黄勇)