From: Keqian Zhu <zhukeqian1@huawei.com>
To: Stefan Hajnoczi <stefanha@redhat.com>
Cc: Peter Maydell <peter.maydell@linaro.org>,
Fam Zheng <famz@redhat.com>,
kuhn.chenqun@huawei.com, qemu-devel@nongnu.org,
"Dr . David Alan Gilbert" <dgilbert@redhat.com>,
qemu-arm@nongnu.org, wanghaibin.wang@huawei.com,
Paolo Bonzini <pbonzini@redhat.com>
Subject: Re: [PATCH v2 1/2] ramlist: Make dirty bitmap blocks of ramlist resizable
Date: Mon, 21 Dec 2020 15:37:36 +0800 [thread overview]
Message-ID: <7306ed10-871a-58ab-06d4-daa1efc5c9a7@huawei.com> (raw)
In-Reply-To: <20201217100501.GE4338@stefanha-x1.localdomain>
On 2020/12/17 18:05, Stefan Hajnoczi wrote:
> On Mon, Nov 30, 2020 at 09:11:03PM +0800, Keqian Zhu wrote:
>> @@ -1839,15 +1841,26 @@ static void dirty_memory_extend(ram_addr_t old_ram_size,
>> new_blocks = g_malloc(sizeof(*new_blocks) +
>> sizeof(new_blocks->blocks[0]) * new_num_blocks);
>>
>> - if (old_num_blocks) {
>> + if (cpy_num_blocks) {
>> memcpy(new_blocks->blocks, old_blocks->blocks,
>> - old_num_blocks * sizeof(old_blocks->blocks[0]));
>> + cpy_num_blocks * sizeof(old_blocks->blocks[0]));
>> }
>>
>> - for (j = old_num_blocks; j < new_num_blocks; j++) {
>> - new_blocks->blocks[j] = bitmap_new(DIRTY_MEMORY_BLOCK_SIZE);
>> + if (extend) {
>> + for (j = cpy_num_blocks; j < new_num_blocks; j++) {
>> + new_blocks->blocks[j] = bitmap_new(DIRTY_MEMORY_BLOCK_SIZE);
>> + }
>> + } else {
>> + for (j = cpy_num_blocks; j < old_num_blocks; j++) {
>> + /* We are safe to free it, for that it is out-of-use */
>> + g_free(old_blocks->blocks[j]);
>
> This looks unsafe because this code uses Read Copy Update (RCU):
>
> old_blocks = qatomic_rcu_read(&ram_list.dirty_memory[i]);
>
> Other threads may still be accessing old_blocks so we cannot modify it
> immediately. Changes need to be deferred until the next RCU period.
> g_free_rcu() needs to be used to do this.
>
Hi Stefan,
You are right. I was thinking about the VM life cycle before. We shrink the dirty_memory
when we are removing unused ramblock. However we can not rely on this.
I also notice that "Organization into blocks allows dirty memory to grow (but not shrink)
under RCU". Why "but not shrink"? Any thoughts?
[...]
* Organization into blocks allows dirty memory to grow (but not shrink) under
* RCU. When adding new RAMBlocks requires the dirty memory to grow, a new
* DirtyMemoryBlocks array is allocated with pointers to existing blocks kept
* the same. Other threads can safely access existing blocks while dirty
* memory is being grown. When no threads are using the old DirtyMemoryBlocks
* anymore it is freed by RCU (but the underlying blocks stay because they are
* pointed to from the new DirtyMemoryBlocks).
*/
#define DIRTY_MEMORY_BLOCK_SIZE ((ram_addr_t)256 * 1024 * 8)
typedef struct {
struct rcu_head rcu;
unsigned long *blocks[];
} DirtyMemoryBlocks;
[...]
Thanks,
Keqian
next prev parent reply other threads:[~2020-12-21 7:39 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-11-30 13:11 [PATCH v2 0/2] bugfix: Decrease dirty bitmap blocks after we remove ramblock Keqian Zhu
2020-11-30 13:11 ` [PATCH v2 1/2] ramlist: Make dirty bitmap blocks of ramlist resizable Keqian Zhu
2020-12-17 10:05 ` Stefan Hajnoczi
2020-12-21 7:37 ` Keqian Zhu [this message]
2020-12-26 7:11 ` Keqian Zhu
2021-03-08 10:52 ` Stefan Hajnoczi
2020-11-30 13:11 ` [PATCH v2 2/2] ramlist: Resize dirty bitmap blocks after remove ramblock Keqian Zhu
2020-12-03 14:02 ` Ping: [PATCH v2 0/2] bugfix: Decrease dirty bitmap blocks after we " zhukeqian
2020-12-17 10:09 ` Stefan Hajnoczi
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=7306ed10-871a-58ab-06d4-daa1efc5c9a7@huawei.com \
--to=zhukeqian1@huawei.com \
--cc=dgilbert@redhat.com \
--cc=famz@redhat.com \
--cc=kuhn.chenqun@huawei.com \
--cc=pbonzini@redhat.com \
--cc=peter.maydell@linaro.org \
--cc=qemu-arm@nongnu.org \
--cc=qemu-devel@nongnu.org \
--cc=stefanha@redhat.com \
--cc=wanghaibin.wang@huawei.com \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).