All of lore.kernel.org
 help / color / mirror / Atom feed
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;

             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 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.