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
[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
[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
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
>>> 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
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
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
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
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/0051d294bb60And you dropped the "From: Olaf Hering <ohering@novell.com>" line. Jan
Andres Lagar-Cavilla
2011-Dec-02 15:51 UTC
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
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
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