[Xen-devel] [PATCH] xen: Fix a grant table resource leak

[Dominic Curran <dominic.curran@xxxxxxxxxx>]

From: Dominic Curran <dominic.curran@xxxxxxxxxx>
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@xxxxxxxxxx>
---
 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;



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel