thr3ads.net - Xen devel - [PATCH 0 of 2] Paging support updates for XCP dom0 [Dec 2011]

If this information is useful, please help other people find it:
Share via:

Andres Lagar-Cavilla

2011-Dec-01 20:51 UTC

[PATCH 0 of 2] Paging support updates for XCP dom0

This is a cherry pick of two patches that add support for guest paged out
frames in the XCP 2.6.32 dom0 patch queue.

First patch propagates the ENOENT returned by the hypervisor in the case
of a paged out page, all the way up the call chain to the MMAPBATCH_V2
ioctl. The ioctl is mainly used to harvest those return values and retry.

The second patch adds retry loops to all backend grant operations (map and
netback copy), in the case of a paged out frame.

Signed-off-by: Olaf Hering <olaf@aepfle.de>
Signed-off-by: Jan Beulich <jbeulich@novell.com>
Acked-by: Patrick Colp <pjcolp@cs.ubc.ca>
Acked-by: Andres Lagar-Cavilla <andres@lagarcavilla>
Ported and submitted by Andres Lagar-Cavilla

 arch/x86/mm/ioremap-xen.c                  |  12 ++----
 drivers/xen/blkback/blkback.c              |   6 ++-
 drivers/xen/blkback/interface.c            |   9 +++-
 drivers/xen/core/gnttab.c                  |   4 +-
 drivers/xen/gntdev/gntdev.c                |  49 +++++++++++++++++------------
 drivers/xen/netback/interface.c            |   5 +-
 drivers/xen/netback/netback.c              |  16 ++++++---
 drivers/xen/scsiback/interface.c           |  10 +++---
 drivers/xen/scsiback/scsiback.c            |   4 +-
 drivers/xen/tpmback/interface.c            |   7 +--
 drivers/xen/tpmback/tpmback.c              |  20 ++++-------
 drivers/xen/usbback/interface.c            |  16 ++++----
 drivers/xen/usbback/usbback.c              |   4 +-
 drivers/xen/xenbus/xenbus_backend_client.c |  10 +++---
 include/xen/gnttab.h                       |  37 ++++++++++++++++++++++
 15 files changed, 130 insertions(+), 79 deletions(-)

Andres Lagar-Cavilla

2011-Dec-01 20:51 UTC

head link

[PATCH 1 of 2] xen/x86: make __direct_remap_pfn_range()''s return value meaningful

arch/x86/mm/ioremap-xen.c |  12 ++++--------
 1 files changed, 4 insertions(+), 8 deletions(-)


This change fixes the xc_map_foreign_bulk interface, which would
otherwise cause SIGBUS when pages are gone because -ENOENT is not
returned as expected by the IOCTL_PRIVCMD_MMAPBATCH_V2 ioctl.

Signed-off-by: Jan Beulich <jbeulich@novell.com>

diff -r 164cbc688ec7 -r df9e25a0189b arch/x86/mm/ioremap-xen.c
--- a/arch/x86/mm/ioremap-xen.c
+++ b/arch/x86/mm/ioremap-xen.c
@@ -48,7 +48,7 @@ static int __direct_remap_pfn_range(stru
 				    pgprot_t prot,
 				    domid_t  domid)
 {
-	int rc;
+	int rc = 0;
 	unsigned long i, start_address;
 	mmu_update_t *u, *v, *w;
 
@@ -68,8 +68,8 @@ static int __direct_remap_pfn_range(stru
 						 direct_remap_area_pte_fn, &w);
 			if (rc)
 				goto out;
-			rc = -EFAULT;
-			if (HYPERVISOR_mmu_update(u, v - u, NULL, domid) < 0)
+			rc = HYPERVISOR_mmu_update(u, v - u, NULL, domid);
+			if (rc  < 0)
 				goto out;
 			v = w = u;
 			start_address = address;
@@ -94,13 +94,9 @@ static int __direct_remap_pfn_range(stru
 					 direct_remap_area_pte_fn, &w);
 		if (rc)
 			goto out;
-		rc = -EFAULT;
-		if (unlikely(HYPERVISOR_mmu_update(u, v - u, NULL, domid) < 0))
-			goto out;
+		rc = HYPERVISOR_mmu_update(u, v - u, NULL, domid);
 	}
 
-	rc = 0;
-
  out:
 	flush_tlb_all();

Andres Lagar-Cavilla

2011-Dec-01 20:51 UTC

head link

[PATCH 2 of 2] xenpaging: handle GNTST_eagain in kernel drivers

drivers/xen/blkback/blkback.c              |   6 ++-
 drivers/xen/blkback/interface.c            |   9 +++-
 drivers/xen/core/gnttab.c                  |   4 +-
 drivers/xen/gntdev/gntdev.c                |  49 +++++++++++++++++------------
 drivers/xen/netback/interface.c            |   5 +-
 drivers/xen/netback/netback.c              |  16 ++++++---
 drivers/xen/scsiback/interface.c           |  10 +++---
 drivers/xen/scsiback/scsiback.c            |   4 +-
 drivers/xen/tpmback/interface.c            |   7 +--
 drivers/xen/tpmback/tpmback.c              |  20 ++++-------
 drivers/xen/usbback/interface.c            |  16 ++++----
 drivers/xen/usbback/usbback.c              |   4 +-
 drivers/xen/xenbus/xenbus_backend_client.c |  10 +++---
 include/xen/gnttab.h                       |  37 ++++++++++++++++++++++
 14 files changed, 126 insertions(+), 71 deletions(-)


Handle GNTST_eagain status from GNTTABOP_map_grant_ref and
GNTTABOP_copy operations properly to allow usage of xenpaging without
causing crashes or data corruption.

Replace all relevant HYPERVISOR_grant_table_op() calls with a retry
loop. This loop is implemented as a macro to allow different
GNTTABOP_* args. It will sleep up to 33 seconds and wait for the
page to be paged in again.

All ->status checks were updated to use the GNTST_* namespace. All
return values are converted from GNTST_* namespace to 0/-EINVAL, since
all callers did not use the actual return value.

Signed-off-by: Olaf Hering <olaf@aepfle.de>
Acked-by: Patrick Colp <pjcolp@cs.ubc.ca>

This is a port from xenlinux 2.6.18 to the 2.6.32 xcp tree
Signed-off-by: Andres Lagar-Cavilla <andres@lagarcavilla.org>

diff -r df9e25a0189b -r 1170bc32db41 drivers/xen/blkback/blkback.c
--- a/drivers/xen/blkback/blkback.c
+++ b/drivers/xen/blkback/blkback.c
@@ -701,11 +701,13 @@ static void dispatch_rw_block_io(blkif_t
 	BUG_ON(ret);
 
 	for (i = 0; i < nseg; i++) {
-		if (unlikely(map[i].status != 0)) {
+		if (unlikely(map[i].status == GNTST_eagain))
+			gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref, &map[i]);
+		if (unlikely(map[i].status != GNTST_okay)) {
 			DPRINTK("grant map of dom %u gref %u failed: status %d\n",
 				blkif->domid, req->seg[i].gref, map[i].status);
 			map[i].handle = BLKBACK_INVALID_HANDLE;
-			ret |= 1;
+			ret = 1;
 			continue;
 		}
 
diff -r df9e25a0189b -r 1170bc32db41 drivers/xen/blkback/interface.c
--- a/drivers/xen/blkback/interface.c
+++ b/drivers/xen/blkback/interface.c
@@ -95,7 +95,7 @@ static int map_frontend_pages(blkif_t *b
 	struct vm_struct *area = blkif->blk_ring_area;
 	struct gnttab_map_grant_ref op[BLKIF_MAX_RING_PAGES];
 	unsigned int i;
-	int status = 0;
+	int status = GNTST_okay;
 
 	for (i = 0; i < nr_shared_pages; i++) {
 		unsigned long addr = (unsigned long)area->addr +
@@ -110,7 +110,10 @@ static int map_frontend_pages(blkif_t *b
 		BUG();
 
 	for (i = 0; i < nr_shared_pages; i++) {
-		if ((status = op[i].status) != 0) {
+		if (op[i].status == GNTST_eagain)
+			gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref, &op[i]);
+		if (op[i].status != GNTST_okay) {
+			status = op[i].status;
 			blkif->shmem_handle[i] = INVALID_GRANT_HANDLE;
 			continue;
 		}
@@ -120,7 +123,7 @@ static int map_frontend_pages(blkif_t *b
 
 	blkif->nr_shared_pages = nr_shared_pages;
 
-	if (status != 0) {
+	if (status != GNTST_okay) {
 		DPRINTK(" Grant table operation failure !\n");
 		unmap_frontend_pages(blkif);
 	}
diff -r df9e25a0189b -r 1170bc32db41 drivers/xen/core/gnttab.c
--- a/drivers/xen/core/gnttab.c
+++ b/drivers/xen/core/gnttab.c
@@ -773,7 +773,7 @@ static int gnttab_map(unsigned int start
 		return -ENOSYS;
 	}
 
-	BUG_ON(rc || setup.status);
+	BUG_ON(rc || (setup.status != GNTST_okay));
 
 	if (shared.raw == NULL)
 		shared.raw = arch_gnttab_alloc_shared(gframes);
@@ -901,7 +901,7 @@ int gnttab_copy_grant_page(grant_ref_t r
 	err = HYPERVISOR_grant_table_op(GNTTABOP_unmap_and_replace,
 					&unmap, 1);
 	BUG_ON(err);
-	BUG_ON(unmap.status);
+	BUG_ON(unmap.status != GNTST_okay);
 
 	write_sequnlock_irq(&gnttab_dma_lock);
 
diff -r df9e25a0189b -r 1170bc32db41 drivers/xen/gntdev/gntdev.c
--- a/drivers/xen/gntdev/gntdev.c
+++ b/drivers/xen/gntdev/gntdev.c
@@ -503,7 +503,7 @@ static int gntdev_mmap (struct file *fli
 	uint64_t ptep;
 	int ret;
 	int flags;
-	int i;
+	int i, exit_ret;
 	struct page *page;
 	gntdev_file_private_data_t *private_data = flip->private_data;
 
@@ -578,6 +578,7 @@ static int gntdev_mmap (struct file *fli
 	vma->vm_mm->context.has_foreign_mappings = 1;
 #endif
 
+	exit_ret = -ENOMEM;
 	for (i = 0; i < size; ++i) {
 
 		flags = GNTMAP_host_map;
@@ -598,14 +599,18 @@ static int gntdev_mmap (struct file *fli
 		ret = HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, 
 						&op, 1);
 		BUG_ON(ret);
-		if (op.status) {
-			printk(KERN_ERR "Error mapping the grant reference "
-			       "into the kernel (%d). domid = %d; ref = %d\n",
-			       op.status,
-			       private_data->grants[slot_index+i]
-			       .u.valid.domid,
-			       private_data->grants[slot_index+i]
-			       .u.valid.ref);
+		if (op.status != GNTST_okay) {
+			if (op.status != GNTST_eagain)
+				printk(KERN_ERR "Error mapping the grant reference "
+					   "into the kernel (%d). domid = %d; ref = %d\n",
+					   op.status,
+					   private_data->grants[slot_index+i]
+					   .u.valid.domid,
+					   private_data->grants[slot_index+i]
+					   .u.valid.ref);
+			else
+				/* Propagate instead of trying to fix it up */
+				exit_ret = -EAGAIN;
 			goto undo_map_out;
 		}
 
@@ -674,14 +679,17 @@ static int gntdev_mmap (struct file *fli
 			ret = HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref,
 							&op, 1);
 			BUG_ON(ret);
-			if (op.status) {
+			if (op.status != GNTST_okay) {
 				printk(KERN_ERR "Error mapping the grant "
-				       "reference into user space (%d). domid "
-				       "= %d; ref = %d\n", op.status,
-				       private_data->grants[slot_index+i].u
-				       .valid.domid,
-				       private_data->grants[slot_index+i].u
-				       .valid.ref);
+					   "reference into user space (%d). domid "
+					   "= %d; ref = %d\n", op.status,
+					   private_data->grants[slot_index+i].u
+					   .valid.domid,
+					   private_data->grants[slot_index+i].u
+					   .valid.ref);
+				/* GNTST_eagain (i.e. page paged out) sohuld never happen
+				 * once we''ve mapped into kernel space */
+				BUG_ON(op.status == GNTST_eagain);
 				goto undo_map_out;
 			}
 			
@@ -705,9 +713,10 @@ static int gntdev_mmap (struct file *fli
 		}
 
 	}
+	exit_ret = 0;
 
 	up_write(&private_data->grants_sem);
-	return 0;
+	return exit_ret;
 
 undo_map_out:
 	/* If we have a mapping failure, the unmapping will be taken care of
@@ -725,7 +734,7 @@ undo_map_out:
 	
 	up_write(&private_data->grants_sem);
 
-	return -ENOMEM;
+	return exit_ret;
 }
 
 static pte_t gntdev_clear_pte(struct vm_area_struct *vma, unsigned long addr,
@@ -777,7 +786,7 @@ static pte_t gntdev_clear_pte(struct vm_
 			ret = HYPERVISOR_grant_table_op(
 				GNTTABOP_unmap_grant_ref, &op, 1);
 			BUG_ON(ret);
-			if (op.status)
+			if (op.status != GNTST_okay)
 				printk("User unmap grant status = %d\n", 
 				       op.status);
 		} else {
@@ -794,7 +803,7 @@ static pte_t gntdev_clear_pte(struct vm_
 		ret = HYPERVISOR_grant_table_op(GNTTABOP_unmap_grant_ref, 
 						&op, 1);
 		BUG_ON(ret);
-		if (op.status)
+		if (op.status != GNTST_okay)
 			printk("Kernel unmap grant status = %d\n", op.status);
 
 
diff -r df9e25a0189b -r 1170bc32db41 drivers/xen/netback/interface.c
--- a/drivers/xen/netback/interface.c
+++ b/drivers/xen/netback/interface.c
@@ -381,10 +381,9 @@ static int map_frontend_pages(struct xen
 	gnttab_set_map_op(&op, (unsigned long)comms->ring_area->addr,
 			  GNTMAP_host_map, ring_ref, domid);
     
-	if (HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, &op, 1))
-		BUG();
+	gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref, &op);
 
-	if (op.status) {
+	if (op.status != GNTST_okay) {
 		DPRINTK(" Gnttab failure mapping ring_ref!\n");
 		free_vm_area(comms->ring_area);
 		return op.status;
diff -r df9e25a0189b -r 1170bc32db41 drivers/xen/netback/netback.c
--- a/drivers/xen/netback/netback.c
+++ b/drivers/xen/netback/netback.c
@@ -419,11 +419,13 @@ static int netbk_check_gop(int nr_copy_s
 
 	for (i = 0; i < nr_copy_slots; i++) {
 		copy_op = npo->copy + npo->copy_cons++;
+		if (copy_op->status == GNTST_eagain)
+			gnttab_check_GNTST_eagain_while(GNTTABOP_copy, copy_op);
 		if (copy_op->status != GNTST_okay) {
-				DPRINTK("Bad status %d from copy to DOM%d.\n",
-					copy_op->status, domid);
-				status = NETIF_RSP_ERROR;
-			}
+			DPRINTK("Bad status %d from copy to DOM%d.\n",
+				copy_op->status, domid);
+			status = NETIF_RSP_ERROR;
+		}
 	}
 
 	return status;
@@ -1020,7 +1022,11 @@ static int netbk_tx_check_mop(struct xen
 
 	/* Check status of header. */
 	err = mop->status;
-	if (unlikely(err)) {
+	if (err == GNTST_eagain) {
+		gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref, mop);
+		err = mop->status;
+	}
+	if (unlikely(err != GNTST_okay)) {
 		pending_ring_idx_t index;
 		index = pending_index(netbk->pending_prod++);
 		txp = &pending_tx_info[pending_idx].req;
diff -r df9e25a0189b -r 1170bc32db41 drivers/xen/scsiback/interface.c
--- a/drivers/xen/scsiback/interface.c
+++ b/drivers/xen/scsiback/interface.c
@@ -69,18 +69,18 @@ static int map_frontend_page( struct vsc
 				GNTMAP_host_map, ring_ref,
 				info->domid);
 
-	err = HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, &op, 1);
-	BUG_ON(err);
+	gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref, &op);
 
-	if (op.status) {
-		printk(KERN_ERR "scsiback: Grant table operation failure !\n");
+	if (op.status != GNTST_okay) {
+		printk(KERN_ERR "scsiback: Grant table operation failure %d!\n", 
+							(int) op.status);
 		return op.status;
 	}
 
 	info->shmem_ref    = ring_ref;
 	info->shmem_handle = op.handle;
 
-	return (GNTST_okay);
+	return 0;
 }
 
 static void unmap_frontend_page(struct vscsibk_info *info)
diff -r df9e25a0189b -r 1170bc32db41 drivers/xen/scsiback/scsiback.c
--- a/drivers/xen/scsiback/scsiback.c
+++ b/drivers/xen/scsiback/scsiback.c
@@ -287,7 +287,9 @@ static int scsiback_gnttab_data_map(vscs
 		for_each_sg (pending_req->sgl, sg, nr_segments, i) {
 			struct page *pg;
 
-			if (unlikely(map[i].status != 0)) {
+			if (unlikely(map[i].status == GNTST_eagain))
+				gnttab_check_GNTST_eagain_while(GNTTABOP_map_grant_ref, &map[i]);
+			if (unlikely(map[i].status != GNTST_okay)) {
 				printk(KERN_ERR "scsiback: invalid buffer -- could not remap
it\n");
 				map[i].handle = SCSIBACK_INVALID_HANDLE;
 				err |= 1;
diff -r df9e25a0189b -r 1170bc32db41 drivers/xen/tpmback/interface.c
--- a/drivers/xen/tpmback/interface.c
+++ b/drivers/xen/tpmback/interface.c
@@ -86,11 +86,10 @@ static int map_frontend_page(tpmif_t *tp
 	gnttab_set_map_op(&op, (unsigned long)tpmif->tx_area->addr,
 			  GNTMAP_host_map, shared_page, tpmif->domid);
 
-	if (HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, &op, 1))
-		BUG();
+	gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref, &op);
 
-	if (op.status) {
-		DPRINTK(" Grant table operation failure !\n");
+	if (op.status != GNTST_okay) {
+		DPRINTK(" Grant table operation failure %d!\n", (int) op.status);
 		return op.status;
 	}
 
diff -r df9e25a0189b -r 1170bc32db41 drivers/xen/tpmback/tpmback.c
--- a/drivers/xen/tpmback/tpmback.c
+++ b/drivers/xen/tpmback/tpmback.c
@@ -256,15 +256,13 @@ int _packet_write(struct packet *pak,
 		gnttab_set_map_op(&map_op, idx_to_kaddr(tpmif, i),
 				  GNTMAP_host_map, tx->ref, tpmif->domid);
 
-		if (unlikely(HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref,
-						       &map_op, 1))) {
-			BUG();
-		}
+		gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref, &map_op);
 
 		handle = map_op.handle;
 
-		if (map_op.status) {
-			DPRINTK(" Grant table operation failure !\n");
+		if (map_op.status != GNTST_okay) {
+			DPRINTK(" Grant table operation failure %d!\n", 
+						(int) map_op.status);
 			return 0;
 		}
 
@@ -394,13 +392,11 @@ static int packet_read_shmem(struct pack
 		gnttab_set_map_op(&map_op, idx_to_kaddr(tpmif, i),
 				  GNTMAP_host_map, tx->ref, tpmif->domid);
 
-		if (unlikely(HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref,
-						       &map_op, 1))) {
-			BUG();
-		}
+		gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref, &map_op);
 
-		if (map_op.status) {
-			DPRINTK(" Grant table operation failure !\n");
+		if (map_op.status != GNTST_okay) {
+			DPRINTK(" Grant table operation failure %d!\n",
+						(int) map_op.status);
 			return -EFAULT;
 		}
 
diff -r df9e25a0189b -r 1170bc32db41 drivers/xen/usbback/interface.c
--- a/drivers/xen/usbback/interface.c
+++ b/drivers/xen/usbback/interface.c
@@ -109,11 +109,11 @@ static int map_frontend_pages(usbif_t *u
 	gnttab_set_map_op(&op, (unsigned long)usbif->urb_ring_area->addr,
 			  GNTMAP_host_map, urb_ring_ref, usbif->domid);
 
-	if (HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, &op, 1))
-		BUG();
+	gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref, &op);
 
-	if (op.status) {
-		printk(KERN_ERR "grant table failure mapping urb_ring_ref\n");
+	if (op.status != GNTST_okay) {
+		printk(KERN_ERR "grant table failure mapping urb_ring_ref %d\n",
+							(int) op.status);
 		return op.status;
 	}
 
@@ -123,17 +123,17 @@ static int map_frontend_pages(usbif_t *u
 	gnttab_set_map_op(&op, (unsigned long)usbif->conn_ring_area->addr,
 			  GNTMAP_host_map, conn_ring_ref, usbif->domid);
 
-	if (HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, &op, 1))
-		BUG();
+	gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref, &op);
 
-	if (op.status) {
+	if (op.status != GNTST_okay) {
 		struct gnttab_unmap_grant_ref unop;
 		gnttab_set_unmap_op(&unop,
 				(unsigned long) usbif->urb_ring_area->addr,
 				GNTMAP_host_map, usbif->urb_shmem_handle);
 		VOID(HYPERVISOR_grant_table_op(GNTTABOP_unmap_grant_ref, &unop,
 				1));
-		printk(KERN_ERR "grant table failure mapping conn_ring_ref\n");
+		printk(KERN_ERR "grant table failure mapping conn_ring_ref %d\n",
+							(int) op.status);
 		return op.status;
 	}
 
diff -r df9e25a0189b -r 1170bc32db41 drivers/xen/usbback/usbback.c
--- a/drivers/xen/usbback/usbback.c
+++ b/drivers/xen/usbback/usbback.c
@@ -428,7 +428,9 @@ static int usbbk_gnttab_map(usbif_t *usb
 		BUG_ON(ret);
 
 		for (i = 0; i < nr_segs; i++) {
-			if (unlikely(map[i].status != 0)) {
+			if (unlikely(map[i].status == GNTST_eagain))
+				gnttab_check_GNTST_eagain_while(GNTTABOP_map_grant_ref, &map[i]);
+			if (unlikely(map[i].status != GNTST_okay)) {
 				printk(KERN_ERR "usbback: invalid buffer -- could not remap
it\n");
 				map[i].handle = USBBACK_INVALID_HANDLE;
 				ret |= 1;
diff -r df9e25a0189b -r 1170bc32db41 drivers/xen/xenbus/xenbus_backend_client.c
--- a/drivers/xen/xenbus/xenbus_backend_client.c
+++ b/drivers/xen/xenbus/xenbus_backend_client.c
@@ -48,8 +48,7 @@ struct vm_struct *xenbus_map_ring_valloc
 	gnttab_set_map_op(&op, (unsigned long)area->addr, GNTMAP_host_map,
 			  gnt_ref, dev->otherend_id);
 	
-	if (HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, &op, 1))
-		BUG();
+	gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref, &op);
 
 	if (op.status != GNTST_okay) {
 		free_vm_area(area);
@@ -75,15 +74,16 @@ int xenbus_map_ring(struct xenbus_device
 	
 	gnttab_set_map_op(&op, (unsigned long)vaddr, GNTMAP_host_map,
 			  gnt_ref, dev->otherend_id);
-	if (HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, &op, 1))
-		BUG();
+	gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref, &op);
 
 	if (op.status != GNTST_okay) {
 		xenbus_dev_fatal(dev, op.status,
 				 "mapping in shared page %d from domain %d",
 				 gnt_ref, dev->otherend_id);
-	} else
+	} else {
 		*handle = op.handle;
+		return 0;
+	}
 
 	return op.status;
 }
diff -r df9e25a0189b -r 1170bc32db41 include/xen/gnttab.h
--- a/include/xen/gnttab.h
+++ b/include/xen/gnttab.h
@@ -187,4 +187,41 @@ gnttab_set_replace_op(struct gnttab_unma
 	unmap->handle = handle;
 }
 
+#define gnttab_check_GNTST_eagain_while(__HCop, __HCarg_p)					\
+do {																		\
+	u8 __hc_delay = 1;														\
+	int __ret;																\
+	while (unlikely((__HCarg_p)->status == GNTST_eagain && __hc_delay))
{	\
+		msleep(__hc_delay++);												\
+		__ret = HYPERVISOR_grant_table_op(__HCop, (__HCarg_p), 1);			\
+		BUG_ON(__ret);														\
+	}																		\
+	if (__hc_delay == 0) {													\
+		printk(KERN_ERR "%s: %s gnt busy\n", __func__, current->comm);	
\
+		(__HCarg_p)->status = GNTST_bad_page;								\
+	}																		\
+	if ((__HCarg_p)->status != GNTST_okay)									\
+		printk(KERN_ERR "%s: %s gnt status %x\n", 							\
+			__func__, current->comm, (__HCarg_p)->status);					\
+} while(0)
+
+#define gnttab_check_GNTST_eagain_do_while(__HCop, __HCarg_p)			\
+do {																	\
+	u8 __hc_delay = 1;													\
+	int __ret;															\
+	do {																\
+		__ret = HYPERVISOR_grant_table_op(__HCop, (__HCarg_p), 1);		\
+		BUG_ON(__ret);													\
+		if ((__HCarg_p)->status == GNTST_eagain)						\
+			msleep(__hc_delay++);										\
+	} while ((__HCarg_p)->status == GNTST_eagain && __hc_delay);		\
+	if (__hc_delay == 0) {												\
+		printk(KERN_ERR "%s: %s gnt busy\n", __func__, current->comm);	\
+		(__HCarg_p)->status = GNTST_bad_page;							\
+	}																	\
+	if ((__HCarg_p)->status != GNTST_okay)								\
+		printk(KERN_ERR "%s: %s gnt status %x\n", 						\
+			__func__, current->comm, (__HCarg_p)->status);				\
+} while(0)
+
 #endif /* __ASM_GNTTAB_H__ */

Konrad Rzeszutek Wilk

2011-Dec-02 01:34 UTC

head link

Re: [PATCH 2 of 2] xenpaging: handle GNTST_eagain in kernel drivers

On Thu, Dec 01, 2011 at 03:51:55PM -0500, Andres Lagar-Cavilla
wrote:>  drivers/xen/blkback/blkback.c              |   6 ++-
>  drivers/xen/blkback/interface.c            |   9 +++-
>  drivers/xen/core/gnttab.c                  |   4 +-
>  drivers/xen/gntdev/gntdev.c                |  49
+++++++++++++++++------------
>  drivers/xen/netback/interface.c            |   5 +-
>  drivers/xen/netback/netback.c              |  16 ++++++---
>  drivers/xen/scsiback/interface.c           |  10 +++---
>  drivers/xen/scsiback/scsiback.c            |   4 +-
>  drivers/xen/tpmback/interface.c            |   7 +--
>  drivers/xen/tpmback/tpmback.c              |  20 ++++-------
>  drivers/xen/usbback/interface.c            |  16 ++++----
>  drivers/xen/usbback/usbback.c              |   4 +-
>  drivers/xen/xenbus/xenbus_backend_client.c |  10 +++---
>  include/xen/gnttab.h                       |  37 ++++++++++++++++++++++
>  14 files changed, 126 insertions(+), 71 deletions(-)
> 
> 
> Handle GNTST_eagain status from GNTTABOP_map_grant_ref and
> GNTTABOP_copy operations properly to allow usage of xenpaging without
> causing crashes or data corruption.
> 
> Replace all relevant HYPERVISOR_grant_table_op() calls with a retry
> loop. This loop is implemented as a macro to allow different
> GNTTABOP_* args. It will sleep up to 33 seconds and wait for the
> page to be paged in again.
> 
> All ->status checks were updated to use the GNTST_* namespace. All
> return values are converted from GNTST_* namespace to 0/-EINVAL, since
> all callers did not use the actual return value.
Any plans to do this for the pvops tree?
> 
> Signed-off-by: Olaf Hering <olaf@aepfle.de>
> Acked-by: Patrick Colp <pjcolp@cs.ubc.ca>
> 
> This is a port from xenlinux 2.6.18 to the 2.6.32 xcp tree
> Signed-off-by: Andres Lagar-Cavilla <andres@lagarcavilla.org>
> 
> diff -r df9e25a0189b -r 1170bc32db41 drivers/xen/blkback/blkback.c
> --- a/drivers/xen/blkback/blkback.c
> +++ b/drivers/xen/blkback/blkback.c
> @@ -701,11 +701,13 @@ static void dispatch_rw_block_io(blkif_t
>  	BUG_ON(ret);
>  
>  	for (i = 0; i < nseg; i++) {
> -		if (unlikely(map[i].status != 0)) {
> +		if (unlikely(map[i].status == GNTST_eagain))
> +			gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref,
&map[i]);
> +		if (unlikely(map[i].status != GNTST_okay)) {
>  			DPRINTK("grant map of dom %u gref %u failed: status %d\n",
>  				blkif->domid, req->seg[i].gref, map[i].status);
>  			map[i].handle = BLKBACK_INVALID_HANDLE;
> -			ret |= 1;
> +			ret = 1;
>  			continue;
>  		}
>  
> diff -r df9e25a0189b -r 1170bc32db41 drivers/xen/blkback/interface.c
> --- a/drivers/xen/blkback/interface.c
> +++ b/drivers/xen/blkback/interface.c
> @@ -95,7 +95,7 @@ static int map_frontend_pages(blkif_t *b
>  	struct vm_struct *area = blkif->blk_ring_area;
>  	struct gnttab_map_grant_ref op[BLKIF_MAX_RING_PAGES];
>  	unsigned int i;
> -	int status = 0;
> +	int status = GNTST_okay;
>  
>  	for (i = 0; i < nr_shared_pages; i++) {
>  		unsigned long addr = (unsigned long)area->addr +
> @@ -110,7 +110,10 @@ static int map_frontend_pages(blkif_t *b
>  		BUG();
>  
>  	for (i = 0; i < nr_shared_pages; i++) {
> -		if ((status = op[i].status) != 0) {
> +		if (op[i].status == GNTST_eagain)
> +			gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref, &op[i]);
> +		if (op[i].status != GNTST_okay) {
> +			status = op[i].status;
>  			blkif->shmem_handle[i] = INVALID_GRANT_HANDLE;
>  			continue;
>  		}
> @@ -120,7 +123,7 @@ static int map_frontend_pages(blkif_t *b
>  
>  	blkif->nr_shared_pages = nr_shared_pages;
>  
> -	if (status != 0) {
> +	if (status != GNTST_okay) {
>  		DPRINTK(" Grant table operation failure !\n");
>  		unmap_frontend_pages(blkif);
>  	}
> diff -r df9e25a0189b -r 1170bc32db41 drivers/xen/core/gnttab.c
> --- a/drivers/xen/core/gnttab.c
> +++ b/drivers/xen/core/gnttab.c
> @@ -773,7 +773,7 @@ static int gnttab_map(unsigned int start
>  		return -ENOSYS;
>  	}
>  
> -	BUG_ON(rc || setup.status);
> +	BUG_ON(rc || (setup.status != GNTST_okay));
>  
>  	if (shared.raw == NULL)
>  		shared.raw = arch_gnttab_alloc_shared(gframes);
> @@ -901,7 +901,7 @@ int gnttab_copy_grant_page(grant_ref_t r
>  	err = HYPERVISOR_grant_table_op(GNTTABOP_unmap_and_replace,
>  					&unmap, 1);
>  	BUG_ON(err);
> -	BUG_ON(unmap.status);
> +	BUG_ON(unmap.status != GNTST_okay);
>  
>  	write_sequnlock_irq(&gnttab_dma_lock);
>  
> diff -r df9e25a0189b -r 1170bc32db41 drivers/xen/gntdev/gntdev.c
> --- a/drivers/xen/gntdev/gntdev.c
> +++ b/drivers/xen/gntdev/gntdev.c
> @@ -503,7 +503,7 @@ static int gntdev_mmap (struct file *fli
>  	uint64_t ptep;
>  	int ret;
>  	int flags;
> -	int i;
> +	int i, exit_ret;
>  	struct page *page;
>  	gntdev_file_private_data_t *private_data = flip->private_data;
>  
> @@ -578,6 +578,7 @@ static int gntdev_mmap (struct file *fli
>  	vma->vm_mm->context.has_foreign_mappings = 1;
>  #endif
>  
> +	exit_ret = -ENOMEM;
>  	for (i = 0; i < size; ++i) {
>  
>  		flags = GNTMAP_host_map;
> @@ -598,14 +599,18 @@ static int gntdev_mmap (struct file *fli
>  		ret = HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, 
>  						&op, 1);
>  		BUG_ON(ret);
> -		if (op.status) {
> -			printk(KERN_ERR "Error mapping the grant reference "
> -			       "into the kernel (%d). domid = %d; ref = %d\n",
> -			       op.status,
> -			       private_data->grants[slot_index+i]
> -			       .u.valid.domid,
> -			       private_data->grants[slot_index+i]
> -			       .u.valid.ref);
> +		if (op.status != GNTST_okay) {
> +			if (op.status != GNTST_eagain)
> +				printk(KERN_ERR "Error mapping the grant reference "
> +					   "into the kernel (%d). domid = %d; ref = %d\n",
> +					   op.status,
> +					   private_data->grants[slot_index+i]
> +					   .u.valid.domid,
> +					   private_data->grants[slot_index+i]
> +					   .u.valid.ref);
> +			else
> +				/* Propagate instead of trying to fix it up */
> +				exit_ret = -EAGAIN;
>  			goto undo_map_out;
>  		}
>  
> @@ -674,14 +679,17 @@ static int gntdev_mmap (struct file *fli
>  			ret = HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref,
>  							&op, 1);
>  			BUG_ON(ret);
> -			if (op.status) {
> +			if (op.status != GNTST_okay) {
>  				printk(KERN_ERR "Error mapping the grant "
> -				       "reference into user space (%d). domid "
> -				       "= %d; ref = %d\n", op.status,
> -				       private_data->grants[slot_index+i].u
> -				       .valid.domid,
> -				       private_data->grants[slot_index+i].u
> -				       .valid.ref);
> +					   "reference into user space (%d). domid "
> +					   "= %d; ref = %d\n", op.status,
> +					   private_data->grants[slot_index+i].u
> +					   .valid.domid,
> +					   private_data->grants[slot_index+i].u
> +					   .valid.ref);
> +				/* GNTST_eagain (i.e. page paged out) sohuld never happen
> +				 * once we''ve mapped into kernel space */
> +				BUG_ON(op.status == GNTST_eagain);
>  				goto undo_map_out;
>  			}
>  			
> @@ -705,9 +713,10 @@ static int gntdev_mmap (struct file *fli
>  		}
>  
>  	}
> +	exit_ret = 0;
>  
>  	up_write(&private_data->grants_sem);
> -	return 0;
> +	return exit_ret;
>  
>  undo_map_out:
>  	/* If we have a mapping failure, the unmapping will be taken care of
> @@ -725,7 +734,7 @@ undo_map_out:
>  	
>  	up_write(&private_data->grants_sem);
>  
> -	return -ENOMEM;
> +	return exit_ret;
>  }
>  
>  static pte_t gntdev_clear_pte(struct vm_area_struct *vma, unsigned long
addr,
> @@ -777,7 +786,7 @@ static pte_t gntdev_clear_pte(struct vm_
>  			ret = HYPERVISOR_grant_table_op(
>  				GNTTABOP_unmap_grant_ref, &op, 1);
>  			BUG_ON(ret);
> -			if (op.status)
> +			if (op.status != GNTST_okay)
>  				printk("User unmap grant status = %d\n", 
>  				       op.status);
>  		} else {
> @@ -794,7 +803,7 @@ static pte_t gntdev_clear_pte(struct vm_
>  		ret = HYPERVISOR_grant_table_op(GNTTABOP_unmap_grant_ref, 
>  						&op, 1);
>  		BUG_ON(ret);
> -		if (op.status)
> +		if (op.status != GNTST_okay)
>  			printk("Kernel unmap grant status = %d\n", op.status);
>  
>  
> diff -r df9e25a0189b -r 1170bc32db41 drivers/xen/netback/interface.c
> --- a/drivers/xen/netback/interface.c
> +++ b/drivers/xen/netback/interface.c
> @@ -381,10 +381,9 @@ static int map_frontend_pages(struct xen
>  	gnttab_set_map_op(&op, (unsigned long)comms->ring_area->addr,
>  			  GNTMAP_host_map, ring_ref, domid);
>      
> -	if (HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, &op, 1))
> -		BUG();
> +	gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref, &op);
>  
> -	if (op.status) {
> +	if (op.status != GNTST_okay) {
>  		DPRINTK(" Gnttab failure mapping ring_ref!\n");
>  		free_vm_area(comms->ring_area);
>  		return op.status;
> diff -r df9e25a0189b -r 1170bc32db41 drivers/xen/netback/netback.c
> --- a/drivers/xen/netback/netback.c
> +++ b/drivers/xen/netback/netback.c
> @@ -419,11 +419,13 @@ static int netbk_check_gop(int nr_copy_s
>  
>  	for (i = 0; i < nr_copy_slots; i++) {
>  		copy_op = npo->copy + npo->copy_cons++;
> +		if (copy_op->status == GNTST_eagain)
> +			gnttab_check_GNTST_eagain_while(GNTTABOP_copy, copy_op);
>  		if (copy_op->status != GNTST_okay) {
> -				DPRINTK("Bad status %d from copy to DOM%d.\n",
> -					copy_op->status, domid);
> -				status = NETIF_RSP_ERROR;
> -			}
> +			DPRINTK("Bad status %d from copy to DOM%d.\n",
> +				copy_op->status, domid);
> +			status = NETIF_RSP_ERROR;
> +		}
>  	}
>  
>  	return status;
> @@ -1020,7 +1022,11 @@ static int netbk_tx_check_mop(struct xen
>  
>  	/* Check status of header. */
>  	err = mop->status;
> -	if (unlikely(err)) {
> +	if (err == GNTST_eagain) {
> +		gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref, mop);
> +		err = mop->status;
> +	}
> +	if (unlikely(err != GNTST_okay)) {
>  		pending_ring_idx_t index;
>  		index = pending_index(netbk->pending_prod++);
>  		txp = &pending_tx_info[pending_idx].req;
> diff -r df9e25a0189b -r 1170bc32db41 drivers/xen/scsiback/interface.c
> --- a/drivers/xen/scsiback/interface.c
> +++ b/drivers/xen/scsiback/interface.c
> @@ -69,18 +69,18 @@ static int map_frontend_page( struct vsc
>  				GNTMAP_host_map, ring_ref,
>  				info->domid);
>  
> -	err = HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, &op, 1);
> -	BUG_ON(err);
> +	gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref, &op);
>  
> -	if (op.status) {
> -		printk(KERN_ERR "scsiback: Grant table operation failure
!\n");
> +	if (op.status != GNTST_okay) {
> +		printk(KERN_ERR "scsiback: Grant table operation failure
%d!\n",
> +							(int) op.status);
>  		return op.status;
>  	}
>  
>  	info->shmem_ref    = ring_ref;
>  	info->shmem_handle = op.handle;
>  
> -	return (GNTST_okay);
> +	return 0;
>  }
>  
>  static void unmap_frontend_page(struct vscsibk_info *info)
> diff -r df9e25a0189b -r 1170bc32db41 drivers/xen/scsiback/scsiback.c
> --- a/drivers/xen/scsiback/scsiback.c
> +++ b/drivers/xen/scsiback/scsiback.c
> @@ -287,7 +287,9 @@ static int scsiback_gnttab_data_map(vscs
>  		for_each_sg (pending_req->sgl, sg, nr_segments, i) {
>  			struct page *pg;
>  
> -			if (unlikely(map[i].status != 0)) {
> +			if (unlikely(map[i].status == GNTST_eagain))
> +				gnttab_check_GNTST_eagain_while(GNTTABOP_map_grant_ref, &map[i]);
> +			if (unlikely(map[i].status != GNTST_okay)) {
>  				printk(KERN_ERR "scsiback: invalid buffer -- could not remap
it\n");
>  				map[i].handle = SCSIBACK_INVALID_HANDLE;
>  				err |= 1;
> diff -r df9e25a0189b -r 1170bc32db41 drivers/xen/tpmback/interface.c
> --- a/drivers/xen/tpmback/interface.c
> +++ b/drivers/xen/tpmback/interface.c
> @@ -86,11 +86,10 @@ static int map_frontend_page(tpmif_t *tp
>  	gnttab_set_map_op(&op, (unsigned long)tpmif->tx_area->addr,
>  			  GNTMAP_host_map, shared_page, tpmif->domid);
>  
> -	if (HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, &op, 1))
> -		BUG();
> +	gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref, &op);
>  
> -	if (op.status) {
> -		DPRINTK(" Grant table operation failure !\n");
> +	if (op.status != GNTST_okay) {
> +		DPRINTK(" Grant table operation failure %d!\n", (int)
op.status);
>  		return op.status;
>  	}
>  
> diff -r df9e25a0189b -r 1170bc32db41 drivers/xen/tpmback/tpmback.c
> --- a/drivers/xen/tpmback/tpmback.c
> +++ b/drivers/xen/tpmback/tpmback.c
> @@ -256,15 +256,13 @@ int _packet_write(struct packet *pak,
>  		gnttab_set_map_op(&map_op, idx_to_kaddr(tpmif, i),
>  				  GNTMAP_host_map, tx->ref, tpmif->domid);
>  
> -		if (unlikely(HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref,
> -						       &map_op, 1))) {
> -			BUG();
> -		}
> +		gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref, &map_op);
>  
>  		handle = map_op.handle;
>  
> -		if (map_op.status) {
> -			DPRINTK(" Grant table operation failure !\n");
> +		if (map_op.status != GNTST_okay) {
> +			DPRINTK(" Grant table operation failure %d!\n", 
> +						(int) map_op.status);
>  			return 0;
>  		}
>  
> @@ -394,13 +392,11 @@ static int packet_read_shmem(struct pack
>  		gnttab_set_map_op(&map_op, idx_to_kaddr(tpmif, i),
>  				  GNTMAP_host_map, tx->ref, tpmif->domid);
>  
> -		if (unlikely(HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref,
> -						       &map_op, 1))) {
> -			BUG();
> -		}
> +		gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref, &map_op);
>  
> -		if (map_op.status) {
> -			DPRINTK(" Grant table operation failure !\n");
> +		if (map_op.status != GNTST_okay) {
> +			DPRINTK(" Grant table operation failure %d!\n",
> +						(int) map_op.status);
>  			return -EFAULT;
>  		}
>  
> diff -r df9e25a0189b -r 1170bc32db41 drivers/xen/usbback/interface.c
> --- a/drivers/xen/usbback/interface.c
> +++ b/drivers/xen/usbback/interface.c
> @@ -109,11 +109,11 @@ static int map_frontend_pages(usbif_t *u
>  	gnttab_set_map_op(&op, (unsigned
long)usbif->urb_ring_area->addr,
>  			  GNTMAP_host_map, urb_ring_ref, usbif->domid);
>  
> -	if (HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, &op, 1))
> -		BUG();
> +	gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref, &op);
>  
> -	if (op.status) {
> -		printk(KERN_ERR "grant table failure mapping urb_ring_ref\n");
> +	if (op.status != GNTST_okay) {
> +		printk(KERN_ERR "grant table failure mapping urb_ring_ref
%d\n",
> +							(int) op.status);
>  		return op.status;
>  	}
>  
> @@ -123,17 +123,17 @@ static int map_frontend_pages(usbif_t *u
>  	gnttab_set_map_op(&op, (unsigned
long)usbif->conn_ring_area->addr,
>  			  GNTMAP_host_map, conn_ring_ref, usbif->domid);
>  
> -	if (HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, &op, 1))
> -		BUG();
> +	gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref, &op);
>  
> -	if (op.status) {
> +	if (op.status != GNTST_okay) {
>  		struct gnttab_unmap_grant_ref unop;
>  		gnttab_set_unmap_op(&unop,
>  				(unsigned long) usbif->urb_ring_area->addr,
>  				GNTMAP_host_map, usbif->urb_shmem_handle);
>  		VOID(HYPERVISOR_grant_table_op(GNTTABOP_unmap_grant_ref, &unop,
>  				1));
> -		printk(KERN_ERR "grant table failure mapping
conn_ring_ref\n");
> +		printk(KERN_ERR "grant table failure mapping conn_ring_ref
%d\n",
> +							(int) op.status);
>  		return op.status;
>  	}
>  
> diff -r df9e25a0189b -r 1170bc32db41 drivers/xen/usbback/usbback.c
> --- a/drivers/xen/usbback/usbback.c
> +++ b/drivers/xen/usbback/usbback.c
> @@ -428,7 +428,9 @@ static int usbbk_gnttab_map(usbif_t *usb
>  		BUG_ON(ret);
>  
>  		for (i = 0; i < nr_segs; i++) {
> -			if (unlikely(map[i].status != 0)) {
> +			if (unlikely(map[i].status == GNTST_eagain))
> +				gnttab_check_GNTST_eagain_while(GNTTABOP_map_grant_ref, &map[i]);
> +			if (unlikely(map[i].status != GNTST_okay)) {
>  				printk(KERN_ERR "usbback: invalid buffer -- could not remap
it\n");
>  				map[i].handle = USBBACK_INVALID_HANDLE;
>  				ret |= 1;
> diff -r df9e25a0189b -r 1170bc32db41
drivers/xen/xenbus/xenbus_backend_client.c
> --- a/drivers/xen/xenbus/xenbus_backend_client.c
> +++ b/drivers/xen/xenbus/xenbus_backend_client.c
> @@ -48,8 +48,7 @@ struct vm_struct *xenbus_map_ring_valloc
>  	gnttab_set_map_op(&op, (unsigned long)area->addr, GNTMAP_host_map,
>  			  gnt_ref, dev->otherend_id);
>  	
> -	if (HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, &op, 1))
> -		BUG();
> +	gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref, &op);
>  
>  	if (op.status != GNTST_okay) {
>  		free_vm_area(area);
> @@ -75,15 +74,16 @@ int xenbus_map_ring(struct xenbus_device
>  	
>  	gnttab_set_map_op(&op, (unsigned long)vaddr, GNTMAP_host_map,
>  			  gnt_ref, dev->otherend_id);
> -	if (HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, &op, 1))
> -		BUG();
> +	gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref, &op);
>  
>  	if (op.status != GNTST_okay) {
>  		xenbus_dev_fatal(dev, op.status,
>  				 "mapping in shared page %d from domain %d",
>  				 gnt_ref, dev->otherend_id);
> -	} else
> +	} else {
>  		*handle = op.handle;
> +		return 0;
> +	}
>  
>  	return op.status;
>  }
> diff -r df9e25a0189b -r 1170bc32db41 include/xen/gnttab.h
> --- a/include/xen/gnttab.h
> +++ b/include/xen/gnttab.h
> @@ -187,4 +187,41 @@ gnttab_set_replace_op(struct gnttab_unma
>  	unmap->handle = handle;
>  }
>  
> +#define gnttab_check_GNTST_eagain_while(__HCop, __HCarg_p)					\
> +do {																		\
> +	u8 __hc_delay = 1;														\
> +	int __ret;																\
> +	while (unlikely((__HCarg_p)->status == GNTST_eagain &&
__hc_delay)) {	\
> +		msleep(__hc_delay++);												\
> +		__ret = HYPERVISOR_grant_table_op(__HCop, (__HCarg_p), 1);			\
> +		BUG_ON(__ret);														\
> +	}																		\
> +	if (__hc_delay == 0) {													\
> +		printk(KERN_ERR "%s: %s gnt busy\n", __func__,
current->comm);		\
> +		(__HCarg_p)->status = GNTST_bad_page;								\
> +	}																		\
> +	if ((__HCarg_p)->status != GNTST_okay)									\
> +		printk(KERN_ERR "%s: %s gnt status %x\n", 							\
> +			__func__, current->comm, (__HCarg_p)->status);					\
> +} while(0)
> +
> +#define gnttab_check_GNTST_eagain_do_while(__HCop, __HCarg_p)			\
> +do {																	\
> +	u8 __hc_delay = 1;													\
> +	int __ret;															\
> +	do {																\
> +		__ret = HYPERVISOR_grant_table_op(__HCop, (__HCarg_p), 1);		\
> +		BUG_ON(__ret);													\
> +		if ((__HCarg_p)->status == GNTST_eagain)						\
> +			msleep(__hc_delay++);										\
> +	} while ((__HCarg_p)->status == GNTST_eagain && __hc_delay);	
\
> +	if (__hc_delay == 0) {												\
> +		printk(KERN_ERR "%s: %s gnt busy\n", __func__,
current->comm);	\
> +		(__HCarg_p)->status = GNTST_bad_page;							\
> +	}																	\
> +	if ((__HCarg_p)->status != GNTST_okay)								\
> +		printk(KERN_ERR "%s: %s gnt status %x\n", 						\
> +			__func__, current->comm, (__HCarg_p)->status);				\
> +} while(0)
> +
>  #endif /* __ASM_GNTTAB_H__ */
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

Jan Beulich

2011-Dec-02 09:22 UTC

head link

Re: [PATCH 0 of 2] Paging support updates for XCP dom0

>>> On 01.12.11 at 21:51, Andres Lagar-Cavilla
<andres@lagarcavilla.org> wrote:
> This is a cherry pick of two patches that add support for guest paged out
> frames in the XCP 2.6.32 dom0 patch queue.
The better thing would probably be to re-base the tree to current
SLE11 SP1 sources. Not sure who would do that though (and also
not sure what state the tree is in maintenance wise). Ian?

Jan
> First patch propagates the ENOENT returned by the hypervisor in the case
> of a paged out page, all the way up the call chain to the MMAPBATCH_V2
> ioctl. The ioctl is mainly used to harvest those return values and retry.
> 
> The second patch adds retry loops to all backend grant operations (map and
> netback copy), in the case of a paged out frame.
> 
> Signed-off-by: Olaf Hering <olaf@aepfle.de>
> Signed-off-by: Jan Beulich <jbeulich@novell.com>
> Acked-by: Patrick Colp <pjcolp@cs.ubc.ca>
> Acked-by: Andres Lagar-Cavilla <andres@lagarcavilla>
> Ported and submitted by Andres Lagar-Cavilla
> 
>  arch/x86/mm/ioremap-xen.c                  |  12 ++----
>  drivers/xen/blkback/blkback.c              |   6 ++-
>  drivers/xen/blkback/interface.c            |   9 +++-
>  drivers/xen/core/gnttab.c                  |   4 +-
>  drivers/xen/gntdev/gntdev.c                |  49
+++++++++++++++++------------
>  drivers/xen/netback/interface.c            |   5 +-
>  drivers/xen/netback/netback.c              |  16 ++++++---
>  drivers/xen/scsiback/interface.c           |  10 +++---
>  drivers/xen/scsiback/scsiback.c            |   4 +-
>  drivers/xen/tpmback/interface.c            |   7 +--
>  drivers/xen/tpmback/tpmback.c              |  20 ++++-------
>  drivers/xen/usbback/interface.c            |  16 ++++----
>  drivers/xen/usbback/usbback.c              |   4 +-
>  drivers/xen/xenbus/xenbus_backend_client.c |  10 +++---
>  include/xen/gnttab.h                       |  37 ++++++++++++++++++++++
>  15 files changed, 130 insertions(+), 79 deletions(-)

Jan Beulich

2011-Dec-02 09:26 UTC

head link

Re: [PATCH 1 of 2] xen/x86: make __direct_remap_pfn_range()''s return value meaningful

>>> On 01.12.11 at 21:51, Andres Lagar-Cavilla
<andres@lagarcavilla.org> wrote:
> arch/x86/mm/ioremap-xen.c |  12 ++++--------
>  1 files changed, 4 insertions(+), 8 deletions(-)
> 
> 
> This change fixes the xc_map_foreign_bulk interface, which would
> otherwise cause SIGBUS when pages are gone because -ENOENT is not
> returned as expected by the IOCTL_PRIVCMD_MMAPBATCH_V2 ioctl.
> 
> Signed-off-by: Jan Beulich <jbeulich@novell.com>
You lost authorship here.

Jan

Andres Lagar-Cavilla

2011-Dec-02 15:27 UTC

head link

Re: [PATCH 2 of 2] xenpaging: handle GNTST_eagain in kernel drivers

> On Thu, Dec 01, 2011 at 03:51:55PM -0500, Andres Lagar-Cavilla wrote:
>>  drivers/xen/blkback/blkback.c              |   6 ++-
>>  drivers/xen/blkback/interface.c            |   9 +++-
>>  drivers/xen/core/gnttab.c                  |   4 +-
>>  drivers/xen/gntdev/gntdev.c                |  49
>> +++++++++++++++++------------
>>  drivers/xen/netback/interface.c            |   5 +-
>>  drivers/xen/netback/netback.c              |  16 ++++++---
>>  drivers/xen/scsiback/interface.c           |  10 +++---
>>  drivers/xen/scsiback/scsiback.c            |   4 +-
>>  drivers/xen/tpmback/interface.c            |   7 +--
>>  drivers/xen/tpmback/tpmback.c              |  20 ++++-------
>>  drivers/xen/usbback/interface.c            |  16 ++++----
>>  drivers/xen/usbback/usbback.c              |   4 +-
>>  drivers/xen/xenbus/xenbus_backend_client.c |  10 +++---
>>  include/xen/gnttab.h                       |  37
++++++++++++++++++++++
>>  14 files changed, 126 insertions(+), 71 deletions(-)
>>
>>
>> Handle GNTST_eagain status from GNTTABOP_map_grant_ref and
>> GNTTABOP_copy operations properly to allow usage of xenpaging without
>> causing crashes or data corruption.
>>
>> Replace all relevant HYPERVISOR_grant_table_op() calls with a retry
>> loop. This loop is implemented as a macro to allow different
>> GNTTABOP_* args. It will sleep up to 33 seconds and wait for the
>> page to be paged in again.
>>
>> All ->status checks were updated to use the GNTST_* namespace. All
>> return values are converted from GNTST_* namespace to 0/-EINVAL, since
>> all callers did not use the actual return value.
>
> Any plans to do this for the pvops tree?Slowly but surely. Working with XCP presently, so pvops is not at the top
of my stack. Do you want to get at a specific merge window?

Andres>
>>
>> Signed-off-by: Olaf Hering <olaf@aepfle.de>
>> Acked-by: Patrick Colp <pjcolp@cs.ubc.ca>
>>
>> This is a port from xenlinux 2.6.18 to the 2.6.32 xcp tree
>> Signed-off-by: Andres Lagar-Cavilla <andres@lagarcavilla.org>
>>
>> diff -r df9e25a0189b -r 1170bc32db41 drivers/xen/blkback/blkback.c
>> --- a/drivers/xen/blkback/blkback.c
>> +++ b/drivers/xen/blkback/blkback.c
>> @@ -701,11 +701,13 @@ static void dispatch_rw_block_io(blkif_t
>>  	BUG_ON(ret);
>>
>>  	for (i = 0; i < nseg; i++) {
>> -		if (unlikely(map[i].status != 0)) {
>> +		if (unlikely(map[i].status == GNTST_eagain))
>> +			gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref,
&map[i]);
>> +		if (unlikely(map[i].status != GNTST_okay)) {
>>  			DPRINTK("grant map of dom %u gref %u failed: status
%d\n",
>>  				blkif->domid, req->seg[i].gref, map[i].status);
>>  			map[i].handle = BLKBACK_INVALID_HANDLE;
>> -			ret |= 1;
>> +			ret = 1;
>>  			continue;
>>  		}
>>
>> diff -r df9e25a0189b -r 1170bc32db41 drivers/xen/blkback/interface.c
>> --- a/drivers/xen/blkback/interface.c
>> +++ b/drivers/xen/blkback/interface.c
>> @@ -95,7 +95,7 @@ static int map_frontend_pages(blkif_t *b
>>  	struct vm_struct *area = blkif->blk_ring_area;
>>  	struct gnttab_map_grant_ref op[BLKIF_MAX_RING_PAGES];
>>  	unsigned int i;
>> -	int status = 0;
>> +	int status = GNTST_okay;
>>
>>  	for (i = 0; i < nr_shared_pages; i++) {
>>  		unsigned long addr = (unsigned long)area->addr +
>> @@ -110,7 +110,10 @@ static int map_frontend_pages(blkif_t *b
>>  		BUG();
>>
>>  	for (i = 0; i < nr_shared_pages; i++) {
>> -		if ((status = op[i].status) != 0) {
>> +		if (op[i].status == GNTST_eagain)
>> +			gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref,
&op[i]);
>> +		if (op[i].status != GNTST_okay) {
>> +			status = op[i].status;
>>  			blkif->shmem_handle[i] = INVALID_GRANT_HANDLE;
>>  			continue;
>>  		}
>> @@ -120,7 +123,7 @@ static int map_frontend_pages(blkif_t *b
>>
>>  	blkif->nr_shared_pages = nr_shared_pages;
>>
>> -	if (status != 0) {
>> +	if (status != GNTST_okay) {
>>  		DPRINTK(" Grant table operation failure !\n");
>>  		unmap_frontend_pages(blkif);
>>  	}
>> diff -r df9e25a0189b -r 1170bc32db41 drivers/xen/core/gnttab.c
>> --- a/drivers/xen/core/gnttab.c
>> +++ b/drivers/xen/core/gnttab.c
>> @@ -773,7 +773,7 @@ static int gnttab_map(unsigned int start
>>  		return -ENOSYS;
>>  	}
>>
>> -	BUG_ON(rc || setup.status);
>> +	BUG_ON(rc || (setup.status != GNTST_okay));
>>
>>  	if (shared.raw == NULL)
>>  		shared.raw = arch_gnttab_alloc_shared(gframes);
>> @@ -901,7 +901,7 @@ int gnttab_copy_grant_page(grant_ref_t r
>>  	err = HYPERVISOR_grant_table_op(GNTTABOP_unmap_and_replace,
>>  					&unmap, 1);
>>  	BUG_ON(err);
>> -	BUG_ON(unmap.status);
>> +	BUG_ON(unmap.status != GNTST_okay);
>>
>>  	write_sequnlock_irq(&gnttab_dma_lock);
>>
>> diff -r df9e25a0189b -r 1170bc32db41 drivers/xen/gntdev/gntdev.c
>> --- a/drivers/xen/gntdev/gntdev.c
>> +++ b/drivers/xen/gntdev/gntdev.c
>> @@ -503,7 +503,7 @@ static int gntdev_mmap (struct file *fli
>>  	uint64_t ptep;
>>  	int ret;
>>  	int flags;
>> -	int i;
>> +	int i, exit_ret;
>>  	struct page *page;
>>  	gntdev_file_private_data_t *private_data = flip->private_data;
>>
>> @@ -578,6 +578,7 @@ static int gntdev_mmap (struct file *fli
>>  	vma->vm_mm->context.has_foreign_mappings = 1;
>>  #endif
>>
>> +	exit_ret = -ENOMEM;
>>  	for (i = 0; i < size; ++i) {
>>
>>  		flags = GNTMAP_host_map;
>> @@ -598,14 +599,18 @@ static int gntdev_mmap (struct file *fli
>>  		ret = HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref,
>>  						&op, 1);
>>  		BUG_ON(ret);
>> -		if (op.status) {
>> -			printk(KERN_ERR "Error mapping the grant reference "
>> -			       "into the kernel (%d). domid = %d; ref = %d\n",
>> -			       op.status,
>> -			       private_data->grants[slot_index+i]
>> -			       .u.valid.domid,
>> -			       private_data->grants[slot_index+i]
>> -			       .u.valid.ref);
>> +		if (op.status != GNTST_okay) {
>> +			if (op.status != GNTST_eagain)
>> +				printk(KERN_ERR "Error mapping the grant reference "
>> +					   "into the kernel (%d). domid = %d; ref = %d\n",
>> +					   op.status,
>> +					   private_data->grants[slot_index+i]
>> +					   .u.valid.domid,
>> +					   private_data->grants[slot_index+i]
>> +					   .u.valid.ref);
>> +			else
>> +				/* Propagate instead of trying to fix it up */
>> +				exit_ret = -EAGAIN;
>>  			goto undo_map_out;
>>  		}
>>
>> @@ -674,14 +679,17 @@ static int gntdev_mmap (struct file *fli
>>  			ret = HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref,
>>  							&op, 1);
>>  			BUG_ON(ret);
>> -			if (op.status) {
>> +			if (op.status != GNTST_okay) {
>>  				printk(KERN_ERR "Error mapping the grant "
>> -				       "reference into user space (%d). domid "
>> -				       "= %d; ref = %d\n", op.status,
>> -				       private_data->grants[slot_index+i].u
>> -				       .valid.domid,
>> -				       private_data->grants[slot_index+i].u
>> -				       .valid.ref);
>> +					   "reference into user space (%d). domid "
>> +					   "= %d; ref = %d\n", op.status,
>> +					   private_data->grants[slot_index+i].u
>> +					   .valid.domid,
>> +					   private_data->grants[slot_index+i].u
>> +					   .valid.ref);
>> +				/* GNTST_eagain (i.e. page paged out) sohuld never happen
>> +				 * once we''ve mapped into kernel space */
>> +				BUG_ON(op.status == GNTST_eagain);
>>  				goto undo_map_out;
>>  			}
>>
>> @@ -705,9 +713,10 @@ static int gntdev_mmap (struct file *fli
>>  		}
>>
>>  	}
>> +	exit_ret = 0;
>>
>>  	up_write(&private_data->grants_sem);
>> -	return 0;
>> +	return exit_ret;
>>
>>  undo_map_out:
>>  	/* If we have a mapping failure, the unmapping will be taken care of
>> @@ -725,7 +734,7 @@ undo_map_out:
>>
>>  	up_write(&private_data->grants_sem);
>>
>> -	return -ENOMEM;
>> +	return exit_ret;
>>  }
>>
>>  static pte_t gntdev_clear_pte(struct vm_area_struct *vma, unsigned
long
>> addr,
>> @@ -777,7 +786,7 @@ static pte_t gntdev_clear_pte(struct vm_
>>  			ret = HYPERVISOR_grant_table_op(
>>  				GNTTABOP_unmap_grant_ref, &op, 1);
>>  			BUG_ON(ret);
>> -			if (op.status)
>> +			if (op.status != GNTST_okay)
>>  				printk("User unmap grant status = %d\n",
>>  				       op.status);
>>  		} else {
>> @@ -794,7 +803,7 @@ static pte_t gntdev_clear_pte(struct vm_
>>  		ret = HYPERVISOR_grant_table_op(GNTTABOP_unmap_grant_ref,
>>  						&op, 1);
>>  		BUG_ON(ret);
>> -		if (op.status)
>> +		if (op.status != GNTST_okay)
>>  			printk("Kernel unmap grant status = %d\n", op.status);
>>
>>
>> diff -r df9e25a0189b -r 1170bc32db41 drivers/xen/netback/interface.c
>> --- a/drivers/xen/netback/interface.c
>> +++ b/drivers/xen/netback/interface.c
>> @@ -381,10 +381,9 @@ static int map_frontend_pages(struct xen
>>  	gnttab_set_map_op(&op, (unsigned
long)comms->ring_area->addr,
>>  			  GNTMAP_host_map, ring_ref, domid);
>>
>> -	if (HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, &op, 1))
>> -		BUG();
>> +	gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref, &op);
>>
>> -	if (op.status) {
>> +	if (op.status != GNTST_okay) {
>>  		DPRINTK(" Gnttab failure mapping ring_ref!\n");
>>  		free_vm_area(comms->ring_area);
>>  		return op.status;
>> diff -r df9e25a0189b -r 1170bc32db41 drivers/xen/netback/netback.c
>> --- a/drivers/xen/netback/netback.c
>> +++ b/drivers/xen/netback/netback.c
>> @@ -419,11 +419,13 @@ static int netbk_check_gop(int nr_copy_s
>>
>>  	for (i = 0; i < nr_copy_slots; i++) {
>>  		copy_op = npo->copy + npo->copy_cons++;
>> +		if (copy_op->status == GNTST_eagain)
>> +			gnttab_check_GNTST_eagain_while(GNTTABOP_copy, copy_op);
>>  		if (copy_op->status != GNTST_okay) {
>> -				DPRINTK("Bad status %d from copy to DOM%d.\n",
>> -					copy_op->status, domid);
>> -				status = NETIF_RSP_ERROR;
>> -			}
>> +			DPRINTK("Bad status %d from copy to DOM%d.\n",
>> +				copy_op->status, domid);
>> +			status = NETIF_RSP_ERROR;
>> +		}
>>  	}
>>
>>  	return status;
>> @@ -1020,7 +1022,11 @@ static int netbk_tx_check_mop(struct xen
>>
>>  	/* Check status of header. */
>>  	err = mop->status;
>> -	if (unlikely(err)) {
>> +	if (err == GNTST_eagain) {
>> +		gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref, mop);
>> +		err = mop->status;
>> +	}
>> +	if (unlikely(err != GNTST_okay)) {
>>  		pending_ring_idx_t index;
>>  		index = pending_index(netbk->pending_prod++);
>>  		txp = &pending_tx_info[pending_idx].req;
>> diff -r df9e25a0189b -r 1170bc32db41 drivers/xen/scsiback/interface.c
>> --- a/drivers/xen/scsiback/interface.c
>> +++ b/drivers/xen/scsiback/interface.c
>> @@ -69,18 +69,18 @@ static int map_frontend_page( struct vsc
>>  				GNTMAP_host_map, ring_ref,
>>  				info->domid);
>>
>> -	err = HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, &op, 1);
>> -	BUG_ON(err);
>> +	gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref, &op);
>>
>> -	if (op.status) {
>> -		printk(KERN_ERR "scsiback: Grant table operation failure
!\n");
>> +	if (op.status != GNTST_okay) {
>> +		printk(KERN_ERR "scsiback: Grant table operation failure
%d!\n",
>> +							(int) op.status);
>>  		return op.status;
>>  	}
>>
>>  	info->shmem_ref    = ring_ref;
>>  	info->shmem_handle = op.handle;
>>
>> -	return (GNTST_okay);
>> +	return 0;
>>  }
>>
>>  static void unmap_frontend_page(struct vscsibk_info *info)
>> diff -r df9e25a0189b -r 1170bc32db41 drivers/xen/scsiback/scsiback.c
>> --- a/drivers/xen/scsiback/scsiback.c
>> +++ b/drivers/xen/scsiback/scsiback.c
>> @@ -287,7 +287,9 @@ static int scsiback_gnttab_data_map(vscs
>>  		for_each_sg (pending_req->sgl, sg, nr_segments, i) {
>>  			struct page *pg;
>>
>> -			if (unlikely(map[i].status != 0)) {
>> +			if (unlikely(map[i].status == GNTST_eagain))
>> +				gnttab_check_GNTST_eagain_while(GNTTABOP_map_grant_ref,
&map[i]);
>> +			if (unlikely(map[i].status != GNTST_okay)) {
>>  				printk(KERN_ERR "scsiback: invalid buffer -- could not remap
>> it\n");
>>  				map[i].handle = SCSIBACK_INVALID_HANDLE;
>>  				err |= 1;
>> diff -r df9e25a0189b -r 1170bc32db41 drivers/xen/tpmback/interface.c
>> --- a/drivers/xen/tpmback/interface.c
>> +++ b/drivers/xen/tpmback/interface.c
>> @@ -86,11 +86,10 @@ static int map_frontend_page(tpmif_t *tp
>>  	gnttab_set_map_op(&op, (unsigned long)tpmif->tx_area->addr,
>>  			  GNTMAP_host_map, shared_page, tpmif->domid);
>>
>> -	if (HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, &op, 1))
>> -		BUG();
>> +	gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref, &op);
>>
>> -	if (op.status) {
>> -		DPRINTK(" Grant table operation failure !\n");
>> +	if (op.status != GNTST_okay) {
>> +		DPRINTK(" Grant table operation failure %d!\n", (int)
op.status);
>>  		return op.status;
>>  	}
>>
>> diff -r df9e25a0189b -r 1170bc32db41 drivers/xen/tpmback/tpmback.c
>> --- a/drivers/xen/tpmback/tpmback.c
>> +++ b/drivers/xen/tpmback/tpmback.c
>> @@ -256,15 +256,13 @@ int _packet_write(struct packet *pak,
>>  		gnttab_set_map_op(&map_op, idx_to_kaddr(tpmif, i),
>>  				  GNTMAP_host_map, tx->ref, tpmif->domid);
>>
>> -		if (unlikely(HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref,
>> -						       &map_op, 1))) {
>> -			BUG();
>> -		}
>> +		gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref,
&map_op);
>>
>>  		handle = map_op.handle;
>>
>> -		if (map_op.status) {
>> -			DPRINTK(" Grant table operation failure !\n");
>> +		if (map_op.status != GNTST_okay) {
>> +			DPRINTK(" Grant table operation failure %d!\n",
>> +						(int) map_op.status);
>>  			return 0;
>>  		}
>>
>> @@ -394,13 +392,11 @@ static int packet_read_shmem(struct pack
>>  		gnttab_set_map_op(&map_op, idx_to_kaddr(tpmif, i),
>>  				  GNTMAP_host_map, tx->ref, tpmif->domid);
>>
>> -		if (unlikely(HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref,
>> -						       &map_op, 1))) {
>> -			BUG();
>> -		}
>> +		gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref,
&map_op);
>>
>> -		if (map_op.status) {
>> -			DPRINTK(" Grant table operation failure !\n");
>> +		if (map_op.status != GNTST_okay) {
>> +			DPRINTK(" Grant table operation failure %d!\n",
>> +						(int) map_op.status);
>>  			return -EFAULT;
>>  		}
>>
>> diff -r df9e25a0189b -r 1170bc32db41 drivers/xen/usbback/interface.c
>> --- a/drivers/xen/usbback/interface.c
>> +++ b/drivers/xen/usbback/interface.c
>> @@ -109,11 +109,11 @@ static int map_frontend_pages(usbif_t *u
>>  	gnttab_set_map_op(&op, (unsigned
long)usbif->urb_ring_area->addr,
>>  			  GNTMAP_host_map, urb_ring_ref, usbif->domid);
>>
>> -	if (HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, &op, 1))
>> -		BUG();
>> +	gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref, &op);
>>
>> -	if (op.status) {
>> -		printk(KERN_ERR "grant table failure mapping
urb_ring_ref\n");
>> +	if (op.status != GNTST_okay) {
>> +		printk(KERN_ERR "grant table failure mapping urb_ring_ref
%d\n",
>> +							(int) op.status);
>>  		return op.status;
>>  	}
>>
>> @@ -123,17 +123,17 @@ static int map_frontend_pages(usbif_t *u
>>  	gnttab_set_map_op(&op, (unsigned
long)usbif->conn_ring_area->addr,
>>  			  GNTMAP_host_map, conn_ring_ref, usbif->domid);
>>
>> -	if (HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, &op, 1))
>> -		BUG();
>> +	gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref, &op);
>>
>> -	if (op.status) {
>> +	if (op.status != GNTST_okay) {
>>  		struct gnttab_unmap_grant_ref unop;
>>  		gnttab_set_unmap_op(&unop,
>>  				(unsigned long) usbif->urb_ring_area->addr,
>>  				GNTMAP_host_map, usbif->urb_shmem_handle);
>>  		VOID(HYPERVISOR_grant_table_op(GNTTABOP_unmap_grant_ref, &unop,
>>  				1));
>> -		printk(KERN_ERR "grant table failure mapping
conn_ring_ref\n");
>> +		printk(KERN_ERR "grant table failure mapping conn_ring_ref
%d\n",
>> +							(int) op.status);
>>  		return op.status;
>>  	}
>>
>> diff -r df9e25a0189b -r 1170bc32db41 drivers/xen/usbback/usbback.c
>> --- a/drivers/xen/usbback/usbback.c
>> +++ b/drivers/xen/usbback/usbback.c
>> @@ -428,7 +428,9 @@ static int usbbk_gnttab_map(usbif_t *usb
>>  		BUG_ON(ret);
>>
>>  		for (i = 0; i < nr_segs; i++) {
>> -			if (unlikely(map[i].status != 0)) {
>> +			if (unlikely(map[i].status == GNTST_eagain))
>> +				gnttab_check_GNTST_eagain_while(GNTTABOP_map_grant_ref,
&map[i]);
>> +			if (unlikely(map[i].status != GNTST_okay)) {
>>  				printk(KERN_ERR "usbback: invalid buffer -- could not remap
it\n");
>>  				map[i].handle = USBBACK_INVALID_HANDLE;
>>  				ret |= 1;
>> diff -r df9e25a0189b -r 1170bc32db41
>> drivers/xen/xenbus/xenbus_backend_client.c
>> --- a/drivers/xen/xenbus/xenbus_backend_client.c
>> +++ b/drivers/xen/xenbus/xenbus_backend_client.c
>> @@ -48,8 +48,7 @@ struct vm_struct *xenbus_map_ring_valloc
>>  	gnttab_set_map_op(&op, (unsigned long)area->addr,
GNTMAP_host_map,
>>  			  gnt_ref, dev->otherend_id);
>>
>> -	if (HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, &op, 1))
>> -		BUG();
>> +	gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref, &op);
>>
>>  	if (op.status != GNTST_okay) {
>>  		free_vm_area(area);
>> @@ -75,15 +74,16 @@ int xenbus_map_ring(struct xenbus_device
>>
>>  	gnttab_set_map_op(&op, (unsigned long)vaddr, GNTMAP_host_map,
>>  			  gnt_ref, dev->otherend_id);
>> -	if (HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, &op, 1))
>> -		BUG();
>> +	gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref, &op);
>>
>>  	if (op.status != GNTST_okay) {
>>  		xenbus_dev_fatal(dev, op.status,
>>  				 "mapping in shared page %d from domain %d",
>>  				 gnt_ref, dev->otherend_id);
>> -	} else
>> +	} else {
>>  		*handle = op.handle;
>> +		return 0;
>> +	}
>>
>>  	return op.status;
>>  }
>> diff -r df9e25a0189b -r 1170bc32db41 include/xen/gnttab.h
>> --- a/include/xen/gnttab.h
>> +++ b/include/xen/gnttab.h
>> @@ -187,4 +187,41 @@ gnttab_set_replace_op(struct gnttab_unma
>>  	unmap->handle = handle;
>>  }
>>
>> +#define gnttab_check_GNTST_eagain_while(__HCop, __HCarg_p)					\
>> +do {																		\
>> +	u8 __hc_delay = 1;														\
>> +	int __ret;																\
>> +	while (unlikely((__HCarg_p)->status == GNTST_eagain &&
__hc_delay))
>> {	\
>> +		msleep(__hc_delay++);												\
>> +		__ret = HYPERVISOR_grant_table_op(__HCop, (__HCarg_p), 1);			\
>> +		BUG_ON(__ret);														\
>> +	}																		\
>> +	if (__hc_delay == 0) {													\
>> +		printk(KERN_ERR "%s: %s gnt busy\n", __func__,
current->comm);		\
>> +		(__HCarg_p)->status = GNTST_bad_page;								\
>> +	}																		\
>> +	if ((__HCarg_p)->status != GNTST_okay)									\
>> +		printk(KERN_ERR "%s: %s gnt status %x\n", 							\
>> +			__func__, current->comm, (__HCarg_p)->status);					\
>> +} while(0)
>> +
>> +#define gnttab_check_GNTST_eagain_do_while(__HCop, __HCarg_p)			\
>> +do {																	\
>> +	u8 __hc_delay = 1;													\
>> +	int __ret;															\
>> +	do {																\
>> +		__ret = HYPERVISOR_grant_table_op(__HCop, (__HCarg_p), 1);		\
>> +		BUG_ON(__ret);													\
>> +		if ((__HCarg_p)->status == GNTST_eagain)						\
>> +			msleep(__hc_delay++);										\
>> +	} while ((__HCarg_p)->status == GNTST_eagain &&
__hc_delay);		\
>> +	if (__hc_delay == 0) {												\
>> +		printk(KERN_ERR "%s: %s gnt busy\n", __func__,
current->comm);	\
>> +		(__HCarg_p)->status = GNTST_bad_page;							\
>> +	}																	\
>> +	if ((__HCarg_p)->status != GNTST_okay)								\
>> +		printk(KERN_ERR "%s: %s gnt status %x\n", 						\
>> +			__func__, current->comm, (__HCarg_p)->status);				\
>> +} while(0)
>> +
>>  #endif /* __ASM_GNTTAB_H__ */
>>
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@lists.xensource.com
>> http://lists.xensource.com/xen-devel
>

Andres Lagar-Cavilla

2011-Dec-02 15:31 UTC

head link

Re: [PATCH 1 of 2] xen/x86: make __direct_remap_pfn_range()''s return value meaningful

>  >>> On 01.12.11 at 21:51, Andres Lagar-Cavilla
<andres@lagarcavilla.org>
> wrote:
>> arch/x86/mm/ioremap-xen.c |  12 ++++--------
>>  1 files changed, 4 insertions(+), 8 deletions(-)
>>
>>
>> This change fixes the xc_map_foreign_bulk interface, which would
>> otherwise cause SIGBUS when pages are gone because -ENOENT is not
>> returned as expected by the IOCTL_PRIVCMD_MMAPBATCH_V2 ioctl.
>>
>> Signed-off-by: Jan Beulich <jbeulich@novell.com>
>
> You lost authorship here.I used this:
http://xenbits.xen.org/hg/linux-2.6.18-xen.hg/rev/0051d294bb60
Andres>
> Jan
>
>

Jan Beulich

2011-Dec-02 15:44 UTC

head link

Re: [PATCH 1 of 2] xen/x86: make __direct_remap_pfn_range()''s return value meaningful

>>> On 02.12.11 at 16:31, "Andres Lagar-Cavilla"
<andres@lagarcavilla.org> wrote:
>>   >>> On 01.12.11 at 21:51, Andres Lagar-Cavilla
<andres@lagarcavilla.org>
>> wrote:
>>> arch/x86/mm/ioremap-xen.c |  12 ++++--------
>>>  1 files changed, 4 insertions(+), 8 deletions(-)
>>>
>>>
>>> This change fixes the xc_map_foreign_bulk interface, which would
>>> otherwise cause SIGBUS when pages are gone because -ENOENT is not
>>> returned as expected by the IOCTL_PRIVCMD_MMAPBATCH_V2 ioctl.
>>>
>>> Signed-off-by: Jan Beulich <jbeulich@novell.com>
>>
>> You lost authorship here.
> I used this:
> http://xenbits.xen.org/hg/linux-2.6.18-xen.hg/rev/0051d294bb60 
And you dropped the "From: Olaf Hering <ohering@novell.com>"
line.

Jan

Andres Lagar-Cavilla

2011-Dec-02 15:51 UTC

head link

Re: [PATCH 1 of 2] xen/x86: make __direct_remap_pfn_range()''s return value meaningful

>>>> On 02.12.11 at 16:31, "Andres Lagar-Cavilla"
<andres@lagarcavilla.org>
>>>> wrote:
>>>   >>> On 01.12.11 at 21:51, Andres Lagar-Cavilla
>>> <andres@lagarcavilla.org>
>>> wrote:
>>>> arch/x86/mm/ioremap-xen.c |  12 ++++--------
>>>>  1 files changed, 4 insertions(+), 8 deletions(-)
>>>>
>>>>
>>>> This change fixes the xc_map_foreign_bulk interface, which
would
>>>> otherwise cause SIGBUS when pages are gone because -ENOENT is
not
>>>> returned as expected by the IOCTL_PRIVCMD_MMAPBATCH_V2 ioctl.
>>>>
>>>> Signed-off-by: Jan Beulich <jbeulich@novell.com>
>>>
>>> You lost authorship here.
>> I used this:
>> http://xenbits.xen.org/hg/linux-2.6.18-xen.hg/rev/0051d294bb60
>
> And you dropped the "From: Olaf Hering
<ohering@novell.com>" line.Righto ... very sorry. I can resend ... if this gets traction.
Andres>
> Jan
>
>

Konrad Rzeszutek Wilk

2011-Dec-02 16:50 UTC

head link

Re: [PATCH 2 of 2] xenpaging: handle GNTST_eagain in kernel drivers

On Fri, Dec 02, 2011 at 07:27:20AM -0800, Andres Lagar-Cavilla
wrote:> > On Thu, Dec 01, 2011 at 03:51:55PM -0500, Andres Lagar-Cavilla wrote:
> >>  drivers/xen/blkback/blkback.c              |   6 ++-
> >>  drivers/xen/blkback/interface.c            |   9 +++-
> >>  drivers/xen/core/gnttab.c                  |   4 +-
> >>  drivers/xen/gntdev/gntdev.c                |  49
> >> +++++++++++++++++------------
> >>  drivers/xen/netback/interface.c            |   5 +-
> >>  drivers/xen/netback/netback.c              |  16 ++++++---
> >>  drivers/xen/scsiback/interface.c           |  10 +++---
> >>  drivers/xen/scsiback/scsiback.c            |   4 +-
> >>  drivers/xen/tpmback/interface.c            |   7 +--
> >>  drivers/xen/tpmback/tpmback.c              |  20 ++++-------
> >>  drivers/xen/usbback/interface.c            |  16 ++++----
> >>  drivers/xen/usbback/usbback.c              |   4 +-
> >>  drivers/xen/xenbus/xenbus_backend_client.c |  10 +++---
> >>  include/xen/gnttab.h                       |  37
++++++++++++++++++++++
> >>  14 files changed, 126 insertions(+), 71 deletions(-)
> >>
> >>
> >> Handle GNTST_eagain status from GNTTABOP_map_grant_ref and
> >> GNTTABOP_copy operations properly to allow usage of xenpaging
without
> >> causing crashes or data corruption.
> >>
> >> Replace all relevant HYPERVISOR_grant_table_op() calls with a
retry
> >> loop. This loop is implemented as a macro to allow different
> >> GNTTABOP_* args. It will sleep up to 33 seconds and wait for the
> >> page to be paged in again.
> >>
> >> All ->status checks were updated to use the GNTST_* namespace.
All
> >> return values are converted from GNTST_* namespace to 0/-EINVAL,
since
> >> all callers did not use the actual return value.
> >
> > Any plans to do this for the pvops tree?
> Slowly but surely. Working with XCP presently, so pvops is not at the top
> of my stack. Do you want to get at a specific merge window?
Its more of a review concern. Meaning that if you send for the upstream
kernel lots of folks are going to be looking at it. So the patches
will change - which means that the version upstream can be very
different from the one that you have for XCP - so in the end you will
have to deal with two different code-bases to maintain.

That can get a bit difficult to manage.

Granted not all of those drivers are in the upstream kernel so might not
make any difference.
> 
> Andres
> >
> >>
> >> Signed-off-by: Olaf Hering <olaf@aepfle.de>
> >> Acked-by: Patrick Colp <pjcolp@cs.ubc.ca>
> >>
> >> This is a port from xenlinux 2.6.18 to the 2.6.32 xcp tree
> >> Signed-off-by: Andres Lagar-Cavilla
<andres@lagarcavilla.org>
> >>
> >> diff -r df9e25a0189b -r 1170bc32db41 drivers/xen/blkback/blkback.c
> >> --- a/drivers/xen/blkback/blkback.c
> >> +++ b/drivers/xen/blkback/blkback.c
> >> @@ -701,11 +701,13 @@ static void dispatch_rw_block_io(blkif_t
> >>  	BUG_ON(ret);
> >>
> >>  	for (i = 0; i < nseg; i++) {
> >> -		if (unlikely(map[i].status != 0)) {
> >> +		if (unlikely(map[i].status == GNTST_eagain))
> >> +			gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref,
&map[i]);
> >> +		if (unlikely(map[i].status != GNTST_okay)) {
> >>  			DPRINTK("grant map of dom %u gref %u failed: status
%d\n",
> >>  				blkif->domid, req->seg[i].gref, map[i].status);
> >>  			map[i].handle = BLKBACK_INVALID_HANDLE;
> >> -			ret |= 1;
> >> +			ret = 1;
> >>  			continue;
> >>  		}
> >>
> >> diff -r df9e25a0189b -r 1170bc32db41
drivers/xen/blkback/interface.c
> >> --- a/drivers/xen/blkback/interface.c
> >> +++ b/drivers/xen/blkback/interface.c
> >> @@ -95,7 +95,7 @@ static int map_frontend_pages(blkif_t *b
> >>  	struct vm_struct *area = blkif->blk_ring_area;
> >>  	struct gnttab_map_grant_ref op[BLKIF_MAX_RING_PAGES];
> >>  	unsigned int i;
> >> -	int status = 0;
> >> +	int status = GNTST_okay;
> >>
> >>  	for (i = 0; i < nr_shared_pages; i++) {
> >>  		unsigned long addr = (unsigned long)area->addr +
> >> @@ -110,7 +110,10 @@ static int map_frontend_pages(blkif_t *b
> >>  		BUG();
> >>
> >>  	for (i = 0; i < nr_shared_pages; i++) {
> >> -		if ((status = op[i].status) != 0) {
> >> +		if (op[i].status == GNTST_eagain)
> >> +			gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref,
&op[i]);
> >> +		if (op[i].status != GNTST_okay) {
> >> +			status = op[i].status;
> >>  			blkif->shmem_handle[i] = INVALID_GRANT_HANDLE;
> >>  			continue;
> >>  		}
> >> @@ -120,7 +123,7 @@ static int map_frontend_pages(blkif_t *b
> >>
> >>  	blkif->nr_shared_pages = nr_shared_pages;
> >>
> >> -	if (status != 0) {
> >> +	if (status != GNTST_okay) {
> >>  		DPRINTK(" Grant table operation failure !\n");
> >>  		unmap_frontend_pages(blkif);
> >>  	}
> >> diff -r df9e25a0189b -r 1170bc32db41 drivers/xen/core/gnttab.c
> >> --- a/drivers/xen/core/gnttab.c
> >> +++ b/drivers/xen/core/gnttab.c
> >> @@ -773,7 +773,7 @@ static int gnttab_map(unsigned int start
> >>  		return -ENOSYS;
> >>  	}
> >>
> >> -	BUG_ON(rc || setup.status);
> >> +	BUG_ON(rc || (setup.status != GNTST_okay));
> >>
> >>  	if (shared.raw == NULL)
> >>  		shared.raw = arch_gnttab_alloc_shared(gframes);
> >> @@ -901,7 +901,7 @@ int gnttab_copy_grant_page(grant_ref_t r
> >>  	err = HYPERVISOR_grant_table_op(GNTTABOP_unmap_and_replace,
> >>  					&unmap, 1);
> >>  	BUG_ON(err);
> >> -	BUG_ON(unmap.status);
> >> +	BUG_ON(unmap.status != GNTST_okay);
> >>
> >>  	write_sequnlock_irq(&gnttab_dma_lock);
> >>
> >> diff -r df9e25a0189b -r 1170bc32db41 drivers/xen/gntdev/gntdev.c
> >> --- a/drivers/xen/gntdev/gntdev.c
> >> +++ b/drivers/xen/gntdev/gntdev.c
> >> @@ -503,7 +503,7 @@ static int gntdev_mmap (struct file *fli
> >>  	uint64_t ptep;
> >>  	int ret;
> >>  	int flags;
> >> -	int i;
> >> +	int i, exit_ret;
> >>  	struct page *page;
> >>  	gntdev_file_private_data_t *private_data =
flip->private_data;
> >>
> >> @@ -578,6 +578,7 @@ static int gntdev_mmap (struct file *fli
> >>  	vma->vm_mm->context.has_foreign_mappings = 1;
> >>  #endif
> >>
> >> +	exit_ret = -ENOMEM;
> >>  	for (i = 0; i < size; ++i) {
> >>
> >>  		flags = GNTMAP_host_map;
> >> @@ -598,14 +599,18 @@ static int gntdev_mmap (struct file *fli
> >>  		ret = HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref,
> >>  						&op, 1);
> >>  		BUG_ON(ret);
> >> -		if (op.status) {
> >> -			printk(KERN_ERR "Error mapping the grant reference "
> >> -			       "into the kernel (%d). domid = %d; ref =
%d\n",
> >> -			       op.status,
> >> -			       private_data->grants[slot_index+i]
> >> -			       .u.valid.domid,
> >> -			       private_data->grants[slot_index+i]
> >> -			       .u.valid.ref);
> >> +		if (op.status != GNTST_okay) {
> >> +			if (op.status != GNTST_eagain)
> >> +				printk(KERN_ERR "Error mapping the grant reference
"
> >> +					   "into the kernel (%d). domid = %d; ref = %d\n",
> >> +					   op.status,
> >> +					   private_data->grants[slot_index+i]
> >> +					   .u.valid.domid,
> >> +					   private_data->grants[slot_index+i]
> >> +					   .u.valid.ref);
> >> +			else
> >> +				/* Propagate instead of trying to fix it up */
> >> +				exit_ret = -EAGAIN;
> >>  			goto undo_map_out;
> >>  		}
> >>
> >> @@ -674,14 +679,17 @@ static int gntdev_mmap (struct file *fli
> >>  			ret = HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref,
> >>  							&op, 1);
> >>  			BUG_ON(ret);
> >> -			if (op.status) {
> >> +			if (op.status != GNTST_okay) {
> >>  				printk(KERN_ERR "Error mapping the grant "
> >> -				       "reference into user space (%d). domid "
> >> -				       "= %d; ref = %d\n", op.status,
> >> -				       private_data->grants[slot_index+i].u
> >> -				       .valid.domid,
> >> -				       private_data->grants[slot_index+i].u
> >> -				       .valid.ref);
> >> +					   "reference into user space (%d). domid "
> >> +					   "= %d; ref = %d\n", op.status,
> >> +					   private_data->grants[slot_index+i].u
> >> +					   .valid.domid,
> >> +					   private_data->grants[slot_index+i].u
> >> +					   .valid.ref);
> >> +				/* GNTST_eagain (i.e. page paged out) sohuld never happen
> >> +				 * once we''ve mapped into kernel space */
> >> +				BUG_ON(op.status == GNTST_eagain);
> >>  				goto undo_map_out;
> >>  			}
> >>
> >> @@ -705,9 +713,10 @@ static int gntdev_mmap (struct file *fli
> >>  		}
> >>
> >>  	}
> >> +	exit_ret = 0;
> >>
> >>  	up_write(&private_data->grants_sem);
> >> -	return 0;
> >> +	return exit_ret;
> >>
> >>  undo_map_out:
> >>  	/* If we have a mapping failure, the unmapping will be taken
care of
> >> @@ -725,7 +734,7 @@ undo_map_out:
> >>
> >>  	up_write(&private_data->grants_sem);
> >>
> >> -	return -ENOMEM;
> >> +	return exit_ret;
> >>  }
> >>
> >>  static pte_t gntdev_clear_pte(struct vm_area_struct *vma,
unsigned long
> >> addr,
> >> @@ -777,7 +786,7 @@ static pte_t gntdev_clear_pte(struct vm_
> >>  			ret = HYPERVISOR_grant_table_op(
> >>  				GNTTABOP_unmap_grant_ref, &op, 1);
> >>  			BUG_ON(ret);
> >> -			if (op.status)
> >> +			if (op.status != GNTST_okay)
> >>  				printk("User unmap grant status = %d\n",
> >>  				       op.status);
> >>  		} else {
> >> @@ -794,7 +803,7 @@ static pte_t gntdev_clear_pte(struct vm_
> >>  		ret = HYPERVISOR_grant_table_op(GNTTABOP_unmap_grant_ref,
> >>  						&op, 1);
> >>  		BUG_ON(ret);
> >> -		if (op.status)
> >> +		if (op.status != GNTST_okay)
> >>  			printk("Kernel unmap grant status = %d\n",
op.status);
> >>
> >>
> >> diff -r df9e25a0189b -r 1170bc32db41
drivers/xen/netback/interface.c
> >> --- a/drivers/xen/netback/interface.c
> >> +++ b/drivers/xen/netback/interface.c
> >> @@ -381,10 +381,9 @@ static int map_frontend_pages(struct xen
> >>  	gnttab_set_map_op(&op, (unsigned
long)comms->ring_area->addr,
> >>  			  GNTMAP_host_map, ring_ref, domid);
> >>
> >> -	if (HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, &op,
1))
> >> -		BUG();
> >> +	gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref,
&op);
> >>
> >> -	if (op.status) {
> >> +	if (op.status != GNTST_okay) {
> >>  		DPRINTK(" Gnttab failure mapping ring_ref!\n");
> >>  		free_vm_area(comms->ring_area);
> >>  		return op.status;
> >> diff -r df9e25a0189b -r 1170bc32db41 drivers/xen/netback/netback.c
> >> --- a/drivers/xen/netback/netback.c
> >> +++ b/drivers/xen/netback/netback.c
> >> @@ -419,11 +419,13 @@ static int netbk_check_gop(int nr_copy_s
> >>
> >>  	for (i = 0; i < nr_copy_slots; i++) {
> >>  		copy_op = npo->copy + npo->copy_cons++;
> >> +		if (copy_op->status == GNTST_eagain)
> >> +			gnttab_check_GNTST_eagain_while(GNTTABOP_copy, copy_op);
> >>  		if (copy_op->status != GNTST_okay) {
> >> -				DPRINTK("Bad status %d from copy to DOM%d.\n",
> >> -					copy_op->status, domid);
> >> -				status = NETIF_RSP_ERROR;
> >> -			}
> >> +			DPRINTK("Bad status %d from copy to DOM%d.\n",
> >> +				copy_op->status, domid);
> >> +			status = NETIF_RSP_ERROR;
> >> +		}
> >>  	}
> >>
> >>  	return status;
> >> @@ -1020,7 +1022,11 @@ static int netbk_tx_check_mop(struct xen
> >>
> >>  	/* Check status of header. */
> >>  	err = mop->status;
> >> -	if (unlikely(err)) {
> >> +	if (err == GNTST_eagain) {
> >> +		gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref,
mop);
> >> +		err = mop->status;
> >> +	}
> >> +	if (unlikely(err != GNTST_okay)) {
> >>  		pending_ring_idx_t index;
> >>  		index = pending_index(netbk->pending_prod++);
> >>  		txp = &pending_tx_info[pending_idx].req;
> >> diff -r df9e25a0189b -r 1170bc32db41
drivers/xen/scsiback/interface.c
> >> --- a/drivers/xen/scsiback/interface.c
> >> +++ b/drivers/xen/scsiback/interface.c
> >> @@ -69,18 +69,18 @@ static int map_frontend_page( struct vsc
> >>  				GNTMAP_host_map, ring_ref,
> >>  				info->domid);
> >>
> >> -	err = HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, &op,
1);
> >> -	BUG_ON(err);
> >> +	gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref,
&op);
> >>
> >> -	if (op.status) {
> >> -		printk(KERN_ERR "scsiback: Grant table operation failure
!\n");
> >> +	if (op.status != GNTST_okay) {
> >> +		printk(KERN_ERR "scsiback: Grant table operation failure
%d!\n",
> >> +							(int) op.status);
> >>  		return op.status;
> >>  	}
> >>
> >>  	info->shmem_ref    = ring_ref;
> >>  	info->shmem_handle = op.handle;
> >>
> >> -	return (GNTST_okay);
> >> +	return 0;
> >>  }
> >>
> >>  static void unmap_frontend_page(struct vscsibk_info *info)
> >> diff -r df9e25a0189b -r 1170bc32db41
drivers/xen/scsiback/scsiback.c
> >> --- a/drivers/xen/scsiback/scsiback.c
> >> +++ b/drivers/xen/scsiback/scsiback.c
> >> @@ -287,7 +287,9 @@ static int scsiback_gnttab_data_map(vscs
> >>  		for_each_sg (pending_req->sgl, sg, nr_segments, i) {
> >>  			struct page *pg;
> >>
> >> -			if (unlikely(map[i].status != 0)) {
> >> +			if (unlikely(map[i].status == GNTST_eagain))
> >> +				gnttab_check_GNTST_eagain_while(GNTTABOP_map_grant_ref,
&map[i]);
> >> +			if (unlikely(map[i].status != GNTST_okay)) {
> >>  				printk(KERN_ERR "scsiback: invalid buffer -- could not
remap
> >> it\n");
> >>  				map[i].handle = SCSIBACK_INVALID_HANDLE;
> >>  				err |= 1;
> >> diff -r df9e25a0189b -r 1170bc32db41
drivers/xen/tpmback/interface.c
> >> --- a/drivers/xen/tpmback/interface.c
> >> +++ b/drivers/xen/tpmback/interface.c
> >> @@ -86,11 +86,10 @@ static int map_frontend_page(tpmif_t *tp
> >>  	gnttab_set_map_op(&op, (unsigned
long)tpmif->tx_area->addr,
> >>  			  GNTMAP_host_map, shared_page, tpmif->domid);
> >>
> >> -	if (HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, &op,
1))
> >> -		BUG();
> >> +	gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref,
&op);
> >>
> >> -	if (op.status) {
> >> -		DPRINTK(" Grant table operation failure !\n");
> >> +	if (op.status != GNTST_okay) {
> >> +		DPRINTK(" Grant table operation failure %d!\n", (int)
op.status);
> >>  		return op.status;
> >>  	}
> >>
> >> diff -r df9e25a0189b -r 1170bc32db41 drivers/xen/tpmback/tpmback.c
> >> --- a/drivers/xen/tpmback/tpmback.c
> >> +++ b/drivers/xen/tpmback/tpmback.c
> >> @@ -256,15 +256,13 @@ int _packet_write(struct packet *pak,
> >>  		gnttab_set_map_op(&map_op, idx_to_kaddr(tpmif, i),
> >>  				  GNTMAP_host_map, tx->ref, tpmif->domid);
> >>
> >> -		if (unlikely(HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref,
> >> -						       &map_op, 1))) {
> >> -			BUG();
> >> -		}
> >> +		gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref,
&map_op);
> >>
> >>  		handle = map_op.handle;
> >>
> >> -		if (map_op.status) {
> >> -			DPRINTK(" Grant table operation failure !\n");
> >> +		if (map_op.status != GNTST_okay) {
> >> +			DPRINTK(" Grant table operation failure %d!\n",
> >> +						(int) map_op.status);
> >>  			return 0;
> >>  		}
> >>
> >> @@ -394,13 +392,11 @@ static int packet_read_shmem(struct pack
> >>  		gnttab_set_map_op(&map_op, idx_to_kaddr(tpmif, i),
> >>  				  GNTMAP_host_map, tx->ref, tpmif->domid);
> >>
> >> -		if (unlikely(HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref,
> >> -						       &map_op, 1))) {
> >> -			BUG();
> >> -		}
> >> +		gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref,
&map_op);
> >>
> >> -		if (map_op.status) {
> >> -			DPRINTK(" Grant table operation failure !\n");
> >> +		if (map_op.status != GNTST_okay) {
> >> +			DPRINTK(" Grant table operation failure %d!\n",
> >> +						(int) map_op.status);
> >>  			return -EFAULT;
> >>  		}
> >>
> >> diff -r df9e25a0189b -r 1170bc32db41
drivers/xen/usbback/interface.c
> >> --- a/drivers/xen/usbback/interface.c
> >> +++ b/drivers/xen/usbback/interface.c
> >> @@ -109,11 +109,11 @@ static int map_frontend_pages(usbif_t *u
> >>  	gnttab_set_map_op(&op, (unsigned
long)usbif->urb_ring_area->addr,
> >>  			  GNTMAP_host_map, urb_ring_ref, usbif->domid);
> >>
> >> -	if (HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, &op,
1))
> >> -		BUG();
> >> +	gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref,
&op);
> >>
> >> -	if (op.status) {
> >> -		printk(KERN_ERR "grant table failure mapping
urb_ring_ref\n");
> >> +	if (op.status != GNTST_okay) {
> >> +		printk(KERN_ERR "grant table failure mapping urb_ring_ref
%d\n",
> >> +							(int) op.status);
> >>  		return op.status;
> >>  	}
> >>
> >> @@ -123,17 +123,17 @@ static int map_frontend_pages(usbif_t *u
> >>  	gnttab_set_map_op(&op, (unsigned
long)usbif->conn_ring_area->addr,
> >>  			  GNTMAP_host_map, conn_ring_ref, usbif->domid);
> >>
> >> -	if (HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, &op,
1))
> >> -		BUG();
> >> +	gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref,
&op);
> >>
> >> -	if (op.status) {
> >> +	if (op.status != GNTST_okay) {
> >>  		struct gnttab_unmap_grant_ref unop;
> >>  		gnttab_set_unmap_op(&unop,
> >>  				(unsigned long) usbif->urb_ring_area->addr,
> >>  				GNTMAP_host_map, usbif->urb_shmem_handle);
> >>  		VOID(HYPERVISOR_grant_table_op(GNTTABOP_unmap_grant_ref,
&unop,
> >>  				1));
> >> -		printk(KERN_ERR "grant table failure mapping
conn_ring_ref\n");
> >> +		printk(KERN_ERR "grant table failure mapping conn_ring_ref
%d\n",
> >> +							(int) op.status);
> >>  		return op.status;
> >>  	}
> >>
> >> diff -r df9e25a0189b -r 1170bc32db41 drivers/xen/usbback/usbback.c
> >> --- a/drivers/xen/usbback/usbback.c
> >> +++ b/drivers/xen/usbback/usbback.c
> >> @@ -428,7 +428,9 @@ static int usbbk_gnttab_map(usbif_t *usb
> >>  		BUG_ON(ret);
> >>
> >>  		for (i = 0; i < nr_segs; i++) {
> >> -			if (unlikely(map[i].status != 0)) {
> >> +			if (unlikely(map[i].status == GNTST_eagain))
> >> +				gnttab_check_GNTST_eagain_while(GNTTABOP_map_grant_ref,
&map[i]);
> >> +			if (unlikely(map[i].status != GNTST_okay)) {
> >>  				printk(KERN_ERR "usbback: invalid buffer -- could not
remap it\n");
> >>  				map[i].handle = USBBACK_INVALID_HANDLE;
> >>  				ret |= 1;
> >> diff -r df9e25a0189b -r 1170bc32db41
> >> drivers/xen/xenbus/xenbus_backend_client.c
> >> --- a/drivers/xen/xenbus/xenbus_backend_client.c
> >> +++ b/drivers/xen/xenbus/xenbus_backend_client.c
> >> @@ -48,8 +48,7 @@ struct vm_struct *xenbus_map_ring_valloc
> >>  	gnttab_set_map_op(&op, (unsigned long)area->addr,
GNTMAP_host_map,
> >>  			  gnt_ref, dev->otherend_id);
> >>
> >> -	if (HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, &op,
1))
> >> -		BUG();
> >> +	gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref,
&op);
> >>
> >>  	if (op.status != GNTST_okay) {
> >>  		free_vm_area(area);
> >> @@ -75,15 +74,16 @@ int xenbus_map_ring(struct xenbus_device
> >>
> >>  	gnttab_set_map_op(&op, (unsigned long)vaddr,
GNTMAP_host_map,
> >>  			  gnt_ref, dev->otherend_id);
> >> -	if (HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, &op,
1))
> >> -		BUG();
> >> +	gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref,
&op);
> >>
> >>  	if (op.status != GNTST_okay) {
> >>  		xenbus_dev_fatal(dev, op.status,
> >>  				 "mapping in shared page %d from domain %d",
> >>  				 gnt_ref, dev->otherend_id);
> >> -	} else
> >> +	} else {
> >>  		*handle = op.handle;
> >> +		return 0;
> >> +	}
> >>
> >>  	return op.status;
> >>  }
> >> diff -r df9e25a0189b -r 1170bc32db41 include/xen/gnttab.h
> >> --- a/include/xen/gnttab.h
> >> +++ b/include/xen/gnttab.h
> >> @@ -187,4 +187,41 @@ gnttab_set_replace_op(struct gnttab_unma
> >>  	unmap->handle = handle;
> >>  }
> >>
> >> +#define gnttab_check_GNTST_eagain_while(__HCop, __HCarg_p)					\
> >> +do {																		\
> >> +	u8 __hc_delay = 1;														\
> >> +	int __ret;																\
> >> +	while (unlikely((__HCarg_p)->status == GNTST_eagain
&& __hc_delay))
> >> {	\
> >> +		msleep(__hc_delay++);												\
> >> +		__ret = HYPERVISOR_grant_table_op(__HCop, (__HCarg_p), 1);			\
> >> +		BUG_ON(__ret);														\
> >> +	}																		\
> >> +	if (__hc_delay == 0) {													\
> >> +		printk(KERN_ERR "%s: %s gnt busy\n", __func__,
current->comm);		\
> >> +		(__HCarg_p)->status = GNTST_bad_page;								\
> >> +	}																		\
> >> +	if ((__HCarg_p)->status != GNTST_okay)									\
> >> +		printk(KERN_ERR "%s: %s gnt status %x\n", 							\
> >> +			__func__, current->comm, (__HCarg_p)->status);					\
> >> +} while(0)
> >> +
> >> +#define gnttab_check_GNTST_eagain_do_while(__HCop, __HCarg_p)			\
> >> +do {																	\
> >> +	u8 __hc_delay = 1;													\
> >> +	int __ret;															\
> >> +	do {																\
> >> +		__ret = HYPERVISOR_grant_table_op(__HCop, (__HCarg_p), 1);		\
> >> +		BUG_ON(__ret);													\
> >> +		if ((__HCarg_p)->status == GNTST_eagain)						\
> >> +			msleep(__hc_delay++);										\
> >> +	} while ((__HCarg_p)->status == GNTST_eagain &&
__hc_delay);		\
> >> +	if (__hc_delay == 0) {												\
> >> +		printk(KERN_ERR "%s: %s gnt busy\n", __func__,
current->comm);	\
> >> +		(__HCarg_p)->status = GNTST_bad_page;							\
> >> +	}																	\
> >> +	if ((__HCarg_p)->status != GNTST_okay)								\
> >> +		printk(KERN_ERR "%s: %s gnt status %x\n", 						\
> >> +			__func__, current->comm, (__HCarg_p)->status);				\
> >> +} while(0)
> >> +
> >>  #endif /* __ASM_GNTTAB_H__ */
> >>
> >> _______________________________________________
> >> Xen-devel mailing list
> >> Xen-devel@lists.xensource.com
> >> http://lists.xensource.com/xen-devel
> >
> 
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

Adin Scannell

2011-Dec-17 03:00 UTC

head link

Re: [PATCH 2 of 2] xenpaging: handle GNTST_eagain in kernel drivers

I had a chance to get a environment up and running with the Kernel
tip, so I''ve ported these patches.  As a requisite, I had to create a
patch that implements MMAP_BATCH_V2 first.  I''ll be posting the set to
the mailing list very shortly for comment.

I wanted to revive this thread so there was a bit of context.

Cheers,
-Adin

On Fri, Dec 2, 2011 at 11:50 AM, Konrad Rzeszutek Wilk
<konrad@darnok.org> wrote:> On Fri, Dec 02, 2011 at 07:27:20AM -0800, Andres Lagar-Cavilla wrote:
>> > On Thu, Dec 01, 2011 at 03:51:55PM -0500, Andres Lagar-Cavilla
wrote:
>> >>  drivers/xen/blkback/blkback.c              |   6 ++-
>> >>  drivers/xen/blkback/interface.c            |   9 +++-
>> >>  drivers/xen/core/gnttab.c                  |   4 +-
>> >>  drivers/xen/gntdev/gntdev.c                |  49
>> >> +++++++++++++++++------------
>> >>  drivers/xen/netback/interface.c            |   5 +-
>> >>  drivers/xen/netback/netback.c              |  16 ++++++---
>> >>  drivers/xen/scsiback/interface.c           |  10 +++---
>> >>  drivers/xen/scsiback/scsiback.c            |   4 +-
>> >>  drivers/xen/tpmback/interface.c            |   7 +--
>> >>  drivers/xen/tpmback/tpmback.c              |  20 ++++-------
>> >>  drivers/xen/usbback/interface.c            |  16 ++++----
>> >>  drivers/xen/usbback/usbback.c              |   4 +-
>> >>  drivers/xen/xenbus/xenbus_backend_client.c |  10 +++---
>> >>  include/xen/gnttab.h                       |  37
++++++++++++++++++++++
>> >>  14 files changed, 126 insertions(+), 71 deletions(-)
>> >>
>> >>
>> >> Handle GNTST_eagain status from GNTTABOP_map_grant_ref and
>> >> GNTTABOP_copy operations properly to allow usage of xenpaging
without
>> >> causing crashes or data corruption.
>> >>
>> >> Replace all relevant HYPERVISOR_grant_table_op() calls with a
retry
>> >> loop. This loop is implemented as a macro to allow different
>> >> GNTTABOP_* args. It will sleep up to 33 seconds and wait for
the
>> >> page to be paged in again.
>> >>
>> >> All ->status checks were updated to use the GNTST_*
namespace. All
>> >> return values are converted from GNTST_* namespace to
0/-EINVAL, since
>> >> all callers did not use the actual return value.
>> >
>> > Any plans to do this for the pvops tree?
>> Slowly but surely. Working with XCP presently, so pvops is not at the
top
>> of my stack. Do you want to get at a specific merge window?
>
> Its more of a review concern. Meaning that if you send for the upstream
> kernel lots of folks are going to be looking at it. So the patches
> will change - which means that the version upstream can be very
> different from the one that you have for XCP - so in the end you will
> have to deal with two different code-bases to maintain.
>
> That can get a bit difficult to manage.
>
> Granted not all of those drivers are in the upstream kernel so might not
> make any difference.
>
>>
>> Andres
>> >
>> >>
>> >> Signed-off-by: Olaf Hering <olaf@aepfle.de>
>> >> Acked-by: Patrick Colp <pjcolp@cs.ubc.ca>
>> >>
>> >> This is a port from xenlinux 2.6.18 to the 2.6.32 xcp tree
>> >> Signed-off-by: Andres Lagar-Cavilla
<andres@lagarcavilla.org>
>> >>
>> >> diff -r df9e25a0189b -r 1170bc32db41
drivers/xen/blkback/blkback.c
>> >> --- a/drivers/xen/blkback/blkback.c
>> >> +++ b/drivers/xen/blkback/blkback.c
>> >> @@ -701,11 +701,13 @@ static void dispatch_rw_block_io(blkif_t
>> >>    BUG_ON(ret);
>> >>
>> >>    for (i = 0; i < nseg; i++) {
>> >> -          if (unlikely(map[i].status != 0)) {
>> >> +          if (unlikely(map[i].status == GNTST_eagain))
>> >> +                
 gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref, &map[i]);
>> >> +          if (unlikely(map[i].status != GNTST_okay)) {
>> >>                    DPRINTK("grant map of dom %u gref %u
failed: status %d\n",
>> >>                            blkif->domid,
req->seg[i].gref, map[i].status);
>> >>                    map[i].handle = BLKBACK_INVALID_HANDLE;
>> >> -                  ret |= 1;
>> >> +                  ret = 1;
>> >>                    continue;
>> >>            }
>> >>
>> >> diff -r df9e25a0189b -r 1170bc32db41
drivers/xen/blkback/interface.c
>> >> --- a/drivers/xen/blkback/interface.c
>> >> +++ b/drivers/xen/blkback/interface.c
>> >> @@ -95,7 +95,7 @@ static int map_frontend_pages(blkif_t *b
>> >>    struct vm_struct *area = blkif->blk_ring_area;
>> >>    struct gnttab_map_grant_ref op[BLKIF_MAX_RING_PAGES];
>> >>    unsigned int i;
>> >> -  int status = 0;
>> >> +  int status = GNTST_okay;
>> >>
>> >>    for (i = 0; i < nr_shared_pages; i++) {
>> >>            unsigned long addr = (unsigned long)area->addr +
>> >> @@ -110,7 +110,10 @@ static int map_frontend_pages(blkif_t *b
>> >>            BUG();
>> >>
>> >>    for (i = 0; i < nr_shared_pages; i++) {
>> >> -          if ((status = op[i].status) != 0) {
>> >> +          if (op[i].status == GNTST_eagain)
>> >> +                
 gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref, &op[i]);
>> >> +          if (op[i].status != GNTST_okay) {
>> >> +                  status = op[i].status;
>> >>                    blkif->shmem_handle[i] =
INVALID_GRANT_HANDLE;
>> >>                    continue;
>> >>            }
>> >> @@ -120,7 +123,7 @@ static int map_frontend_pages(blkif_t *b
>> >>
>> >>    blkif->nr_shared_pages = nr_shared_pages;
>> >>
>> >> -  if (status != 0) {
>> >> +  if (status != GNTST_okay) {
>> >>            DPRINTK(" Grant table operation failure
!\n");
>> >>            unmap_frontend_pages(blkif);
>> >>    }
>> >> diff -r df9e25a0189b -r 1170bc32db41 drivers/xen/core/gnttab.c
>> >> --- a/drivers/xen/core/gnttab.c
>> >> +++ b/drivers/xen/core/gnttab.c
>> >> @@ -773,7 +773,7 @@ static int gnttab_map(unsigned int start
>> >>            return -ENOSYS;
>> >>    }
>> >>
>> >> -  BUG_ON(rc || setup.status);
>> >> +  BUG_ON(rc || (setup.status != GNTST_okay));
>> >>
>> >>    if (shared.raw == NULL)
>> >>            shared.raw = arch_gnttab_alloc_shared(gframes);
>> >> @@ -901,7 +901,7 @@ int gnttab_copy_grant_page(grant_ref_t r
>> >>    err = HYPERVISOR_grant_table_op(GNTTABOP_unmap_and_replace,
>> >>                                    &unmap, 1);
>> >>    BUG_ON(err);
>> >> -  BUG_ON(unmap.status);
>> >> +  BUG_ON(unmap.status != GNTST_okay);
>> >>
>> >>    write_sequnlock_irq(&gnttab_dma_lock);
>> >>
>> >> diff -r df9e25a0189b -r 1170bc32db41
drivers/xen/gntdev/gntdev.c
>> >> --- a/drivers/xen/gntdev/gntdev.c
>> >> +++ b/drivers/xen/gntdev/gntdev.c
>> >> @@ -503,7 +503,7 @@ static int gntdev_mmap (struct file *fli
>> >>    uint64_t ptep;
>> >>    int ret;
>> >>    int flags;
>> >> -  int i;
>> >> +  int i, exit_ret;
>> >>    struct page *page;
>> >>    gntdev_file_private_data_t *private_data =
flip->private_data;
>> >>
>> >> @@ -578,6 +578,7 @@ static int gntdev_mmap (struct file *fli
>> >>    vma->vm_mm->context.has_foreign_mappings = 1;
>> >>  #endif
>> >>
>> >> +  exit_ret = -ENOMEM;
>> >>    for (i = 0; i < size; ++i) {
>> >>
>> >>            flags = GNTMAP_host_map;
>> >> @@ -598,14 +599,18 @@ static int gntdev_mmap (struct file *fli
>> >>            ret =
HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref,
>> >>                                            &op, 1);
>> >>            BUG_ON(ret);
>> >> -          if (op.status) {
>> >> -                  printk(KERN_ERR "Error mapping the
grant reference "
>> >> -                         "into the kernel (%d). domid =
%d; ref = %d\n",
>> >> -                         op.status,
>> >> -                        
private_data->grants[slot_index+i]
>> >> -                         .u.valid.domid,
>> >> -                        
private_data->grants[slot_index+i]
>> >> -                         .u.valid.ref);
>> >> +          if (op.status != GNTST_okay) {
>> >> +                  if (op.status != GNTST_eagain)
>> >> +                          printk(KERN_ERR "Error mapping
the grant reference "
>> >> +                                     "into the kernel
(%d). domid = %d; ref = %d\n",
>> >> +                                     op.status,
>> >> +                                    
private_data->grants[slot_index+i]
>> >> +                                     .u.valid.domid,
>> >> +                                    
private_data->grants[slot_index+i]
>> >> +                                     .u.valid.ref);
>> >> +                  else
>> >> +                          /* Propagate instead of trying to
fix it up */
>> >> +                          exit_ret = -EAGAIN;
>> >>                    goto undo_map_out;
>> >>            }
>> >>
>> >> @@ -674,14 +679,17 @@ static int gntdev_mmap (struct file *fli
>> >>                    ret =
HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref,
>> >>                                                    &op,
1);
>> >>                    BUG_ON(ret);
>> >> -                  if (op.status) {
>> >> +                  if (op.status != GNTST_okay) {
>> >>                            printk(KERN_ERR "Error mapping
the grant "
>> >> -                                 "reference into user
space (%d). domid "
>> >> -                                 "= %d; ref =
%d\n", op.status,
>> >> -                                
private_data->grants[slot_index+i].u
>> >> -                                 .valid.domid,
>> >> -                                
private_data->grants[slot_index+i].u
>> >> -                                 .valid.ref);
>> >> +                                     "reference into
user space (%d). domid "
>> >> +                                     "= %d; ref =
%d\n", op.status,
>> >> +                                    
private_data->grants[slot_index+i].u
>> >> +                                     .valid.domid,
>> >> +                                    
private_data->grants[slot_index+i].u
>> >> +                                     .valid.ref);
>> >> +                          /* GNTST_eagain (i.e. page paged
out) sohuld never happen
>> >> +                           * once we''ve mapped into
kernel space */
>> >> +                          BUG_ON(op.status == GNTST_eagain);
>> >>                            goto undo_map_out;
>> >>                    }
>> >>
>> >> @@ -705,9 +713,10 @@ static int gntdev_mmap (struct file *fli
>> >>            }
>> >>
>> >>    }
>> >> +  exit_ret = 0;
>> >>
>> >>    up_write(&private_data->grants_sem);
>> >> -  return 0;
>> >> +  return exit_ret;
>> >>
>> >>  undo_map_out:
>> >>    /* If we have a mapping failure, the unmapping will be
taken care of
>> >> @@ -725,7 +734,7 @@ undo_map_out:
>> >>
>> >>    up_write(&private_data->grants_sem);
>> >>
>> >> -  return -ENOMEM;
>> >> +  return exit_ret;
>> >>  }
>> >>
>> >>  static pte_t gntdev_clear_pte(struct vm_area_struct *vma,
unsigned long
>> >> addr,
>> >> @@ -777,7 +786,7 @@ static pte_t gntdev_clear_pte(struct vm_
>> >>                    ret = HYPERVISOR_grant_table_op(
>> >>                            GNTTABOP_unmap_grant_ref, &op,
1);
>> >>                    BUG_ON(ret);
>> >> -                  if (op.status)
>> >> +                  if (op.status != GNTST_okay)
>> >>                            printk("User unmap grant
status = %d\n",
>> >>                                   op.status);
>> >>            } else {
>> >> @@ -794,7 +803,7 @@ static pte_t gntdev_clear_pte(struct vm_
>> >>            ret =
HYPERVISOR_grant_table_op(GNTTABOP_unmap_grant_ref,
>> >>                                            &op, 1);
>> >>            BUG_ON(ret);
>> >> -          if (op.status)
>> >> +          if (op.status != GNTST_okay)
>> >>                    printk("Kernel unmap grant status =
%d\n", op.status);
>> >>
>> >>
>> >> diff -r df9e25a0189b -r 1170bc32db41
drivers/xen/netback/interface.c
>> >> --- a/drivers/xen/netback/interface.c
>> >> +++ b/drivers/xen/netback/interface.c
>> >> @@ -381,10 +381,9 @@ static int map_frontend_pages(struct xen
>> >>    gnttab_set_map_op(&op, (unsigned
long)comms->ring_area->addr,
>> >>                      GNTMAP_host_map, ring_ref, domid);
>> >>
>> >> -  if (HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref,
&op, 1))
>> >> -          BUG();
>> >> +  gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref,
&op);
>> >>
>> >> -  if (op.status) {
>> >> +  if (op.status != GNTST_okay) {
>> >>            DPRINTK(" Gnttab failure mapping
ring_ref!\n");
>> >>            free_vm_area(comms->ring_area);
>> >>            return op.status;
>> >> diff -r df9e25a0189b -r 1170bc32db41
drivers/xen/netback/netback.c
>> >> --- a/drivers/xen/netback/netback.c
>> >> +++ b/drivers/xen/netback/netback.c
>> >> @@ -419,11 +419,13 @@ static int netbk_check_gop(int nr_copy_s
>> >>
>> >>    for (i = 0; i < nr_copy_slots; i++) {
>> >>            copy_op = npo->copy + npo->copy_cons++;
>> >> +          if (copy_op->status == GNTST_eagain)
>> >> +                
 gnttab_check_GNTST_eagain_while(GNTTABOP_copy, copy_op);
>> >>            if (copy_op->status != GNTST_okay) {
>> >> -                          DPRINTK("Bad status %d from
copy to DOM%d.\n",
>> >> -                                  copy_op->status, domid);
>> >> -                          status = NETIF_RSP_ERROR;
>> >> -                  }
>> >> +                  DPRINTK("Bad status %d from copy to
DOM%d.\n",
>> >> +                          copy_op->status, domid);
>> >> +                  status = NETIF_RSP_ERROR;
>> >> +          }
>> >>    }
>> >>
>> >>    return status;
>> >> @@ -1020,7 +1022,11 @@ static int netbk_tx_check_mop(struct
xen
>> >>
>> >>    /* Check status of header. */
>> >>    err = mop->status;
>> >> -  if (unlikely(err)) {
>> >> +  if (err == GNTST_eagain) {
>> >> +        
 gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref, mop);
>> >> +          err = mop->status;
>> >> +  }
>> >> +  if (unlikely(err != GNTST_okay)) {
>> >>            pending_ring_idx_t index;
>> >>            index = pending_index(netbk->pending_prod++);
>> >>            txp = &pending_tx_info[pending_idx].req;
>> >> diff -r df9e25a0189b -r 1170bc32db41
drivers/xen/scsiback/interface.c
>> >> --- a/drivers/xen/scsiback/interface.c
>> >> +++ b/drivers/xen/scsiback/interface.c
>> >> @@ -69,18 +69,18 @@ static int map_frontend_page( struct vsc
>> >>                            GNTMAP_host_map, ring_ref,
>> >>                            info->domid);
>> >>
>> >> -  err = HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref,
&op, 1);
>> >> -  BUG_ON(err);
>> >> +  gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref,
&op);
>> >>
>> >> -  if (op.status) {
>> >> -          printk(KERN_ERR "scsiback: Grant table
operation failure !\n");
>> >> +  if (op.status != GNTST_okay) {
>> >> +          printk(KERN_ERR "scsiback: Grant table
operation failure %d!\n",
>> >> +                                                  (int)
op.status);
>> >>            return op.status;
>> >>    }
>> >>
>> >>    info->shmem_ref    = ring_ref;
>> >>    info->shmem_handle = op.handle;
>> >>
>> >> -  return (GNTST_okay);
>> >> +  return 0;
>> >>  }
>> >>
>> >>  static void unmap_frontend_page(struct vscsibk_info *info)
>> >> diff -r df9e25a0189b -r 1170bc32db41
drivers/xen/scsiback/scsiback.c
>> >> --- a/drivers/xen/scsiback/scsiback.c
>> >> +++ b/drivers/xen/scsiback/scsiback.c
>> >> @@ -287,7 +287,9 @@ static int scsiback_gnttab_data_map(vscs
>> >>            for_each_sg (pending_req->sgl, sg, nr_segments,
i) {
>> >>                    struct page *pg;
>> >>
>> >> -                  if (unlikely(map[i].status != 0)) {
>> >> +                  if (unlikely(map[i].status ==
GNTST_eagain))
>> >> +                        
 gnttab_check_GNTST_eagain_while(GNTTABOP_map_grant_ref, &map[i]);
>> >> +                  if (unlikely(map[i].status != GNTST_okay))
{
>> >>                            printk(KERN_ERR "scsiback:
invalid buffer -- could not remap
>> >> it\n");
>> >>                            map[i].handle =
SCSIBACK_INVALID_HANDLE;
>> >>                            err |= 1;
>> >> diff -r df9e25a0189b -r 1170bc32db41
drivers/xen/tpmback/interface.c
>> >> --- a/drivers/xen/tpmback/interface.c
>> >> +++ b/drivers/xen/tpmback/interface.c
>> >> @@ -86,11 +86,10 @@ static int map_frontend_page(tpmif_t *tp
>> >>    gnttab_set_map_op(&op, (unsigned
long)tpmif->tx_area->addr,
>> >>                      GNTMAP_host_map, shared_page,
tpmif->domid);
>> >>
>> >> -  if (HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref,
&op, 1))
>> >> -          BUG();
>> >> +  gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref,
&op);
>> >>
>> >> -  if (op.status) {
>> >> -          DPRINTK(" Grant table operation failure
!\n");
>> >> +  if (op.status != GNTST_okay) {
>> >> +          DPRINTK(" Grant table operation failure
%d!\n", (int) op.status);
>> >>            return op.status;
>> >>    }
>> >>
>> >> diff -r df9e25a0189b -r 1170bc32db41
drivers/xen/tpmback/tpmback.c
>> >> --- a/drivers/xen/tpmback/tpmback.c
>> >> +++ b/drivers/xen/tpmback/tpmback.c
>> >> @@ -256,15 +256,13 @@ int _packet_write(struct packet *pak,
>> >>            gnttab_set_map_op(&map_op, idx_to_kaddr(tpmif,
i),
>> >>                              GNTMAP_host_map, tx->ref,
tpmif->domid);
>> >>
>> >> -          if
(unlikely(HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref,
>> >> -                                                 &map_op,
1))) {
>> >> -                  BUG();
>> >> -          }
>> >> +        
 gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref, &map_op);
>> >>
>> >>            handle = map_op.handle;
>> >>
>> >> -          if (map_op.status) {
>> >> -                  DPRINTK(" Grant table operation
failure !\n");
>> >> +          if (map_op.status != GNTST_okay) {
>> >> +                  DPRINTK(" Grant table operation
failure %d!\n",
>> >> +                                          (int)
map_op.status);
>> >>                    return 0;
>> >>            }
>> >>
>> >> @@ -394,13 +392,11 @@ static int packet_read_shmem(struct pack
>> >>            gnttab_set_map_op(&map_op, idx_to_kaddr(tpmif,
i),
>> >>                              GNTMAP_host_map, tx->ref,
tpmif->domid);
>> >>
>> >> -          if
(unlikely(HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref,
>> >> -                                                 &map_op,
1))) {
>> >> -                  BUG();
>> >> -          }
>> >> +        
 gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref, &map_op);
>> >>
>> >> -          if (map_op.status) {
>> >> -                  DPRINTK(" Grant table operation
failure !\n");
>> >> +          if (map_op.status != GNTST_okay) {
>> >> +                  DPRINTK(" Grant table operation
failure %d!\n",
>> >> +                                          (int)
map_op.status);
>> >>                    return -EFAULT;
>> >>            }
>> >>
>> >> diff -r df9e25a0189b -r 1170bc32db41
drivers/xen/usbback/interface.c
>> >> --- a/drivers/xen/usbback/interface.c
>> >> +++ b/drivers/xen/usbback/interface.c
>> >> @@ -109,11 +109,11 @@ static int map_frontend_pages(usbif_t *u
>> >>    gnttab_set_map_op(&op, (unsigned
long)usbif->urb_ring_area->addr,
>> >>                      GNTMAP_host_map, urb_ring_ref,
usbif->domid);
>> >>
>> >> -  if (HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref,
&op, 1))
>> >> -          BUG();
>> >> +  gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref,
&op);
>> >>
>> >> -  if (op.status) {
>> >> -          printk(KERN_ERR "grant table failure mapping
urb_ring_ref\n");
>> >> +  if (op.status != GNTST_okay) {
>> >> +          printk(KERN_ERR "grant table failure mapping
urb_ring_ref %d\n",
>> >> +                                                  (int)
op.status);
>> >>            return op.status;
>> >>    }
>> >>
>> >> @@ -123,17 +123,17 @@ static int map_frontend_pages(usbif_t *u
>> >>    gnttab_set_map_op(&op, (unsigned
long)usbif->conn_ring_area->addr,
>> >>                      GNTMAP_host_map, conn_ring_ref,
usbif->domid);
>> >>
>> >> -  if (HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref,
&op, 1))
>> >> -          BUG();
>> >> +  gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref,
&op);
>> >>
>> >> -  if (op.status) {
>> >> +  if (op.status != GNTST_okay) {
>> >>            struct gnttab_unmap_grant_ref unop;
>> >>            gnttab_set_unmap_op(&unop,
>> >>                            (unsigned long)
usbif->urb_ring_area->addr,
>> >>                            GNTMAP_host_map,
usbif->urb_shmem_handle);
>> >>          
 VOID(HYPERVISOR_grant_table_op(GNTTABOP_unmap_grant_ref, &unop,
>> >>                            1));
>> >> -          printk(KERN_ERR "grant table failure mapping
conn_ring_ref\n");
>> >> +          printk(KERN_ERR "grant table failure mapping
conn_ring_ref %d\n",
>> >> +                                                  (int)
op.status);
>> >>            return op.status;
>> >>    }
>> >>
>> >> diff -r df9e25a0189b -r 1170bc32db41
drivers/xen/usbback/usbback.c
>> >> --- a/drivers/xen/usbback/usbback.c
>> >> +++ b/drivers/xen/usbback/usbback.c
>> >> @@ -428,7 +428,9 @@ static int usbbk_gnttab_map(usbif_t *usb
>> >>            BUG_ON(ret);
>> >>
>> >>            for (i = 0; i < nr_segs; i++) {
>> >> -                  if (unlikely(map[i].status != 0)) {
>> >> +                  if (unlikely(map[i].status ==
GNTST_eagain))
>> >> +                        
 gnttab_check_GNTST_eagain_while(GNTTABOP_map_grant_ref, &map[i]);
>> >> +                  if (unlikely(map[i].status != GNTST_okay))
{
>> >>                            printk(KERN_ERR "usbback:
invalid buffer -- could not remap it\n");
>> >>                            map[i].handle =
USBBACK_INVALID_HANDLE;
>> >>                            ret |= 1;
>> >> diff -r df9e25a0189b -r 1170bc32db41
>> >> drivers/xen/xenbus/xenbus_backend_client.c
>> >> --- a/drivers/xen/xenbus/xenbus_backend_client.c
>> >> +++ b/drivers/xen/xenbus/xenbus_backend_client.c
>> >> @@ -48,8 +48,7 @@ struct vm_struct *xenbus_map_ring_valloc
>> >>    gnttab_set_map_op(&op, (unsigned long)area->addr,
GNTMAP_host_map,
>> >>                      gnt_ref, dev->otherend_id);
>> >>
>> >> -  if (HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref,
&op, 1))
>> >> -          BUG();
>> >> +  gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref,
&op);
>> >>
>> >>    if (op.status != GNTST_okay) {
>> >>            free_vm_area(area);
>> >> @@ -75,15 +74,16 @@ int xenbus_map_ring(struct xenbus_device
>> >>
>> >>    gnttab_set_map_op(&op, (unsigned long)vaddr,
GNTMAP_host_map,
>> >>                      gnt_ref, dev->otherend_id);
>> >> -  if (HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref,
&op, 1))
>> >> -          BUG();
>> >> +  gnttab_check_GNTST_eagain_do_while(GNTTABOP_map_grant_ref,
&op);
>> >>
>> >>    if (op.status != GNTST_okay) {
>> >>            xenbus_dev_fatal(dev, op.status,
>> >>                             "mapping in shared page %d
from domain %d",
>> >>                             gnt_ref, dev->otherend_id);
>> >> -  } else
>> >> +  } else {
>> >>            *handle = op.handle;
>> >> +          return 0;
>> >> +  }
>> >>
>> >>    return op.status;
>> >>  }
>> >> diff -r df9e25a0189b -r 1170bc32db41 include/xen/gnttab.h
>> >> --- a/include/xen/gnttab.h
>> >> +++ b/include/xen/gnttab.h
>> >> @@ -187,4 +187,41 @@ gnttab_set_replace_op(struct gnttab_unma
>> >>    unmap->handle = handle;
>> >>  }
>> >>
>> >> +#define gnttab_check_GNTST_eagain_while(__HCop, __HCarg_p)  
                                     \
>> >> +do {                                                        
                                                                               
     \
>> >> +  u8 __hc_delay = 1;                                        
                                                                     \
>> >> +  int __ret;                                                
                                                                             \
>> >> +  while (unlikely((__HCarg_p)->status == GNTST_eagain
&& __hc_delay))
>> >> {  \
>> >> +          msleep(__hc_delay++);                              
                                                            \
>> >> +          __ret = HYPERVISOR_grant_table_op(__HCop,
(__HCarg_p), 1);                      \
>> >> +          BUG_ON(__ret);                                    
                                                                     \
>> >> +  }                                                          
                                                                               
    \
>> >> +  if (__hc_delay == 0) {                                    
                                                             \
>> >> +          printk(KERN_ERR "%s: %s gnt busy\n",
__func__, current->comm);          \
>> >> +          (__HCarg_p)->status = GNTST_bad_page;          
                                                \
>> >> +  }                                                          
                                                                               
    \
>> >> +  if ((__HCarg_p)->status != GNTST_okay)                  
                                               \
>> >> +          printk(KERN_ERR "%s: %s gnt status %x\n",
                                                      \
>> >> +                  __func__, current->comm,
(__HCarg_p)->status);                                  \
>> >> +} while(0)
>> >> +
>> >> +#define gnttab_check_GNTST_eagain_do_while(__HCop, __HCarg_p)
                    \
>> >> +do {                                                        
                                                                             \
>> >> +  u8 __hc_delay = 1;                                        
                                                             \
>> >> +  int __ret;                                                
                                                                     \
>> >> +  do {                                                      
                                                                     \
>> >> +          __ret = HYPERVISOR_grant_table_op(__HCop,
(__HCarg_p), 1);              \
>> >> +          BUG_ON(__ret);                                    
                                                             \
>> >> +          if ((__HCarg_p)->status == GNTST_eagain)        
                                       \
>> >> +                  msleep(__hc_delay++);                      
                                                    \
>> >> +  } while ((__HCarg_p)->status == GNTST_eagain &&
__hc_delay);            \
>> >> +  if (__hc_delay == 0) {                                    
                                                     \
>> >> +          printk(KERN_ERR "%s: %s gnt busy\n",
__func__, current->comm);  \
>> >> +          (__HCarg_p)->status = GNTST_bad_page;          
                                        \
>> >> +  }                                                          
                                                                            \
>> >> +  if ((__HCarg_p)->status != GNTST_okay)                  
                                       \
>> >> +          printk(KERN_ERR "%s: %s gnt status %x\n",
                                              \
>> >> +                  __func__, current->comm,
(__HCarg_p)->status);                          \
>> >> +} while(0)
>> >> +
>> >>  #endif /* __ASM_GNTTAB_H__ */
>> >>
>> >> _______________________________________________
>> >> Xen-devel mailing list
>> >> Xen-devel@lists.xensource.com
>> >> http://lists.xensource.com/xen-devel
>> >
>>
>>
>>
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@lists.xensource.com
>> http://lists.xensource.com/xen-devel
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

Seemingly Similar Threads

Search for more seemingly similar threads

Xen devel - Dec 2011 - [PATCH 0 of 2] Paging support updates for XCP dom0

[PATCH 0 of 2] Paging support updates for XCP dom0

[PATCH 1 of 2] xen/x86: make __direct_remap_pfn_range()''s return value meaningful

[PATCH 2 of 2] xenpaging: handle GNTST_eagain in kernel drivers

Re: [PATCH 2 of 2] xenpaging: handle GNTST_eagain in kernel drivers

Re: [PATCH 0 of 2] Paging support updates for XCP dom0

Re: [PATCH 1 of 2] xen/x86: make __direct_remap_pfn_range()''s return value meaningful

Re: [PATCH 2 of 2] xenpaging: handle GNTST_eagain in kernel drivers

Re: [PATCH 1 of 2] xen/x86: make __direct_remap_pfn_range()''s return value meaningful

Re: [PATCH 1 of 2] xen/x86: make __direct_remap_pfn_range()''s return value meaningful

Re: [PATCH 1 of 2] xen/x86: make __direct_remap_pfn_range()''s return value meaningful

Re: [PATCH 2 of 2] xenpaging: handle GNTST_eagain in kernel drivers

Re: [PATCH 2 of 2] xenpaging: handle GNTST_eagain in kernel drivers

Seemingly Similar Threads