qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: David Hildenbrand <david@redhat.com>
To: "Andrey Ryabinin" <arbn@yandex-team.com>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"Peter Xu" <peterx@redhat.com>,
	"Philippe Mathieu-Daudé" <f4bug@amsat.org>,
	"Stefan Hajnoczi" <stefanha@redhat.com>
Cc: qemu-devel@nongnu.org, yc-core@yandex-team.ru, qemu-stable@nongnu.org
Subject: Re: [PATCH 2/2] softmmu/physmem: fix dirty memory bitmap memleak
Date: Thu, 31 Mar 2022 10:37:39 +0200	[thread overview]
Message-ID: <3888d585-c090-24b4-3be9-7be3f03ddadb@redhat.com> (raw)
In-Reply-To: <20220325154013.16809-2-arbn@yandex-team.com>

On 25.03.22 16:40, Andrey Ryabinin wrote:
> The sequence of ram_block_add()/qemu_ram_free()/ram_block_add()
> function calls leads to leaking some memory.
> 
> ram_block_add() calls dirty_memory_extend() to allocate bitmap blocks
> for new memory. These blocks only grow but never shrink. So the
> qemu_ram_free() restores RAM size back to it's original stat but
> doesn't touch dirty memory bitmaps.
> 
> After qemu_ram_free() there is no way of knowing that we have
> allocated dirty memory bitmaps beyond current RAM size.
> So the next ram_block_add() will call dirty_memory_extend() again to
> to allocate new bitmaps and rewrite pointers to bitmaps left after
> the first ram_block_add()/dirty_memory_extend() calls.
> 
> Rework dirty_memory_extend() to be able to shrink dirty maps,
> also rename it to dirty_memory_resize(). And fix the leak by
> shrinking dirty memory maps on qemu_ram_free() if needed.
> 
> Fixes: 5b82b703b69a ("memory: RCU ram_list.dirty_memory[] for safe RAM hotplug")
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Andrey Ryabinin <arbn@yandex-team.com>

I looked at this a while ago and I think the problem is more involved,
because we might actually generate holes for which we can free the
bitmap. I think this patch impoves the situation, though.


IIRC if you hotplug two dimms and then hotunplug only the latter, the
bitmap for the first dimm will remain as long as the second dimm isn't
hotunplugged.


-- 
Thanks,

David / dhildenb



  parent reply	other threads:[~2022-03-31  8:41 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-25 15:40 [PATCH 1/2] softmmu/physmem: move last_ram_page() call under qemu_mutex_lock_ramlist() Andrey Ryabinin
2022-03-25 15:40 ` [PATCH 2/2] softmmu/physmem: fix dirty memory bitmap memleak Andrey Ryabinin
2022-03-30 19:25   ` Peter Xu
2022-03-31 16:14     ` Andrey Ryabinin
2022-03-31 17:34       ` Peter Xu
2022-03-31  8:37   ` David Hildenbrand [this message]
2022-03-31 12:27     ` Peter Xu
2022-03-31 16:26       ` David Hildenbrand
2022-03-31 17:29   ` Peter Xu
2022-03-30 18:47 ` [PATCH 1/2] softmmu/physmem: move last_ram_page() call under qemu_mutex_lock_ramlist() Peter Xu

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=3888d585-c090-24b4-3be9-7be3f03ddadb@redhat.com \
    --to=david@redhat.com \
    --cc=arbn@yandex-team.com \
    --cc=f4bug@amsat.org \
    --cc=pbonzini@redhat.com \
    --cc=peterx@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-stable@nongnu.org \
    --cc=stefanha@redhat.com \
    --cc=yc-core@yandex-team.ru \
    /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).