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

[PATCH v2 0/2] 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 series 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 gnttab_[un]map_refs,
  because it is needed always
- update the function prototypes as kmap_ops are no longer needed

v2:
- move the storing of the old mfn in page->index to gnttab_map_refs
- move the function header update to a separate patch

Signed-off-by: Zoltan Kiss <zoltan.kiss@citrix.com>
Suggested-by: David Vrabel <david.vrabel@citrix.com>

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

[Zoltan Kiss]

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

Signed-off-by: Zoltan Kiss <zoltan.kiss@citrix.com>
Suggested-by: David Vrabel <david.vrabel@citrix.com>
---
 arch/x86/xen/p2m.c        |    5 ---
 drivers/xen/gntdev.c      |   62 ++++++++++++++++++++++++++++++++++---
 drivers/xen/grant-table.c |   75 +++++++++++++++------------------------------
 3 files changed, 83 insertions(+), 59 deletions(-)

diff --git a/arch/x86/xen/p2m.c b/arch/x86/xen/p2m.c
index 2ae8699..b1e9407 100644
--- a/arch/x86/xen/p2m.c
+++ b/arch/x86/xen/p2m.c
@@ -891,10 +891,6 @@ int m2p_add_override(unsigned long mfn, struct page *page,
 	WARN_ON(PagePrivate(page));
 	SetPagePrivate(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)) {
@@ -962,7 +958,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/xen/gntdev.c b/drivers/xen/gntdev.c
index e41c79c..b89aaa2 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, NULL, 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,29 @@ 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);
+				NULL,
+				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..ad281e4 100644
--- a/drivers/xen/grant-table.c
+++ b/drivers/xen/grant-table.c
@@ -885,7 +885,6 @@ int gnttab_map_refs(struct gnttab_map_grant_ref *map_ops,
 		    struct page **pages, unsigned int count)
 {
 	int i, ret;
-	bool lazy = false;
 	pte_t *pte;
 	unsigned long mfn;
 
@@ -904,40 +903,29 @@ 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);
+			}
+			pages[i]->index = pfn_to_mfn(page_to_pfn(pages[i]));
+			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);
 
@@ -946,7 +934,6 @@ int gnttab_unmap_refs(struct gnttab_unmap_grant_ref *unmap_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 +945,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);

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

[Zoltan Kiss]

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

Signed-off-by: Zoltan Kiss <zoltan.kiss@citrix.com>
Suggested-by: David Vrabel <david.vrabel@citrix.com>
---
 arch/x86/xen/p2m.c        |    5 ---
 drivers/xen/gntdev.c      |   62 ++++++++++++++++++++++++++++++++++---
 drivers/xen/grant-table.c |   75 +++++++++++++++------------------------------
 3 files changed, 83 insertions(+), 59 deletions(-)

diff --git a/arch/x86/xen/p2m.c b/arch/x86/xen/p2m.c
index 2ae8699..b1e9407 100644
--- a/arch/x86/xen/p2m.c
+++ b/arch/x86/xen/p2m.c
@@ -891,10 +891,6 @@ int m2p_add_override(unsigned long mfn, struct page *page,
 	WARN_ON(PagePrivate(page));
 	SetPagePrivate(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)) {
@@ -962,7 +958,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/xen/gntdev.c b/drivers/xen/gntdev.c
index e41c79c..b89aaa2 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, NULL, 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,29 @@ 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);
+				NULL,
+				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..ad281e4 100644
--- a/drivers/xen/grant-table.c
+++ b/drivers/xen/grant-table.c
@@ -885,7 +885,6 @@ int gnttab_map_refs(struct gnttab_map_grant_ref *map_ops,
 		    struct page **pages, unsigned int count)
 {
 	int i, ret;
-	bool lazy = false;
 	pte_t *pte;
 	unsigned long mfn;
 
@@ -904,40 +903,29 @@ 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);
+			}
+			pages[i]->index = pfn_to_mfn(page_to_pfn(pages[i]));
+			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);
 
@@ -946,7 +934,6 @@ int gnttab_unmap_refs(struct gnttab_unmap_grant_ref *unmap_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 +945,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);
 

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

[Stefano Stabellini]

On Mon, 13 Jan 2014, Zoltan Kiss wrote:
> This patch does the following:
> - move the m2p_override part from grant_[un]map_refs to gntdev, where it is
>   needed after mapping operations

As I wrote earlier, I am not against the idea of moving the m2p_override
calls in principle, but I would like to see either the calls being moved
to x86 specific areas or left in grant-table.c.

The reason is simple: from an architectural perspective if we wanted to
introduce an m2p on arm (actual we already have it but at the moment we
are not setting it up using m2p_override calls), we would want to do it
by calls from grant-table.c because unfortunately we need it for things
other than gntdev. In fact for devices that are not behind an IOMMU, we
do need to make sure that DMA operations involving granted pages are
handled correctly (by translating the pfns to mfns and vice versa). This
would have to happen for netback and blkback too.
So if we say that m2p_override functions are arch-independent, then they
need to stay where they are.

On the other hand, we if say that the m2p_override is an x86 specific
hack that is going away, then I don't want it in common code.
If you insist on calling it from common code, then please remove the
m2p_override stubs from arch/arm and ifdef x86 the whole thing in
gntdev.c.


> - but move set_phys_to_machine from m2p_override to grant_[un]map_refs,
>   because it is needed always
>
> Signed-off-by: Zoltan Kiss <zoltan.kiss@citrix.com>
> Suggested-by: David Vrabel <david.vrabel@citrix.com>
> ---
>  arch/x86/xen/p2m.c        |    5 ---
>  drivers/xen/gntdev.c      |   62 ++++++++++++++++++++++++++++++++++---
>  drivers/xen/grant-table.c |   75 +++++++++++++++------------------------------
>  3 files changed, 83 insertions(+), 59 deletions(-)
> 
> diff --git a/arch/x86/xen/p2m.c b/arch/x86/xen/p2m.c
> index 2ae8699..b1e9407 100644
> --- a/arch/x86/xen/p2m.c
> +++ b/arch/x86/xen/p2m.c
> @@ -891,10 +891,6 @@ int m2p_add_override(unsigned long mfn, struct page *page,
>  	WARN_ON(PagePrivate(page));
>  	SetPagePrivate(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)) {
> @@ -962,7 +958,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/xen/gntdev.c b/drivers/xen/gntdev.c
> index e41c79c..b89aaa2 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, NULL, 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,29 @@ 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);
> +				NULL,
> +				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..ad281e4 100644
> --- a/drivers/xen/grant-table.c
> +++ b/drivers/xen/grant-table.c
> @@ -885,7 +885,6 @@ int gnttab_map_refs(struct gnttab_map_grant_ref *map_ops,
>  		    struct page **pages, unsigned int count)
>  {
>  	int i, ret;
> -	bool lazy = false;
>  	pte_t *pte;
>  	unsigned long mfn;
>  
> @@ -904,40 +903,29 @@ 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);
> +			}
> +			pages[i]->index = pfn_to_mfn(page_to_pfn(pages[i]));
> +			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);
>  
> @@ -946,7 +934,6 @@ int gnttab_unmap_refs(struct gnttab_unmap_grant_ref *unmap_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 +945,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);
>  

Re: [Xen-devel] [PATCH v2 1/2] xen/grant-table: Avoid m2p_override during mapping

[Stefano Stabellini]

On Mon, 13 Jan 2014, Zoltan Kiss wrote:
> This patch does the following:
> - move the m2p_override part from grant_[un]map_refs to gntdev, where it is
>   needed after mapping operations

As I wrote earlier, I am not against the idea of moving the m2p_override
calls in principle, but I would like to see either the calls being moved
to x86 specific areas or left in grant-table.c.

The reason is simple: from an architectural perspective if we wanted to
introduce an m2p on arm (actual we already have it but at the moment we
are not setting it up using m2p_override calls), we would want to do it
by calls from grant-table.c because unfortunately we need it for things
other than gntdev. In fact for devices that are not behind an IOMMU, we
do need to make sure that DMA operations involving granted pages are
handled correctly (by translating the pfns to mfns and vice versa). This
would have to happen for netback and blkback too.
So if we say that m2p_override functions are arch-independent, then they
need to stay where they are.

On the other hand, we if say that the m2p_override is an x86 specific
hack that is going away, then I don't want it in common code.
If you insist on calling it from common code, then please remove the
m2p_override stubs from arch/arm and ifdef x86 the whole thing in
gntdev.c.


> - but move set_phys_to_machine from m2p_override to grant_[un]map_refs,
>   because it is needed always
>
> Signed-off-by: Zoltan Kiss <zoltan.kiss@citrix.com>
> Suggested-by: David Vrabel <david.vrabel@citrix.com>
> ---
>  arch/x86/xen/p2m.c        |    5 ---
>  drivers/xen/gntdev.c      |   62 ++++++++++++++++++++++++++++++++++---
>  drivers/xen/grant-table.c |   75 +++++++++++++++------------------------------
>  3 files changed, 83 insertions(+), 59 deletions(-)
> 
> diff --git a/arch/x86/xen/p2m.c b/arch/x86/xen/p2m.c
> index 2ae8699..b1e9407 100644
> --- a/arch/x86/xen/p2m.c
> +++ b/arch/x86/xen/p2m.c
> @@ -891,10 +891,6 @@ int m2p_add_override(unsigned long mfn, struct page *page,
>  	WARN_ON(PagePrivate(page));
>  	SetPagePrivate(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)) {
> @@ -962,7 +958,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/xen/gntdev.c b/drivers/xen/gntdev.c
> index e41c79c..b89aaa2 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, NULL, 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,29 @@ 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);
> +				NULL,
> +				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..ad281e4 100644
> --- a/drivers/xen/grant-table.c
> +++ b/drivers/xen/grant-table.c
> @@ -885,7 +885,6 @@ int gnttab_map_refs(struct gnttab_map_grant_ref *map_ops,
>  		    struct page **pages, unsigned int count)
>  {
>  	int i, ret;
> -	bool lazy = false;
>  	pte_t *pte;
>  	unsigned long mfn;
>  
> @@ -904,40 +903,29 @@ 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);
> +			}
> +			pages[i]->index = pfn_to_mfn(page_to_pfn(pages[i]));
> +			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);
>  
> @@ -946,7 +934,6 @@ int gnttab_unmap_refs(struct gnttab_unmap_grant_ref *unmap_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 +945,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);
>  

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

[Zoltan Kiss]

On 13/01/14 19:08, Zoltan Kiss wrote:
> @@ -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, NULL, 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;
>   
>
This patch has a fundamental problem here: we change the pfn in 
gnttab_map_refs, then fetch it in m2p_override again, but then we have a 
different one than we need. This causes Dom0 crash. I will send a new 
version to fix that.

Zoli

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

[Zoltan Kiss]

On 13/01/14 19:08, Zoltan Kiss wrote:
> @@ -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, NULL, 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;
>   
>
This patch has a fundamental problem here: we change the pfn in 
gnttab_map_refs, then fetch it in m2p_override again, but then we have a 
different one than we need. This causes Dom0 crash. I will send a new 
version to fix that.

Zoli

[PATCH v2 2/2] xen/grant-table: Remove kmap_ops

[Zoltan Kiss]

This patch updates the function prototypes as kmap_ops is no longer needed.

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

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 b89aaa2..3a97342 100644
--- a/drivers/xen/gntdev.c
+++ b/drivers/xen/gntdev.c
@@ -287,7 +287,7 @@ 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, NULL, map->pages, map->count);
+	err = gnttab_map_refs(map->map_ops, map->pages, map->count);
 	if (err)
 		return err;
 
@@ -349,7 +349,6 @@ static int __unmap_grant_pages(struct grant_map *map, int offset, int pages)
 	}
 
 	err = gnttab_unmap_refs(map->unmap_ops + offset,
-				NULL,
 				map->pages + offset,
 				pages);
 	if (err)
diff --git a/drivers/xen/grant-table.c b/drivers/xen/grant-table.c
index ad281e4..1471b5f 100644
--- a/drivers/xen/grant-table.c
+++ b/drivers/xen/grant-table.c
@@ -881,7 +881,6 @@ 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;
@@ -930,7 +929,6 @@ int gnttab_map_refs(struct gnttab_map_grant_ref *map_ops,
 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;
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

[PATCH v2 2/2] xen/grant-table: Remove kmap_ops

[Zoltan Kiss]

This patch updates the function prototypes as kmap_ops is no longer needed.

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

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 b89aaa2..3a97342 100644
--- a/drivers/xen/gntdev.c
+++ b/drivers/xen/gntdev.c
@@ -287,7 +287,7 @@ 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, NULL, map->pages, map->count);
+	err = gnttab_map_refs(map->map_ops, map->pages, map->count);
 	if (err)
 		return err;
 
@@ -349,7 +349,6 @@ static int __unmap_grant_pages(struct grant_map *map, int offset, int pages)
 	}
 
 	err = gnttab_unmap_refs(map->unmap_ops + offset,
-				NULL,
 				map->pages + offset,
 				pages);
 	if (err)
diff --git a/drivers/xen/grant-table.c b/drivers/xen/grant-table.c
index ad281e4..1471b5f 100644
--- a/drivers/xen/grant-table.c
+++ b/drivers/xen/grant-table.c
@@ -881,7 +881,6 @@ 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;
@@ -930,7 +929,6 @@ int gnttab_map_refs(struct gnttab_map_grant_ref *map_ops,
 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;
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