[PATCH] xen/grant-table: Avoid m2p_override during mapping

[PATCH] xen/grant-table: Avoid m2p_override during mapping

[Zoltan Kiss]

The grant mapping API does m2p_override unnecessarily: only gntdev needs it,
for blkback and future netback patches it just cause a lock contention, as
those pages never go to userspace. Therefore this patch does the following:
- move the m2p_override part from grant_[un]map_refs to gntdev, where it is
  needed after mapping operations
- but move set_phys_to_machine from m2p_override to grant_[un]map_refs,
  because it is needed always
- update the function prototypes as kmap_ops are no longer needed

Signed-off-by: Zoltan Kiss <zoltan.kiss@citrix.com>
Suggested-by: David Vrabel <david.vrabel@citrix.com>
---
 arch/x86/xen/p2m.c                  |    4 --
 drivers/block/xen-blkback/blkback.c |   15 +++----
 drivers/xen/gntdev.c                |   61 ++++++++++++++++++++++++++--
 drivers/xen/grant-table.c           |   76 +++++++++++------------------------
 include/xen/grant_table.h           |    2 -
 5 files changed, 87 insertions(+), 71 deletions(-)

diff --git a/arch/x86/xen/p2m.c b/arch/x86/xen/p2m.c
index 2ae8699..3e78eb9 100644
--- a/arch/x86/xen/p2m.c
+++ b/arch/x86/xen/p2m.c
@@ -893,9 +893,6 @@ int m2p_add_override(unsigned long mfn, struct page *page,
 	set_page_private(page, mfn);
 	page->index = pfn_to_mfn(pfn);
 
-	if (unlikely(!set_phys_to_machine(pfn, FOREIGN_FRAME(mfn))))
-		return -ENOMEM;
-
 	if (kmap_op != NULL) {
 		if (!PageHighMem(page)) {
 			struct multicall_space mcs =
@@ -962,7 +959,6 @@ int m2p_remove_override(struct page *page,
 	WARN_ON(!PagePrivate(page));
 	ClearPagePrivate(page);
 
-	set_phys_to_machine(pfn, page->index);
 	if (kmap_op != NULL) {
 		if (!PageHighMem(page)) {
 			struct multicall_space mcs;
diff --git a/drivers/block/xen-blkback/blkback.c b/drivers/block/xen-blkback/blkback.c
index 6620b73..875025f 100644
--- a/drivers/block/xen-blkback/blkback.c
+++ b/drivers/block/xen-blkback/blkback.c
@@ -285,8 +285,7 @@ static void free_persistent_gnts(struct xen_blkif *blkif, struct rb_root *root,
 
 		if (++segs_to_unmap == BLKIF_MAX_SEGMENTS_PER_REQUEST ||
 			!rb_next(&persistent_gnt->node)) {
-			ret = gnttab_unmap_refs(unmap, NULL, pages,
-				segs_to_unmap);
+			ret = gnttab_unmap_refs(unmap, pages, segs_to_unmap);
 			BUG_ON(ret);
 			put_free_pages(blkif, pages, segs_to_unmap);
 			segs_to_unmap = 0;
@@ -321,8 +320,7 @@ static void unmap_purged_grants(struct work_struct *work)
 		pages[segs_to_unmap] = persistent_gnt->page;
 
 		if (++segs_to_unmap == BLKIF_MAX_SEGMENTS_PER_REQUEST) {
-			ret = gnttab_unmap_refs(unmap, NULL, pages,
-				segs_to_unmap);
+			ret = gnttab_unmap_refs(unmap, pages, segs_to_unmap);
 			BUG_ON(ret);
 			put_free_pages(blkif, pages, segs_to_unmap);
 			segs_to_unmap = 0;
@@ -330,7 +328,7 @@ static void unmap_purged_grants(struct work_struct *work)
 		kfree(persistent_gnt);
 	}
 	if (segs_to_unmap > 0) {
-		ret = gnttab_unmap_refs(unmap, NULL, pages, segs_to_unmap);
+		ret = gnttab_unmap_refs(unmap, pages, segs_to_unmap);
 		BUG_ON(ret);
 		put_free_pages(blkif, pages, segs_to_unmap);
 	}
@@ -670,15 +668,14 @@ static void xen_blkbk_unmap(struct xen_blkif *blkif,
 				    GNTMAP_host_map, pages[i]->handle);
 		pages[i]->handle = BLKBACK_INVALID_HANDLE;
 		if (++invcount == BLKIF_MAX_SEGMENTS_PER_REQUEST) {
-			ret = gnttab_unmap_refs(unmap, NULL, unmap_pages,
-			                        invcount);
+			ret = gnttab_unmap_refs(unmap, unmap_pages, invcount);
 			BUG_ON(ret);
 			put_free_pages(blkif, unmap_pages, invcount);
 			invcount = 0;
 		}
 	}
 	if (invcount) {
-		ret = gnttab_unmap_refs(unmap, NULL, unmap_pages, invcount);
+		ret = gnttab_unmap_refs(unmap, unmap_pages, invcount);
 		BUG_ON(ret);
 		put_free_pages(blkif, unmap_pages, invcount);
 	}
@@ -740,7 +737,7 @@ again:
 	}
 
 	if (segs_to_map) {
-		ret = gnttab_map_refs(map, NULL, pages_to_gnt, segs_to_map);
+		ret = gnttab_map_refs(map, pages_to_gnt, segs_to_map);
 		BUG_ON(ret);
 	}
 
diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c
index e41c79c..3a97342 100644
--- a/drivers/xen/gntdev.c
+++ b/drivers/xen/gntdev.c
@@ -250,6 +250,9 @@ static int find_grant_ptes(pte_t *pte, pgtable_t token,
 static int map_grant_pages(struct grant_map *map)
 {
 	int i, err = 0;
+	bool lazy = false;
+	pte_t *pte;
+	unsigned long mfn;
 
 	if (!use_ptemod) {
 		/* Note: it could already be mapped */
@@ -284,8 +287,37 @@ static int map_grant_pages(struct grant_map *map)
 	}
 
 	pr_debug("map %d+%d\n", map->index, map->count);
-	err = gnttab_map_refs(map->map_ops, use_ptemod ? map->kmap_ops : NULL,
-			map->pages, map->count);
+	err = gnttab_map_refs(map->map_ops, map->pages, map->count);
+	if (err)
+		return err;
+
+	if (!in_interrupt() && paravirt_get_lazy_mode() == PARAVIRT_LAZY_NONE) {
+		arch_enter_lazy_mmu_mode();
+		lazy = true;
+	}
+
+	for (i = 0; i < map->count; i++) {
+		/* Do not add to override if the map failed. */
+		if (map->map_ops[i].status)
+			continue;
+
+		if (map->map_ops[i].flags & GNTMAP_contains_pte) {
+			pte = (pte_t *) (mfn_to_virt(PFN_DOWN(map->map_ops[i].host_addr)) +
+				(map->map_ops[i].host_addr & ~PAGE_MASK));
+			mfn = pte_mfn(*pte);
+		} else {
+			mfn = PFN_DOWN(map->map_ops[i].dev_bus_addr);
+		}
+		err = m2p_add_override(mfn,
+				       map->pages[i],
+				       use_ptemod ? &map->kmap_ops[i] : NULL);
+		if (err)
+			break;
+	}
+
+	if (lazy)
+		arch_leave_lazy_mmu_mode();
+
 	if (err)
 		return err;
 
@@ -304,6 +336,7 @@ static int map_grant_pages(struct grant_map *map)
 static int __unmap_grant_pages(struct grant_map *map, int offset, int pages)
 {
 	int i, err = 0;
+	bool lazy = false;
 
 	if (map->notify.flags & UNMAP_NOTIFY_CLEAR_BYTE) {
 		int pgno = (map->notify.addr >> PAGE_SHIFT);
@@ -316,8 +349,28 @@ static int __unmap_grant_pages(struct grant_map *map, int offset, int pages)
 	}
 
 	err = gnttab_unmap_refs(map->unmap_ops + offset,
-			use_ptemod ? map->kmap_ops + offset : NULL, map->pages + offset,
-			pages);
+				map->pages + offset,
+				pages);
+	if (err)
+		return err;
+
+	if (!in_interrupt() && paravirt_get_lazy_mode() == PARAVIRT_LAZY_NONE) {
+		arch_enter_lazy_mmu_mode();
+		lazy = true;
+	}
+
+	for (i = 0; i < pages; i++) {
+		err = m2p_remove_override((map->pages + offset)[i],
+					  use_ptemod ?
+					  &(map->kmap_ops + offset)[i] :
+					  NULL);
+		if (err)
+			break;
+	}
+
+	if (lazy)
+		arch_leave_lazy_mmu_mode();
+
 	if (err)
 		return err;
 
diff --git a/drivers/xen/grant-table.c b/drivers/xen/grant-table.c
index aa846a4..59019f2 100644
--- a/drivers/xen/grant-table.c
+++ b/drivers/xen/grant-table.c
@@ -881,11 +881,9 @@ void gnttab_batch_copy(struct gnttab_copy *batch, unsigned count)
 EXPORT_SYMBOL_GPL(gnttab_batch_copy);
 
 int gnttab_map_refs(struct gnttab_map_grant_ref *map_ops,
-		    struct gnttab_map_grant_ref *kmap_ops,
 		    struct page **pages, unsigned int count)
 {
 	int i, ret;
-	bool lazy = false;
 	pte_t *pte;
 	unsigned long mfn;
 
@@ -904,49 +902,35 @@ int gnttab_map_refs(struct gnttab_map_grant_ref *map_ops,
 		for (i = 0; i < count; i++) {
 			if (map_ops[i].status)
 				continue;
-			set_phys_to_machine(map_ops[i].host_addr >> PAGE_SHIFT,
-					map_ops[i].dev_bus_addr >> PAGE_SHIFT);
+			if (unlikely(!set_phys_to_machine(map_ops[i].host_addr >> PAGE_SHIFT,
+							  map_ops[i].dev_bus_addr >> PAGE_SHIFT)))
+				return -ENOMEM;
 		}
-		return ret;
-	}
-
-	if (!in_interrupt() && paravirt_get_lazy_mode() == PARAVIRT_LAZY_NONE) {
-		arch_enter_lazy_mmu_mode();
-		lazy = true;
-	}
-
-	for (i = 0; i < count; i++) {
-		/* Do not add to override if the map failed. */
-		if (map_ops[i].status)
-			continue;
-
-		if (map_ops[i].flags & GNTMAP_contains_pte) {
-			pte = (pte_t *) (mfn_to_virt(PFN_DOWN(map_ops[i].host_addr)) +
-				(map_ops[i].host_addr & ~PAGE_MASK));
-			mfn = pte_mfn(*pte);
-		} else {
-			mfn = PFN_DOWN(map_ops[i].dev_bus_addr);
+	} else {
+		for (i = 0; i < count; i++) {
+			if (map_ops[i].status)
+				continue;
+			if (map_ops[i].flags & GNTMAP_contains_pte) {
+				pte = (pte_t *) (mfn_to_virt(PFN_DOWN(map_ops[i].host_addr)) +
+					(map_ops[i].host_addr & ~PAGE_MASK));
+				mfn = pte_mfn(*pte);
+			} else {
+				mfn = PFN_DOWN(map_ops[i].dev_bus_addr);
+			}
+			if (unlikely(!set_phys_to_machine(page_to_pfn(pages[i]),
+							  FOREIGN_FRAME(mfn))))
+				return -ENOMEM;
 		}
-		ret = m2p_add_override(mfn, pages[i], kmap_ops ?
-				       &kmap_ops[i] : NULL);
-		if (ret)
-			goto out;
 	}
 
- out:
-	if (lazy)
-		arch_leave_lazy_mmu_mode();
-
-	return ret;
+	return 0;
 }
 EXPORT_SYMBOL_GPL(gnttab_map_refs);
 
 int gnttab_unmap_refs(struct gnttab_unmap_grant_ref *unmap_ops,
-		      struct gnttab_map_grant_ref *kmap_ops,
 		      struct page **pages, unsigned int count)
 {
 	int i, ret;
-	bool lazy = false;
 
 	ret = HYPERVISOR_grant_table_op(GNTTABOP_unmap_grant_ref, unmap_ops, count);
 	if (ret)
@@ -958,26 +942,14 @@ int gnttab_unmap_refs(struct gnttab_unmap_grant_ref *unmap_ops,
 			set_phys_to_machine(unmap_ops[i].host_addr >> PAGE_SHIFT,
 					INVALID_P2M_ENTRY);
 		}
-		return ret;
-	}
-
-	if (!in_interrupt() && paravirt_get_lazy_mode() == PARAVIRT_LAZY_NONE) {
-		arch_enter_lazy_mmu_mode();
-		lazy = true;
-	}
-
-	for (i = 0; i < count; i++) {
-		ret = m2p_remove_override(pages[i], kmap_ops ?
-				       &kmap_ops[i] : NULL);
-		if (ret)
-			goto out;
+	} else {
+		for (i = 0; i < count; i++) {
+				set_phys_to_machine(page_to_pfn(pages[i]),
+						    pages[i]->index);
+		}
 	}
 
- out:
-	if (lazy)
-		arch_leave_lazy_mmu_mode();
-
-	return ret;
+	return 0;
 }
 EXPORT_SYMBOL_GPL(gnttab_unmap_refs);
 
diff --git a/include/xen/grant_table.h b/include/xen/grant_table.h
index 694dcaf..93d363a 100644
--- a/include/xen/grant_table.h
+++ b/include/xen/grant_table.h
@@ -184,10 +184,8 @@ unsigned int gnttab_max_grant_frames(void);
 #define gnttab_map_vaddr(map) ((void *)(map.host_virt_addr))
 
 int gnttab_map_refs(struct gnttab_map_grant_ref *map_ops,
-		    struct gnttab_map_grant_ref *kmap_ops,
 		    struct page **pages, unsigned int count);
 int gnttab_unmap_refs(struct gnttab_unmap_grant_ref *unmap_ops,
-		      struct gnttab_map_grant_ref *kunmap_ops,
 		      struct page **pages, unsigned int count);
 
 /* Perform a batch of grant map/copy operations. Retry every batch slot

Re: [PATCH] xen/grant-table: Avoid m2p_override during mapping

[Roger Pau Monné]

On 13/01/14 00:15, Zoltan Kiss wrote:
> The grant mapping API does m2p_override unnecessarily: only gntdev needs it,
> for blkback and future netback patches it just cause a lock contention, as
> those pages never go to userspace. Therefore this patch does the following:
> - move the m2p_override part from grant_[un]map_refs to gntdev, where it is
>   needed after mapping operations
> - but move set_phys_to_machine from m2p_override to grant_[un]map_refs,
>   because it is needed always
> - update the function prototypes as kmap_ops are no longer needed
> 
> Signed-off-by: Zoltan Kiss <zoltan.kiss@citrix.com>
> Suggested-by: David Vrabel <david.vrabel@citrix.com>
> ---
>  arch/x86/xen/p2m.c                  |    4 --
>  drivers/block/xen-blkback/blkback.c |   15 +++----
>  drivers/xen/gntdev.c                |   61 ++++++++++++++++++++++++++--
>  drivers/xen/grant-table.c           |   76 +++++++++++------------------------
>  include/xen/grant_table.h           |    2 -
>  5 files changed, 87 insertions(+), 71 deletions(-)
> 
> diff --git a/arch/x86/xen/p2m.c b/arch/x86/xen/p2m.c
> index 2ae8699..3e78eb9 100644
> --- a/arch/x86/xen/p2m.c
> +++ b/arch/x86/xen/p2m.c
> @@ -893,9 +893,6 @@ int m2p_add_override(unsigned long mfn, struct page *page,
>  	set_page_private(page, mfn);
>  	page->index = pfn_to_mfn(pfn);
>  
> -	if (unlikely(!set_phys_to_machine(pfn, FOREIGN_FRAME(mfn))))
> -		return -ENOMEM;
> -
>  	if (kmap_op != NULL) {
>  		if (!PageHighMem(page)) {
>  			struct multicall_space mcs =
> @@ -962,7 +959,6 @@ int m2p_remove_override(struct page *page,
>  	WARN_ON(!PagePrivate(page));
>  	ClearPagePrivate(page);
>  
> -	set_phys_to_machine(pfn, page->index);
>  	if (kmap_op != NULL) {
>  		if (!PageHighMem(page)) {
>  			struct multicall_space mcs;
> diff --git a/drivers/block/xen-blkback/blkback.c b/drivers/block/xen-blkback/blkback.c
> index 6620b73..875025f 100644
> --- a/drivers/block/xen-blkback/blkback.c
> +++ b/drivers/block/xen-blkback/blkback.c
> @@ -285,8 +285,7 @@ static void free_persistent_gnts(struct xen_blkif *blkif, struct rb_root *root,
>  
>  		if (++segs_to_unmap == BLKIF_MAX_SEGMENTS_PER_REQUEST ||
>  			!rb_next(&persistent_gnt->node)) {
> -			ret = gnttab_unmap_refs(unmap, NULL, pages,
> -				segs_to_unmap);
> +			ret = gnttab_unmap_refs(unmap, pages, segs_to_unmap);
>  			BUG_ON(ret);
>  			put_free_pages(blkif, pages, segs_to_unmap);
>  			segs_to_unmap = 0;
> @@ -321,8 +320,7 @@ static void unmap_purged_grants(struct work_struct *work)
>  		pages[segs_to_unmap] = persistent_gnt->page;
>  
>  		if (++segs_to_unmap == BLKIF_MAX_SEGMENTS_PER_REQUEST) {
> -			ret = gnttab_unmap_refs(unmap, NULL, pages,
> -				segs_to_unmap);
> +			ret = gnttab_unmap_refs(unmap, pages, segs_to_unmap);
>  			BUG_ON(ret);
>  			put_free_pages(blkif, pages, segs_to_unmap);
>  			segs_to_unmap = 0;
> @@ -330,7 +328,7 @@ static void unmap_purged_grants(struct work_struct *work)
>  		kfree(persistent_gnt);
>  	}
>  	if (segs_to_unmap > 0) {
> -		ret = gnttab_unmap_refs(unmap, NULL, pages, segs_to_unmap);
> +		ret = gnttab_unmap_refs(unmap, pages, segs_to_unmap);
>  		BUG_ON(ret);
>  		put_free_pages(blkif, pages, segs_to_unmap);
>  	}
> @@ -670,15 +668,14 @@ static void xen_blkbk_unmap(struct xen_blkif *blkif,
>  				    GNTMAP_host_map, pages[i]->handle);
>  		pages[i]->handle = BLKBACK_INVALID_HANDLE;
>  		if (++invcount == BLKIF_MAX_SEGMENTS_PER_REQUEST) {
> -			ret = gnttab_unmap_refs(unmap, NULL, unmap_pages,
> -			                        invcount);
> +			ret = gnttab_unmap_refs(unmap, unmap_pages, invcount);
>  			BUG_ON(ret);
>  			put_free_pages(blkif, unmap_pages, invcount);
>  			invcount = 0;
>  		}
>  	}
>  	if (invcount) {
> -		ret = gnttab_unmap_refs(unmap, NULL, unmap_pages, invcount);
> +		ret = gnttab_unmap_refs(unmap, unmap_pages, invcount);
>  		BUG_ON(ret);
>  		put_free_pages(blkif, unmap_pages, invcount);
>  	}
> @@ -740,7 +737,7 @@ again:
>  	}
>  
>  	if (segs_to_map) {
> -		ret = gnttab_map_refs(map, NULL, pages_to_gnt, segs_to_map);
> +		ret = gnttab_map_refs(map, pages_to_gnt, segs_to_map);
>  		BUG_ON(ret);
>  	}
>  
> diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c
> index e41c79c..3a97342 100644
> --- a/drivers/xen/gntdev.c
> +++ b/drivers/xen/gntdev.c
> @@ -250,6 +250,9 @@ static int find_grant_ptes(pte_t *pte, pgtable_t token,
>  static int map_grant_pages(struct grant_map *map)
>  {
>  	int i, err = 0;
> +	bool lazy = false;
> +	pte_t *pte;
> +	unsigned long mfn;
>  
>  	if (!use_ptemod) {
>  		/* Note: it could already be mapped */
> @@ -284,8 +287,37 @@ static int map_grant_pages(struct grant_map *map)
>  	}
>  
>  	pr_debug("map %d+%d\n", map->index, map->count);
> -	err = gnttab_map_refs(map->map_ops, use_ptemod ? map->kmap_ops : NULL,
> -			map->pages, map->count);
> +	err = gnttab_map_refs(map->map_ops, map->pages, map->count);
> +	if (err)
> +		return err;
> +
> +	if (!in_interrupt() && paravirt_get_lazy_mode() == PARAVIRT_LAZY_NONE) {
> +		arch_enter_lazy_mmu_mode();
> +		lazy = true;
> +	}
> +
> +	for (i = 0; i < map->count; i++) {
> +		/* Do not add to override if the map failed. */
> +		if (map->map_ops[i].status)
> +			continue;
> +
> +		if (map->map_ops[i].flags & GNTMAP_contains_pte) {
> +			pte = (pte_t *) (mfn_to_virt(PFN_DOWN(map->map_ops[i].host_addr)) +
> +				(map->map_ops[i].host_addr & ~PAGE_MASK));
> +			mfn = pte_mfn(*pte);
> +		} else {
> +			mfn = PFN_DOWN(map->map_ops[i].dev_bus_addr);
> +		}
> +		err = m2p_add_override(mfn,
> +				       map->pages[i],
> +				       use_ptemod ? &map->kmap_ops[i] : NULL);
> +		if (err)
> +			break;
> +	}
> +
> +	if (lazy)
> +		arch_leave_lazy_mmu_mode();
> +
>  	if (err)
>  		return err;
>  
> @@ -304,6 +336,7 @@ static int map_grant_pages(struct grant_map *map)
>  static int __unmap_grant_pages(struct grant_map *map, int offset, int pages)
>  {
>  	int i, err = 0;
> +	bool lazy = false;
>  
>  	if (map->notify.flags & UNMAP_NOTIFY_CLEAR_BYTE) {
>  		int pgno = (map->notify.addr >> PAGE_SHIFT);
> @@ -316,8 +349,28 @@ static int __unmap_grant_pages(struct grant_map *map, int offset, int pages)
>  	}
>  
>  	err = gnttab_unmap_refs(map->unmap_ops + offset,
> -			use_ptemod ? map->kmap_ops + offset : NULL, map->pages + offset,
> -			pages);
> +				map->pages + offset,
> +				pages);
> +	if (err)
> +		return err;
> +
> +	if (!in_interrupt() && paravirt_get_lazy_mode() == PARAVIRT_LAZY_NONE) {
> +		arch_enter_lazy_mmu_mode();
> +		lazy = true;
> +	}
> +
> +	for (i = 0; i < pages; i++) {
> +		err = m2p_remove_override((map->pages + offset)[i],
> +					  use_ptemod ?
> +					  &(map->kmap_ops + offset)[i] :
> +					  NULL);
> +		if (err)
> +			break;
> +	}
> +
> +	if (lazy)
> +		arch_leave_lazy_mmu_mode();
> +
>  	if (err)
>  		return err;
>  
> diff --git a/drivers/xen/grant-table.c b/drivers/xen/grant-table.c
> index aa846a4..59019f2 100644
> --- a/drivers/xen/grant-table.c
> +++ b/drivers/xen/grant-table.c
> @@ -881,11 +881,9 @@ void gnttab_batch_copy(struct gnttab_copy *batch, unsigned count)
>  EXPORT_SYMBOL_GPL(gnttab_batch_copy);
>  
>  int gnttab_map_refs(struct gnttab_map_grant_ref *map_ops,
> -		    struct gnttab_map_grant_ref *kmap_ops,
>  		    struct page **pages, unsigned int count)
>  {
>  	int i, ret;
> -	bool lazy = false;
>  	pte_t *pte;
>  	unsigned long mfn;
>  
> @@ -904,49 +902,35 @@ int gnttab_map_refs(struct gnttab_map_grant_ref *map_ops,
>  		for (i = 0; i < count; i++) {
>  			if (map_ops[i].status)
>  				continue;
> -			set_phys_to_machine(map_ops[i].host_addr >> PAGE_SHIFT,
> -					map_ops[i].dev_bus_addr >> PAGE_SHIFT);
> +			if (unlikely(!set_phys_to_machine(map_ops[i].host_addr >> PAGE_SHIFT,
> +							  map_ops[i].dev_bus_addr >> PAGE_SHIFT)))
> +				return -ENOMEM;
>  		}
> -		return ret;
> -	}
> -
> -	if (!in_interrupt() && paravirt_get_lazy_mode() == PARAVIRT_LAZY_NONE) {
> -		arch_enter_lazy_mmu_mode();
> -		lazy = true;
> -	}
> -
> -	for (i = 0; i < count; i++) {
> -		/* Do not add to override if the map failed. */
> -		if (map_ops[i].status)
> -			continue;
> -
> -		if (map_ops[i].flags & GNTMAP_contains_pte) {
> -			pte = (pte_t *) (mfn_to_virt(PFN_DOWN(map_ops[i].host_addr)) +
> -				(map_ops[i].host_addr & ~PAGE_MASK));
> -			mfn = pte_mfn(*pte);
> -		} else {
> -			mfn = PFN_DOWN(map_ops[i].dev_bus_addr);
> +	} else {
> +		for (i = 0; i < count; i++) {
> +			if (map_ops[i].status)
> +				continue;
> +			if (map_ops[i].flags & GNTMAP_contains_pte) {
> +				pte = (pte_t *) (mfn_to_virt(PFN_DOWN(map_ops[i].host_addr)) +
> +					(map_ops[i].host_addr & ~PAGE_MASK));
> +				mfn = pte_mfn(*pte);
> +			} else {
> +				mfn = PFN_DOWN(map_ops[i].dev_bus_addr);
> +			}
> +			if (unlikely(!set_phys_to_machine(page_to_pfn(pages[i]),
> +							  FOREIGN_FRAME(mfn))))
> +				return -ENOMEM;
>  		}
> -		ret = m2p_add_override(mfn, pages[i], kmap_ops ?
> -				       &kmap_ops[i] : NULL);
> -		if (ret)
> -			goto out;
>  	}
>  
> - out:
> -	if (lazy)
> -		arch_leave_lazy_mmu_mode();
> -
> -	return ret;
> +	return 0;
>  }
>  EXPORT_SYMBOL_GPL(gnttab_map_refs);
>  
>  int gnttab_unmap_refs(struct gnttab_unmap_grant_ref *unmap_ops,
> -		      struct gnttab_map_grant_ref *kmap_ops,
>  		      struct page **pages, unsigned int count)
>  {
>  	int i, ret;
> -	bool lazy = false;
>  
>  	ret = HYPERVISOR_grant_table_op(GNTTABOP_unmap_grant_ref, unmap_ops, count);
>  	if (ret)
> @@ -958,26 +942,14 @@ int gnttab_unmap_refs(struct gnttab_unmap_grant_ref *unmap_ops,
>  			set_phys_to_machine(unmap_ops[i].host_addr >> PAGE_SHIFT,
>  					INVALID_P2M_ENTRY);
>  		}
> -		return ret;
> -	}
> -
> -	if (!in_interrupt() && paravirt_get_lazy_mode() == PARAVIRT_LAZY_NONE) {
> -		arch_enter_lazy_mmu_mode();
> -		lazy = true;
> -	}
> -
> -	for (i = 0; i < count; i++) {
> -		ret = m2p_remove_override(pages[i], kmap_ops ?
> -				       &kmap_ops[i] : NULL);
> -		if (ret)
> -			goto out;
> +	} else {
> +		for (i = 0; i < count; i++) {
> +				set_phys_to_machine(page_to_pfn(pages[i]),
> +						    pages[i]->index);

You seem to relay on page->index containing the old mfn, but I don't see
it being set on gnttab_map_refs (it's only set on m2p_add_override
AFAICT), maybe I'm missing something?

Roger.

Re: [PATCH] xen/grant-table: Avoid m2p_override during mapping

[Zoltan Kiss]

On 13/01/14 10:18, Roger Pau Monné wrote:
>> @@ -958,26 +942,14 @@ int gnttab_unmap_refs(struct gnttab_unmap_grant_ref *unmap_ops,
>>   			set_phys_to_machine(unmap_ops[i].host_addr >> PAGE_SHIFT,
>>   					INVALID_P2M_ENTRY);
>>   		}
>> -		return ret;
>> -	}
>> -
>> -	if (!in_interrupt() && paravirt_get_lazy_mode() == PARAVIRT_LAZY_NONE) {
>> -		arch_enter_lazy_mmu_mode();
>> -		lazy = true;
>> -	}
>> -
>> -	for (i = 0; i < count; i++) {
>> -		ret = m2p_remove_override(pages[i], kmap_ops ?
>> -				       &kmap_ops[i] : NULL);
>> -		if (ret)
>> -			goto out;
>> +	} else {
>> +		for (i = 0; i < count; i++) {
>> +				set_phys_to_machine(page_to_pfn(pages[i]),
>> +						    pages[i]->index);
>
> You seem to relay on page->index containing the old mfn, but I don't see
> it being set on gnttab_map_refs (it's only set on m2p_add_override
> AFAICT), maybe I'm missing something?
>
> Roger.
>

Indeed, I'll fix that, and I will also cut the function prototype update 
out into a separate patch for better readability.

Zoli