[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH 09/14] xen/grant-table: add a mechanism to safely unmap pages that are in use



On 01/12/2015 10:43 AM, David Vrabel wrote:
From: Jenny Herbert <jennifer.herbert@xxxxxxxxxx>

Introduce gnttab_unmap_refs_async() that can be used to safely unmap
pages that may be in use (ref count > 1).  If the pages are in use the
unmap is deferred and retried later.  This polling is not very clever
but it should be good enough if the cases where the delay is necessary
are rare.

The initial delay is 5 ms and is increased linearly on each subsequent
retry (to reduce load if the page is in use for a long time).

This is needed to allow block backends using grant mapping to safely
use network storage (block or filesystem based such as iSCSI or NFS).

The network storage driver may complete a block request whilst there
is a queued network packet retry (because the ack from the remote end
races with deciding to queue the retry).  The pages for the retried
packet would be grant unmapped and the network driver (or hardware)
would access the unmapped page.

Signed-off-by: Jenny Herbert <jennifer.herbert@xxxxxxxxxx>
Signed-off-by: David Vrabel <david.vrabel@xxxxxxxxxx>
---
  drivers/xen/grant-table.c |   44 ++++++++++++++++++++++++++++++++++++++++++++
  include/xen/grant_table.h |   18 ++++++++++++++++++
  2 files changed, 62 insertions(+)

diff --git a/drivers/xen/grant-table.c b/drivers/xen/grant-table.c
index 9c7dc75..d9beffc 100644
--- a/drivers/xen/grant-table.c
+++ b/drivers/xen/grant-table.c
@@ -42,6 +42,7 @@
  #include <linux/io.h>
  #include <linux/delay.h>
  #include <linux/hardirq.h>
+#include <linux/workqueue.h>
#include <xen/xen.h>
  #include <xen/interface/xen.h>
@@ -817,6 +818,49 @@ int gnttab_unmap_refs(struct gnttab_unmap_grant_ref 
*unmap_ops,
  }
  EXPORT_SYMBOL_GPL(gnttab_unmap_refs);
+#define GNTTAB_UNMAP_REFS_DELAY 5
+
+static void __gnttab_unmap_refs_async(struct gntab_unmap_queue_data* item);
+
+static void gnttab_unmap_work(struct work_struct *work)
+{
+       struct gntab_unmap_queue_data
+               *unmap_data = container_of(work,
+                                          struct gntab_unmap_queue_data,
+                                          gnttab_work.work);
+       if (unmap_data->age != UINT_MAX)
+               unmap_data->age++;
+       __gnttab_unmap_refs_async(unmap_data);

Should there be a termination condition if pages are never (for some definition of "never") released? Return -ETIMEDOUT in done()? Or at least print a warning once?

-boris

+}
+
+static void __gnttab_unmap_refs_async(struct gntab_unmap_queue_data* item)
+{
+       int ret;
+       int pc;
+
+       for (pc = 0; pc < item->count; pc++) {
+               if (page_count(item->pages[pc]) > 1) {
+                       unsigned long delay = GNTTAB_UNMAP_REFS_DELAY * 
(item->age + 1);
+                       schedule_delayed_work(&item->gnttab_work,
+                                             msecs_to_jiffies(delay));
+                       return;
+               }
+       }
+
+       ret = gnttab_unmap_refs(item->unmap_ops, item->kunmap_ops,
+                               item->pages, item->count);
+       item->done(ret, item);
+}
+
+void gnttab_unmap_refs_async(struct gntab_unmap_queue_data* item)
+{
+       INIT_DELAYED_WORK(&item->gnttab_work, gnttab_unmap_work);
+       item->age = 0;
+
+       __gnttab_unmap_refs_async(item);
+}
+EXPORT_SYMBOL_GPL(gnttab_unmap_refs_async);
+
  static int gnttab_map_frames_v1(xen_pfn_t *frames, unsigned int nr_gframes)
  {
        int rc;
diff --git a/include/xen/grant_table.h b/include/xen/grant_table.h
index d3bef56..143ca5f 100644
--- a/include/xen/grant_table.h
+++ b/include/xen/grant_table.h
@@ -60,6 +60,22 @@ struct gnttab_free_callback {
        u16 count;
  };
+struct gntab_unmap_queue_data;
+
+typedef void (*gnttab_unmap_refs_done)(int result, struct 
gntab_unmap_queue_data *data);
+
+struct gntab_unmap_queue_data
+{
+       struct delayed_work     gnttab_work;
+       void *data;
+       gnttab_unmap_refs_done  done;
+       struct gnttab_unmap_grant_ref *unmap_ops;
+       struct gnttab_unmap_grant_ref *kunmap_ops;
+       struct page **pages;
+       unsigned int count;
+       unsigned int age;
+};
+
  int gnttab_init(void);
  int gnttab_suspend(void);
  int gnttab_resume(void);
@@ -174,6 +190,8 @@ int gnttab_map_refs(struct gnttab_map_grant_ref *map_ops,
  int gnttab_unmap_refs(struct gnttab_unmap_grant_ref *unmap_ops,
                      struct gnttab_unmap_grant_ref *kunmap_ops,
                      struct page **pages, unsigned int count);
+void gnttab_unmap_refs_async(struct gntab_unmap_queue_data* item);
+
/* Perform a batch of grant map/copy operations. Retry every batch slot
   * for which the hypervisor returns GNTST_eagain. This is typically due


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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.