From: Dominic Curran <dominic.curran@citrix.com>
To: xen-devel@lists.xen.org, konrad.wilk@oracle.com
Subject: [PATCH] xen: Fix a grant table resource leak
Date: Fri, 11 May 2012 15:20:18 +0100 [thread overview]
Message-ID: <4FAD2022.6050009@citrix.com> (raw)
From: Dominic Curran <dominic.curran@citrix.com>
Date: Fri, 11 May 2012 12:24:42 +0100
Subject: [PATCH] xen: Fix a grant table resource leak
If the resource (my testing shows an object in TX/RX ring) still has a
reference held on it, then add to a list and kick-off a timer to try
and free later.
Signed-off-by: Dominic Curran <dominic.curran@citrix.com>
---
drivers/xen/grant-table.c | 72
+++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 68 insertions(+), 4 deletions(-)
Index: linux-2.6/drivers/xen/grant-table.c
===================================================================
--- linux-2.6.orig/drivers/xen/grant-table.c 2012-05-08
13:52:54.000000000 +0100
+++ linux-2.6/drivers/xen/grant-table.c 2012-05-08 16:53:52.000000000
+0100
@@ -33,6 +33,7 @@
#include <linux/module.h>
#include <linux/sched.h>
+#include <linux/timer.h>
#include <linux/mm.h>
#include <linux/slab.h>
#include <linux/vmalloc.h>
@@ -57,6 +58,7 @@
(grant_table_version == 1 ? \
(PAGE_SIZE / sizeof(struct grant_entry_v1)) : \
(PAGE_SIZE / sizeof(union grant_entry_v2)))
+#define GNTTAB_CLEANUP_TIMER 10
static grant_ref_t **gnttab_list;
static unsigned int nr_grant_frames;
@@ -66,6 +68,9 @@ static grant_ref_t gnttab_free_head;
static DEFINE_SPINLOCK(gnttab_list_lock);
unsigned long xen_hvm_resume_frames;
EXPORT_SYMBOL_GPL(xen_hvm_resume_frames);
+static LIST_HEAD(gnttab_used_ref_list);
+static DEFINE_SPINLOCK(gnttab_used_ref_lock);
+static struct timer_list gnttab_timer;
static union {
struct grant_entry_v1 *v1;
@@ -145,6 +150,16 @@ struct gnttab_ops {
domid_t trans_domid, grant_ref_t trans_gref);
};
+/* This structure is used to hold references to pages until they become
+ ready to free.
+*/
+struct gnttab_used_ref {
+ struct list_head list;
+ grant_ref_t ref;
+ unsigned long page;
+ int readonly;
+};
+
static struct gnttab_ops *gnttab_interface;
/*This reflects status of grant entries, so act as a global value*/
@@ -464,18 +479,62 @@ int gnttab_end_foreign_access_ref(grant_
}
EXPORT_SYMBOL_GPL(gnttab_end_foreign_access_ref);
+static void gnttab_attempt_clean_used_refs(unsigned long arg)
+{
+ struct gnttab_used_ref *u;
+ struct list_head *pos, *q;
+ unsigned long flags;
+
+ spin_lock_irqsave(&gnttab_used_ref_lock, flags);
+
+ list_for_each_safe(pos, q, &gnttab_used_ref_list) {
+ u = list_entry(pos, struct gnttab_used_ref, list);
+
+ if (gnttab_end_foreign_access_ref(u->ref, u->readonly)) {
+ list_del(pos);
+
+ put_free_entry(u->ref);
+ if (u->page != 0)
+ free_page(u->page);
+
+ kfree(u);
+ }
+ }
+ spin_unlock_irqrestore(&gnttab_used_ref_lock, flags);
+
+ if (!list_empty(&gnttab_used_ref_list))
+ mod_timer(&gnttab_timer, jiffies + HZ);
+}
+
void gnttab_end_foreign_access(grant_ref_t ref, int readonly,
unsigned long page)
{
+ struct gnttab_used_ref *u;
+ unsigned long flags;
+
if (gnttab_end_foreign_access_ref(ref, readonly)) {
put_free_entry(ref);
if (page != 0)
free_page(page);
} else {
- /* XXX This needs to be fixed so that the ref and page are
- placed on a list to be freed up later. */
- printk(KERN_WARNING
- "WARNING: leaking g.e. and page still in use!\n");
+ /* References that aren't cleaned up immediately sit on
+ * a list until they are abandoned & can be cleaned safely.
+ */
+ u = kmalloc(sizeof(struct gnttab_used_ref), GFP_KERNEL);
+ if (!u)
+ return;
+
+ u->page = page;
+ u->ref = ref;
+ u->readonly = readonly;
+
+ spin_lock_irqsave(&gnttab_used_ref_lock, flags);
+ list_add(&(u->list), &gnttab_used_ref_list);
+ spin_unlock_irqrestore(&gnttab_used_ref_lock, flags);
+
+ /* Kick off the cleanup timer. */
+ if (!timer_pending(&gnttab_timer))
+ add_timer(&gnttab_timer);
}
}
EXPORT_SYMBOL_GPL(gnttab_end_foreign_access);
@@ -1068,6 +1127,11 @@ int gnttab_init(void)
gnttab_free_count = nr_init_grefs - NR_RESERVED_ENTRIES;
gnttab_free_head = NR_RESERVED_ENTRIES;
+ init_timer(&gnttab_timer);
+ gnttab_timer.data = 0;
+ gnttab_timer.function = gnttab_attempt_clean_used_refs;
+ gnttab_timer.expires = jiffies +
msecs_to_jiffies(GNTTAB_CLEANUP_TIMER);
+
printk("Grant table initialized\n");
return 0;
next reply other threads:[~2012-05-11 14:20 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-05-11 14:20 Dominic Curran [this message]
2012-05-11 14:30 ` [PATCH] xen: Fix a grant table resource leak Jan Beulich
2012-05-11 14:30 ` Konrad Rzeszutek Wilk
2012-05-11 14:55 ` Dominic Curran
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=4FAD2022.6050009@citrix.com \
--to=dominic.curran@citrix.com \
--cc=konrad.wilk@oracle.com \
--cc=xen-devel@lists.xen.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;
as well as URLs for NNTP newsgroup(s).