All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Hildenbrand <david@redhat.com>
To: Paolo Bonzini <pbonzini@redhat.com>, qemu-devel@nongnu.org
Cc: Richard Henderson <rth@twiddle.net>,
	"Dr . David Alan Gilbert" <dgilbert@redhat.com>,
	Eduardo Habkost <ehabkost@redhat.com>,
	Igor Mammedov <imammedo@redhat.com>, Peter Xu <peterx@redhat.com>,
	Marcel Apfelbaum <marcel.apfelbaum@gmail.com>,
	kvm@vger.kernel.org
Subject: Re: [PATCH RFC 4/4] kvm: Implement atomic memory region resizes via region_resize()
Date: Fri, 6 Mar 2020 15:30:55 +0100	[thread overview]
Message-ID: <d5704319-e9b8-be6b-6c95-d2e2edc6614c@redhat.com> (raw)
In-Reply-To: <2a8d8b63-d54f-c1e7-9668-5d065e36aa1d@redhat.com>

On 06.03.20 12:38, Paolo Bonzini wrote:
> On 06/03/20 11:20, David Hildenbrand wrote:
>> Yeah, rwlocks are not optimal and I am still looking for better
>> alternatives (suggestions welcome :) ). Using RCU might not work,
>> because the rcu_read region might be too big (esp. while in KVM_RUN).
>>
>> I had a prototype which used a bunch of atomics + qemu_cond_wait. But it
>> was quite elaborate and buggy.
>>
>> (I assume only going into KVM_RUN is really affected, and I do wonder if
>> it will be noticeable at all. Doing an ioctl is always already an
>> expensive operation.)
>>
>> I can look into per-cpu locks instead of the rwlock.
> 
> Assuming we're only talking about CPU ioctls (seems like a good
> approximation) maybe you could use start_exclusive/end_exclusive?  The
> current_cpu->in_exclusive_context assignments can be made conditional on
> "if (current_cpu)".
> 
> However that means you have to drop the BQL, see
> process_queued_cpu_work.  It may be a problem.
> 

Yeah, start_exclusive() is expected to be called without the BQL,
otherwise the other CPUs would not be able to make progress and can
eventually be "caught".

It's essentially the same reason why I can't use high-level
pause_all_vcpus()/resume_all_vcpus(). Will drop the BQL which is very
bad for resizing code.

-- 
Thanks,

David / dhildenb


WARNING: multiple messages have this Message-ID (diff)
From: David Hildenbrand <david@redhat.com>
To: Paolo Bonzini <pbonzini@redhat.com>, qemu-devel@nongnu.org
Cc: Eduardo Habkost <ehabkost@redhat.com>,
	kvm@vger.kernel.org,
	"Dr . David Alan Gilbert" <dgilbert@redhat.com>,
	Peter Xu <peterx@redhat.com>, Igor Mammedov <imammedo@redhat.com>,
	Richard Henderson <rth@twiddle.net>
Subject: Re: [PATCH RFC 4/4] kvm: Implement atomic memory region resizes via region_resize()
Date: Fri, 6 Mar 2020 15:30:55 +0100	[thread overview]
Message-ID: <d5704319-e9b8-be6b-6c95-d2e2edc6614c@redhat.com> (raw)
In-Reply-To: <2a8d8b63-d54f-c1e7-9668-5d065e36aa1d@redhat.com>

On 06.03.20 12:38, Paolo Bonzini wrote:
> On 06/03/20 11:20, David Hildenbrand wrote:
>> Yeah, rwlocks are not optimal and I am still looking for better
>> alternatives (suggestions welcome :) ). Using RCU might not work,
>> because the rcu_read region might be too big (esp. while in KVM_RUN).
>>
>> I had a prototype which used a bunch of atomics + qemu_cond_wait. But it
>> was quite elaborate and buggy.
>>
>> (I assume only going into KVM_RUN is really affected, and I do wonder if
>> it will be noticeable at all. Doing an ioctl is always already an
>> expensive operation.)
>>
>> I can look into per-cpu locks instead of the rwlock.
> 
> Assuming we're only talking about CPU ioctls (seems like a good
> approximation) maybe you could use start_exclusive/end_exclusive?  The
> current_cpu->in_exclusive_context assignments can be made conditional on
> "if (current_cpu)".
> 
> However that means you have to drop the BQL, see
> process_queued_cpu_work.  It may be a problem.
> 

Yeah, start_exclusive() is expected to be called without the BQL,
otherwise the other CPUs would not be able to make progress and can
eventually be "caught".

It's essentially the same reason why I can't use high-level
pause_all_vcpus()/resume_all_vcpus(). Will drop the BQL which is very
bad for resizing code.

-- 
Thanks,

David / dhildenb



  parent reply	other threads:[~2020-03-06 14:31 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-03 14:19 [PATCH RFC 0/4] kvm: Implement atomic memory region resizes David Hildenbrand
2020-03-03 14:19 ` [PATCH RFC 1/4] openpic_kvm: Use kvm_device_ioctl() instead of ioctl() David Hildenbrand
2020-03-03 14:19 ` [PATCH RFC 2/4] intc/s390_flic_kvm.c: " David Hildenbrand
2020-03-04  8:22   ` Christian Borntraeger
2020-03-04  8:30     ` David Hildenbrand
2020-03-03 14:19 ` [PATCH RFC 3/4] memory: Add region_resize() callback to memory notifier David Hildenbrand
2020-03-03 14:19 ` [PATCH RFC 4/4] kvm: Implement atomic memory region resizes via region_resize() David Hildenbrand
2020-03-03 14:19   ` David Hildenbrand
2020-03-06  9:50   ` Paolo Bonzini
2020-03-06  9:50     ` Paolo Bonzini
2020-03-06 10:20     ` David Hildenbrand
2020-03-06 10:20       ` David Hildenbrand
2020-03-06 11:38       ` Paolo Bonzini
2020-03-06 11:38         ` Paolo Bonzini
2020-03-06 12:18         ` David Hildenbrand
2020-03-06 12:18           ` David Hildenbrand
2020-03-06 14:30         ` David Hildenbrand [this message]
2020-03-06 14:30           ` David Hildenbrand
2020-03-06 14:39           ` Paolo Bonzini
2020-03-06 14:39             ` Paolo Bonzini
2020-03-06 14:44             ` David Hildenbrand
2020-03-06 14:44               ` David Hildenbrand

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=d5704319-e9b8-be6b-6c95-d2e2edc6614c@redhat.com \
    --to=david@redhat.com \
    --cc=dgilbert@redhat.com \
    --cc=ehabkost@redhat.com \
    --cc=imammedo@redhat.com \
    --cc=kvm@vger.kernel.org \
    --cc=marcel.apfelbaum@gmail.com \
    --cc=pbonzini@redhat.com \
    --cc=peterx@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=rth@twiddle.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.