|
From: | David Hildenbrand |
Subject: | Re: [PATCH v1 3/3] util/oslib-posix: Support concurrent os_mem_prealloc() invocation |
Date: | Tue, 20 Jul 2021 16:27:32 +0200 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.11.0 |
On 20.07.21 16:22, Daniel P. Berrangé wrote:
On Wed, Jul 14, 2021 at 01:23:06PM +0200, David Hildenbrand wrote:Add a mutext to protect the SIGBUS case, as we cannot mess concurrentlytypo s/mutext/mutex/with the sigbus handler and we have to manage the global variable sigbus_memset_context. The MADV_POPULATE_WRITE path can run concurrently. Note that page_mutex and page_cond are shared between concurrent invocations, which shouldn't be a problem. This is a preparation for future virtio-mem prealloc code, which will call os_mem_prealloc() asynchronously from an iothread when handling guest requests.Hmm, I'm wondering how the need to temporarily play with SIGBUS at runtime for mem preallocation will interact with the SIGBUS handler installed by softmmu/cpus.c.
That's exactly why I came up with MADV_POPULATE_WRITE, to avoid having to mess with different kinds of sigbus at the same time. You can only get it wrong.
The SIGBUS handler the preallocation code is installed just blindly assumes the SIGBUS is related to the preallocation work being done. This is a fine assumption during initially startup where we're single threaded and not running guest CPUs. I'm less clear on whether that's a valid assumption at runtime once guest CPUs are running.
I assume it's quite broken, for example, already when hotplugging a DIMM and prallocating memory for the memory backend.
If the sigbus_handler method in softmmu/cpus.c is doing something important for QEMU, then why is it ok for us to periodically disable that handler and replace it with something else that takes a completely different action ?
I don't think it is ok. I assume it has been broken for a long time, just nobody ever ran into that race.
Of course with the madvise impl we're bypassing the SIGBUS dance entirely. This is good for people with new kernels, but is this SIGBUS stuff safe for older kernels ?
It remains broken with old kernels I guess. There isn't too much that we can do: disabling prealloc=on once the VM is running breaks existing use cases.
Fortunately, running into that race seems to be rare, at least I never hear reports.
-- Thanks, David / dhildenb
[Prev in Thread] | Current Thread | [Next in Thread] |