* [PATCH 0 of 2] Paging support updates for XCP dom0
@ 2011-12-01 20:51 Andres Lagar-Cavilla
2011-12-01 20:51 ` [PATCH 1 of 2] xen/x86: make __direct_remap_pfn_range()'s return value meaningful Andres Lagar-Cavilla
` (2 more replies)
0 siblings, 3 replies; 12+ messages in thread
From: Andres Lagar-Cavilla @ 2011-12-01 20:51 UTC (permalink / raw)
To: xen-devel; +Cc: mike.mcclurg, andres, olaf, jonathan.ludlam, JBeulich
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(-)
^ permalink raw reply [flat|nested] 12+ messages in thread* [PATCH 1 of 2] xen/x86: make __direct_remap_pfn_range()'s return value meaningful 2011-12-01 20:51 [PATCH 0 of 2] Paging support updates for XCP dom0 Andres Lagar-Cavilla @ 2011-12-01 20:51 ` Andres Lagar-Cavilla 2011-12-02 9:26 ` Jan Beulich 2011-12-01 20:51 ` [PATCH 2 of 2] xenpaging: handle GNTST_eagain in kernel drivers Andres Lagar-Cavilla 2011-12-02 9:22 ` [PATCH 0 of 2] Paging support updates for XCP dom0 Jan Beulich 2 siblings, 1 reply; 12+ messages in thread From: Andres Lagar-Cavilla @ 2011-12-01 20:51 UTC (permalink / raw) To: xen-devel; +Cc: mike.mcclurg, andres, olaf, jonathan.ludlam, JBeulich 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(); ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1 of 2] xen/x86: make __direct_remap_pfn_range()'s return value meaningful 2011-12-01 20:51 ` [PATCH 1 of 2] xen/x86: make __direct_remap_pfn_range()'s return value meaningful Andres Lagar-Cavilla @ 2011-12-02 9:26 ` Jan Beulich 2011-12-02 15:31 ` Andres Lagar-Cavilla 0 siblings, 1 reply; 12+ messages in thread From: Jan Beulich @ 2011-12-02 9:26 UTC (permalink / raw) To: Andres Lagar-Cavilla Cc: xen-devel, olaf, andres, mike.mcclurg, jonathan.ludlam >>> 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1 of 2] xen/x86: make __direct_remap_pfn_range()'s return value meaningful 2011-12-02 9:26 ` Jan Beulich @ 2011-12-02 15:31 ` Andres Lagar-Cavilla 2011-12-02 15:44 ` Jan Beulich 0 siblings, 1 reply; 12+ messages in thread From: Andres Lagar-Cavilla @ 2011-12-02 15:31 UTC (permalink / raw) To: Jan Beulich; +Cc: xen-devel, olaf, andres, mike.mcclurg, jonathan.ludlam > >>> 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 > > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1 of 2] xen/x86: make __direct_remap_pfn_range()'s return value meaningful 2011-12-02 15:31 ` Andres Lagar-Cavilla @ 2011-12-02 15:44 ` Jan Beulich 2011-12-02 15:51 ` Andres Lagar-Cavilla 0 siblings, 1 reply; 12+ messages in thread From: Jan Beulich @ 2011-12-02 15:44 UTC (permalink / raw) To: andres; +Cc: xen-devel, olaf, andres, mike.mcclurg, jonathan.ludlam >>> On 02.12.11 at 16:31, "Andres Lagar-Cavilla" <andres@lagarcavilla.org> wrote: >> >>> On 01.12.11 at 21:51, Andres Lagar-Cavilla <andres@lagarcavilla.org> >> wrote: >>> arch/x86/mm/ioremap-xen.c | 12 ++++-------- >>> 1 files changed, 4 insertions(+), 8 deletions(-) >>> >>> >>> This change fixes the xc_map_foreign_bulk interface, which would >>> otherwise cause SIGBUS when pages are gone because -ENOENT is not >>> returned as expected by the IOCTL_PRIVCMD_MMAPBATCH_V2 ioctl. >>> >>> Signed-off-by: Jan Beulich <jbeulich@novell.com> >> >> You lost authorship here. > I used this: > http://xenbits.xen.org/hg/linux-2.6.18-xen.hg/rev/0051d294bb60 And you dropped the "From: Olaf Hering <ohering@novell.com>" line. Jan ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1 of 2] xen/x86: make __direct_remap_pfn_range()'s return value meaningful 2011-12-02 15:44 ` Jan Beulich @ 2011-12-02 15:51 ` Andres Lagar-Cavilla 0 siblings, 0 replies; 12+ messages in thread From: Andres Lagar-Cavilla @ 2011-12-02 15:51 UTC (permalink / raw) To: Jan Beulich; +Cc: xen-devel, olaf, andres, mike.mcclurg, jonathan.ludlam >>>> 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 > > ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 2 of 2] xenpaging: handle GNTST_eagain in kernel drivers 2011-12-01 20:51 [PATCH 0 of 2] Paging support updates for XCP dom0 Andres Lagar-Cavilla 2011-12-01 20:51 ` [PATCH 1 of 2] xen/x86: make __direct_remap_pfn_range()'s return value meaningful Andres Lagar-Cavilla @ 2011-12-01 20:51 ` Andres Lagar-Cavilla 2011-12-02 1:34 ` Konrad Rzeszutek Wilk 2011-12-02 9:22 ` [PATCH 0 of 2] Paging support updates for XCP dom0 Jan Beulich 2 siblings, 1 reply; 12+ messages in thread From: Andres Lagar-Cavilla @ 2011-12-01 20:51 UTC (permalink / raw) To: xen-devel; +Cc: mike.mcclurg, andres, olaf, jonathan.ludlam, JBeulich 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__ */ ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2 of 2] xenpaging: handle GNTST_eagain in kernel drivers 2011-12-01 20:51 ` [PATCH 2 of 2] xenpaging: handle GNTST_eagain in kernel drivers Andres Lagar-Cavilla @ 2011-12-02 1:34 ` Konrad Rzeszutek Wilk 2011-12-02 15:27 ` Andres Lagar-Cavilla 0 siblings, 1 reply; 12+ messages in thread From: Konrad Rzeszutek Wilk @ 2011-12-02 1:34 UTC (permalink / raw) To: Andres Lagar-Cavilla Cc: olaf, xen-devel, mike.mcclurg, jonathan.ludlam, JBeulich, andres 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2 of 2] xenpaging: handle GNTST_eagain in kernel drivers 2011-12-02 1:34 ` Konrad Rzeszutek Wilk @ 2011-12-02 15:27 ` Andres Lagar-Cavilla 2011-12-02 16:50 ` Konrad Rzeszutek Wilk 0 siblings, 1 reply; 12+ messages in thread From: Andres Lagar-Cavilla @ 2011-12-02 15:27 UTC (permalink / raw) To: Konrad Rzeszutek Wilk Cc: olaf, xen-devel, mike.mcclurg, jonathan.ludlam, jbeulich, andres > 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 > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2 of 2] xenpaging: handle GNTST_eagain in kernel drivers 2011-12-02 15:27 ` Andres Lagar-Cavilla @ 2011-12-02 16:50 ` Konrad Rzeszutek Wilk 2011-12-17 3:00 ` Adin Scannell 0 siblings, 1 reply; 12+ messages in thread From: Konrad Rzeszutek Wilk @ 2011-12-02 16:50 UTC (permalink / raw) To: Andres Lagar-Cavilla Cc: olaf, xen-devel, andres, jonathan.ludlam, jbeulich, mike.mcclurg 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2 of 2] xenpaging: handle GNTST_eagain in kernel drivers 2011-12-02 16:50 ` Konrad Rzeszutek Wilk @ 2011-12-17 3:00 ` Adin Scannell 0 siblings, 0 replies; 12+ messages in thread From: Adin Scannell @ 2011-12-17 3:00 UTC (permalink / raw) To: Konrad Rzeszutek Wilk Cc: olaf, xen-devel, andres, jonathan.ludlam, Andres Lagar-Cavilla, jbeulich, mike.mcclurg 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 0 of 2] Paging support updates for XCP dom0 2011-12-01 20:51 [PATCH 0 of 2] Paging support updates for XCP dom0 Andres Lagar-Cavilla 2011-12-01 20:51 ` [PATCH 1 of 2] xen/x86: make __direct_remap_pfn_range()'s return value meaningful Andres Lagar-Cavilla 2011-12-01 20:51 ` [PATCH 2 of 2] xenpaging: handle GNTST_eagain in kernel drivers Andres Lagar-Cavilla @ 2011-12-02 9:22 ` Jan Beulich 2 siblings, 0 replies; 12+ messages in thread From: Jan Beulich @ 2011-12-02 9:22 UTC (permalink / raw) To: Ian Campbell, Andres Lagar-Cavilla Cc: xen-devel, olaf, andres, mike.mcclurg, jonathan.ludlam >>> 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(-) ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2011-12-17 3:00 UTC | newest] Thread overview: 12+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-12-01 20:51 [PATCH 0 of 2] Paging support updates for XCP dom0 Andres Lagar-Cavilla 2011-12-01 20:51 ` [PATCH 1 of 2] xen/x86: make __direct_remap_pfn_range()'s return value meaningful Andres Lagar-Cavilla 2011-12-02 9:26 ` Jan Beulich 2011-12-02 15:31 ` Andres Lagar-Cavilla 2011-12-02 15:44 ` Jan Beulich 2011-12-02 15:51 ` Andres Lagar-Cavilla 2011-12-01 20:51 ` [PATCH 2 of 2] xenpaging: handle GNTST_eagain in kernel drivers Andres Lagar-Cavilla 2011-12-02 1:34 ` Konrad Rzeszutek Wilk 2011-12-02 15:27 ` Andres Lagar-Cavilla 2011-12-02 16:50 ` Konrad Rzeszutek Wilk 2011-12-17 3:00 ` Adin Scannell 2011-12-02 9:22 ` [PATCH 0 of 2] Paging support updates for XCP dom0 Jan Beulich
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.