From: Roger Pau Monne <roger.pau@citrix.com>
To: <xen-devel@lists.xenproject.org>, <linux-kernel@vger.kernel.org>
Cc: Roger Pau Monne <roger.pau@citrix.com>,
Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>,
David Vrabel <david.vrabel@citrix.com>,
Boris Ostrovsky <boris.ostrovsky@oracle.com>,
Matt Rushton <mrushton@amazon.com>, Matt Wilson <msw@amazon.com>,
Ian Campbell <Ian.Campbell@citrix.com>
Subject: [PATCH v2 2/4] xen-blkback: fix memory leaks
Date: Tue, 4 Feb 2014 11:26:13 +0100 [thread overview]
Message-ID: <1391509575-3949-3-git-send-email-roger.pau@citrix.com> (raw)
In-Reply-To: <1391509575-3949-1-git-send-email-roger.pau@citrix.com>
I've at least identified two possible memory leaks in blkback, both
related to the shutdown path of a VBD:
- blkback doesn't wait for any pending purge work to finish before
cleaning the list of free_pages. The purge work will call
put_free_pages and thus we might end up with pages being added to
the free_pages list after we have emptied it. Fix this by making
sure there's no pending purge work before exiting
xen_blkif_schedule, and moving the free_page cleanup code to
xen_blkif_free.
- blkback doesn't wait for pending requests to end before cleaning
persistent grants and the list of free_pages. Again this can add
pages to the free_pages list or persistent grants to the
persistent_gnts red-black tree. Fixed by moving the persistent
grants and free_pages cleanup code to xen_blkif_free.
Also, add some checks in xen_blkif_free to make sure we are cleaning
everything.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: David Vrabel <david.vrabel@citrix.com>
Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Cc: Matt Rushton <mrushton@amazon.com>
Cc: Matt Wilson <msw@amazon.com>
Cc: Ian Campbell <Ian.Campbell@citrix.com>
---
drivers/block/xen-blkback/blkback.c | 27 ++++++++++++++++++---------
drivers/block/xen-blkback/common.h | 1 +
drivers/block/xen-blkback/xenbus.c | 12 ++++++++++++
3 files changed, 31 insertions(+), 9 deletions(-)
diff --git a/drivers/block/xen-blkback/blkback.c b/drivers/block/xen-blkback/blkback.c
index 30ef7b3..dcfe49f 100644
--- a/drivers/block/xen-blkback/blkback.c
+++ b/drivers/block/xen-blkback/blkback.c
@@ -375,7 +375,7 @@ static void purge_persistent_gnt(struct xen_blkif *blkif)
pr_debug(DRV_PFX "Going to purge %u persistent grants\n", num_clean);
- INIT_LIST_HEAD(&blkif->persistent_purge_list);
+ BUG_ON(!list_empty(&blkif->persistent_purge_list));
root = &blkif->persistent_gnts;
purge_list:
foreach_grant_safe(persistent_gnt, n, root, node) {
@@ -625,6 +625,23 @@ purge_gnt_list:
print_stats(blkif);
}
+ /* Drain pending purge work */
+ flush_work(&blkif->persistent_purge_work);
+
+ if (log_stats)
+ print_stats(blkif);
+
+ blkif->xenblkd = NULL;
+ xen_blkif_put(blkif);
+
+ return 0;
+}
+
+/*
+ * Remove persistent grants and empty the pool of free pages
+ */
+void xen_blkbk_free_caches(struct xen_blkif *blkif)
+{
/* Free all persistent grant pages */
if (!RB_EMPTY_ROOT(&blkif->persistent_gnts))
free_persistent_gnts(blkif, &blkif->persistent_gnts,
@@ -635,14 +652,6 @@ purge_gnt_list:
/* Since we are shutting down remove all pages from the buffer */
shrink_free_pagepool(blkif, 0 /* All */);
-
- if (log_stats)
- print_stats(blkif);
-
- blkif->xenblkd = NULL;
- xen_blkif_put(blkif);
-
- return 0;
}
/*
diff --git a/drivers/block/xen-blkback/common.h b/drivers/block/xen-blkback/common.h
index 8d88075..f733d76 100644
--- a/drivers/block/xen-blkback/common.h
+++ b/drivers/block/xen-blkback/common.h
@@ -376,6 +376,7 @@ int xen_blkif_xenbus_init(void);
irqreturn_t xen_blkif_be_int(int irq, void *dev_id);
int xen_blkif_schedule(void *arg);
int xen_blkif_purge_persistent(void *arg);
+void xen_blkbk_free_caches(struct xen_blkif *blkif);
int xen_blkbk_flush_diskcache(struct xenbus_transaction xbt,
struct backend_info *be, int state);
diff --git a/drivers/block/xen-blkback/xenbus.c b/drivers/block/xen-blkback/xenbus.c
index c2014a0..8afef67 100644
--- a/drivers/block/xen-blkback/xenbus.c
+++ b/drivers/block/xen-blkback/xenbus.c
@@ -125,6 +125,7 @@ static struct xen_blkif *xen_blkif_alloc(domid_t domid)
blkif->persistent_gnts.rb_node = NULL;
spin_lock_init(&blkif->free_pages_lock);
INIT_LIST_HEAD(&blkif->free_pages);
+ INIT_LIST_HEAD(&blkif->persistent_purge_list);
blkif->free_pages_num = 0;
atomic_set(&blkif->persistent_gnt_in_use, 0);
@@ -259,6 +260,17 @@ static void xen_blkif_free(struct xen_blkif *blkif)
if (!atomic_dec_and_test(&blkif->refcnt))
BUG();
+ /* Remove all persistent grants and the cache of ballooned pages. */
+ xen_blkbk_free_caches(blkif);
+
+ /* Make sure everything is drained before shutting down */
+ BUG_ON(blkif->persistent_gnt_c != 0);
+ BUG_ON(atomic_read(&blkif->persistent_gnt_in_use) != 0);
+ BUG_ON(blkif->free_pages_num != 0);
+ BUG_ON(!list_empty(&blkif->persistent_purge_list));
+ BUG_ON(!list_empty(&blkif->free_pages));
+ BUG_ON(!RB_EMPTY_ROOT(&blkif->persistent_gnts));
+
/* Check that there is no request in use */
list_for_each_entry_safe(req, n, &blkif->pending_free, free_list) {
list_del(&req->free_list);
--
1.7.7.5 (Apple Git-26)
next prev parent reply other threads:[~2014-02-04 10:27 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-02-04 10:26 [PATCH v2 0/4] xen-blk: bug fixes Roger Pau Monne
2014-02-04 10:26 ` [PATCH v2 1/4] xen-blkback: fix memory leak when persistent grants are used Roger Pau Monne
2014-02-04 10:30 ` David Vrabel
2014-02-04 10:26 ` Roger Pau Monne [this message]
2014-02-04 10:32 ` [PATCH v2 2/4] xen-blkback: fix memory leaks David Vrabel
2014-02-04 10:26 ` [PATCH v2 3/4] xen-blkback: fix shutdown race Roger Pau Monne
2014-02-04 10:34 ` David Vrabel
2014-02-04 10:26 ` [PATCH v2 4/4] xen-blkif: drop struct blkif_request_segment_aligned Roger Pau Monne
2014-02-04 10:35 ` David Vrabel
2014-02-04 15:15 ` [Xen-devel] [PATCH v2 0/4] xen-blk: bug fixes Konrad Rzeszutek Wilk
2014-02-06 4:57 ` Matt Wilson
2014-02-06 16:20 ` Konrad Rzeszutek Wilk
2014-02-07 4:24 ` Matt Wilson
2014-02-07 8:08 ` Roger Pau Monné
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=1391509575-3949-3-git-send-email-roger.pau@citrix.com \
--to=roger.pau@citrix.com \
--cc=Ian.Campbell@citrix.com \
--cc=boris.ostrovsky@oracle.com \
--cc=david.vrabel@citrix.com \
--cc=konrad.wilk@oracle.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mrushton@amazon.com \
--cc=msw@amazon.com \
--cc=xen-devel@lists.xenproject.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®