All of lore.kernel.org
 help / color / mirror / Atom feed
From: msalter@redhat.com (Mark Salter)
To: linux-arm-kernel@lists.infradead.org
Subject: memory leak in arm kvm
Date: Fri, 25 Jul 2014 15:00:37 -0400	[thread overview]
Message-ID: <1406314837.27055.25.camel@deneb.redhat.com> (raw)

I've been looking into a memory leak in the arm kvm code which has been
seen on arm64 platforms. It seems there is a small (2-4MB depending on
pagesize) leak when a guest is started and stopped. The leak is
happening in arch/arm/kvm/mmu.c when kvm_free_stage2_pgd() tries to
unmap everything and free the pgd allocation. The problem I see is that
unmap_range() does not remove all the page references to the pgd so when
free_pages() is called, the pages are not actually freed because of
page->count > 1. Looking further, the call to unmap_range() only ever
calls put_page() once for the pgd and then it bails out of the while
loop. This happens because of the arm64 kvm_p?d_addr_end() macros. They
are defined to the normal p?d_addr_end macros. On arm64 with 3-level
page tables, pud_addr_end() simply advances to the end. With 2-level
page tables, pud_addr_end() and pmd_addr_end() both advance to end. So
when the bottom of the while loop in unmap_range() uses those to advance
to next pmd or pud, the loop ends up exiting. I can get around this by
open coding the  kvm_p?d_addr_end macros thusly:

diff --git a/arch/arm64/include/asm/kvm_mmu.h b/arch/arm64/include/asm/kvm_mmu.h
index 7d29847..d7f77ff 100644
--- a/arch/arm64/include/asm/kvm_mmu.h
+++ b/arch/arm64/include/asm/kvm_mmu.h
@@ -122,8 +122,16 @@ static inline void kvm_set_s2pmd_writable(pmd_t *pmd)
 }
 
 #define kvm_pgd_addr_end(addr, end)	pgd_addr_end(addr, end)
-#define kvm_pud_addr_end(addr, end)	pud_addr_end(addr, end)
-#define kvm_pmd_addr_end(addr, end)	pmd_addr_end(addr, end)
+
+#define kvm_pud_addr_end(addr, end)					\
+({	unsigned long __boundary = ((addr) + PUD_SIZE) & PUD_MASK;	\
+	(__boundary - 1 < (end) - 1)? __boundary: (end);		\
+})
+
+#define kvm_pmd_addr_end(addr, end)					\
+({	unsigned long __boundary = ((addr) + PMD_SIZE) & PMD_MASK;	\
+	(__boundary - 1 < (end) - 1)? __boundary: (end);		\
+})
 
 struct kvm;
 
I'm not@all sure this is a correct/complete fix and I'm not really
familiar with the design of the arm kvm code, so I'll leave it to
others to decide that.

             reply	other threads:[~2014-07-25 19:00 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-07-25 19:00 Mark Salter [this message]
2014-07-30 12:58 ` memory leak in arm kvm Christoffer Dall

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=1406314837.27055.25.camel@deneb.redhat.com \
    --to=msalter@redhat.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /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.