From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stefano Stabellini Subject: Re: [PATCHv4 09/14] xen/grant-table: add a mechanism to safely unmap pages that are in use Date: Mon, 26 Jan 2015 19:14:13 +0000 Message-ID: References: <1422291687-7398-1-git-send-email-david.vrabel@citrix.com> <1422291687-7398-10-git-send-email-david.vrabel@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta4.messagelabs.com ([85.158.143.247]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1YFp7L-0004Af-85 for xen-devel@lists.xenproject.org; Mon, 26 Jan 2015 19:14:35 +0000 In-Reply-To: <1422291687-7398-10-git-send-email-david.vrabel@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: David Vrabel Cc: xen-devel@lists.xenproject.org, Boris Ostrovsky , Jennifer Herbert List-Id: xen-devel@lists.xenproject.org On Mon, 26 Jan 2015, David Vrabel wrote: > From: Jennifer Herbert > > 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: Jennifer Herbert > Signed-off-by: David Vrabel > --- > 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 89dcca4..17972fb 100644 > --- a/drivers/xen/grant-table.c > +++ b/drivers/xen/grant-table.c > @@ -42,6 +42,7 @@ > #include > #include > #include > +#include > > #include > #include > @@ -819,6 +820,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); > +} > + > +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); I think that the +1 here is unnecessary. Regardess: Acked-by: Stefano Stabellini > + 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 > -- > 1.7.10.4 > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel >