[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [Xen-changelog] Fix grant-table transfer implementation. Also fix transfer
# HG changeset patch # User kaf24@xxxxxxxxxxxxxxxxxxxx # Node ID 0add9fbe93ab843d239364386ff2e5bc82f75c5f # Parent 675862d22347c8f7efa87b9b49fff5d62e2b3516 Fix grant-table transfer implementation. Also fix transfer error paths in network frontend and backend drivers. Signed-off-by: Keir Fraser <keir@xxxxxxxxxxxxx> diff -r 675862d22347 -r 0add9fbe93ab linux-2.6-xen-sparse/arch/xen/kernel/gnttab.c --- a/linux-2.6-xen-sparse/arch/xen/kernel/gnttab.c Mon Nov 21 12:17:29 2005 +++ b/linux-2.6-xen-sparse/arch/xen/kernel/gnttab.c Mon Nov 21 13:17:50 2005 @@ -229,20 +229,29 @@ unsigned long gnttab_end_foreign_transfer_ref(grant_ref_t ref) { - unsigned long frame = 0; + unsigned long frame; u16 flags; - flags = shared[ref].flags; - /* - * If a transfer is committed then wait for the frame address to - * appear. Otherwise invalidate the grant entry against future use. - */ - if (likely(flags != GTF_accept_transfer) || - (synch_cmpxchg(&shared[ref].flags, flags, 0) != - GTF_accept_transfer)) - while (unlikely((frame = shared[ref].frame) == 0)) - cpu_relax(); + * If a transfer is not even yet started, try to reclaim the grant + * reference and return failure (== 0). + */ + while (!((flags = shared[ref].flags) & GTF_transfer_committed)) { + if ( synch_cmpxchg(&shared[ref].flags, flags, 0) == flags ) + return 0; + cpu_relax(); + } + + /* If a transfer is in progress then wait until it is completed. */ + while (!(flags & GTF_transfer_completed)) { + flags = shared[ref].flags; + cpu_relax(); + } + + /* Read the frame number /after/ reading completion status. */ + rmb(); + frame = shared[ref].frame; + BUG_ON(frame == 0); return frame; } diff -r 675862d22347 -r 0add9fbe93ab linux-2.6-xen-sparse/drivers/xen/netback/netback.c --- a/linux-2.6-xen-sparse/drivers/xen/netback/netback.c Mon Nov 21 12:17:29 2005 +++ b/linux-2.6-xen-sparse/drivers/xen/netback/netback.c Mon Nov 21 13:17:50 2005 @@ -99,7 +99,6 @@ return mfn; } -#if 0 static void free_mfn(unsigned long mfn) { unsigned long flags; @@ -120,7 +119,6 @@ } spin_unlock_irqrestore(&mfn_lock, flags); } -#endif static inline void maybe_schedule_tx_action(void) { @@ -287,28 +285,19 @@ ret = HYPERVISOR_multicall(rx_mcl, mcl - rx_mcl); BUG_ON(ret != 0); + ret = HYPERVISOR_grant_table_op(GNTTABOP_transfer, grant_rx_op, + gop - grant_rx_op); + BUG_ON(ret != 0); + mcl = rx_mcl; - if( HYPERVISOR_grant_table_op(GNTTABOP_transfer, grant_rx_op, - gop - grant_rx_op)) { - /* - * The other side has given us a bad grant ref, or has no - * headroom, or has gone away. Unfortunately the current grant - * table code doesn't inform us which is the case, so not much - * we can do. - */ - DPRINTK("net_rx: transfer to DOM%u failed; dropping (up to) " - "%d packets.\n", - grant_rx_op[0].domid, gop - grant_rx_op); - } gop = grant_rx_op; - while ((skb = __skb_dequeue(&rxq)) != NULL) { netif = netdev_priv(skb->dev); size = skb->tail - skb->data; /* Rederive the machine addresses. */ - new_mfn = mcl[0].args[1] >> PAGE_SHIFT; - old_mfn = 0; /* XXX Fix this so we can free_mfn() on error! */ + new_mfn = mcl->args[1] >> PAGE_SHIFT; + old_mfn = gop->mfn; atomic_set(&(skb_shinfo(skb)->dataref), 1); skb_shinfo(skb)->nr_frags = 0; skb_shinfo(skb)->frag_list = NULL; @@ -321,10 +310,10 @@ /* Check the reassignment error code. */ status = NETIF_RSP_OKAY; - if(gop->status != 0) { + if (gop->status != 0) { DPRINTK("Bad status %d from grant transfer to DOM%u\n", gop->status, netif->domid); - /* XXX SMH: should free 'old_mfn' here */ + free_mfn(old_mfn); status = NETIF_RSP_ERROR; } irq = netif->irq; diff -r 675862d22347 -r 0add9fbe93ab linux-2.6-xen-sparse/drivers/xen/netfront/netfront.c --- a/linux-2.6-xen-sparse/drivers/xen/netfront/netfront.c Mon Nov 21 12:17:29 2005 +++ b/linux-2.6-xen-sparse/drivers/xen/netfront/netfront.c Mon Nov 21 13:17:50 2005 @@ -739,14 +739,23 @@ (i != rp) && (work_done < budget); i++, work_done++) { rx = &np->rx->ring[MASK_NETIF_RX_IDX(i)].resp; + /* - * An error here is very odd. Usually indicates a backend bug, - * low-mem condition, or we didn't have reservation headroom. - */ - if (unlikely(rx->status <= 0)) { + * This definitely indicates a bug, either in this driver or + * in the backend driver. In future this should flag the bad + * situation to the system controller to reboot the backed. + */ + if ((ref = np->grant_rx_ref[rx->id]) == GRANT_INVALID_REF) { + WPRINTK("Bad rx response id %d.\n", rx->id); + work_done--; + continue; + } + + /* Memory pressure, insufficient buffer headroom, ... */ + if ((mfn = gnttab_end_foreign_transfer_ref(ref)) == 0) { if (net_ratelimit()) - printk(KERN_WARNING "Bad rx buffer " - "(memory squeeze?).\n"); + WPRINTK("Unfulfilled rx req (id=%d, st=%d).\n", + rx->id, rx->status); np->rx->ring[MASK_NETIF_RX_IDX(np->rx->req_prod)]. req.id = rx->id; wmb(); @@ -755,23 +764,8 @@ continue; } - ref = np->grant_rx_ref[rx->id]; - - if(ref == GRANT_INVALID_REF) { - printk(KERN_WARNING "Bad rx grant reference %d " - "from dom %d.\n", - ref, np->xbdev->otherend_id); - np->rx->ring[MASK_NETIF_RX_IDX(np->rx->req_prod)]. - req.id = rx->id; - wmb(); - np->rx->req_prod++; - work_done--; - continue; - } - + gnttab_release_grant_reference(&np->gref_rx_head, ref); np->grant_rx_ref[rx->id] = GRANT_INVALID_REF; - mfn = gnttab_end_foreign_transfer_ref(ref); - gnttab_release_grant_reference(&np->gref_rx_head, ref); skb = np->rx_skbs[rx->id]; ADD_ID_TO_FREELIST(np->rx_skbs, rx->id); diff -r 675862d22347 -r 0add9fbe93ab xen/common/grant_table.c --- a/xen/common/grant_table.c Mon Nov 21 12:17:29 2005 +++ b/xen/common/grant_table.c Mon Nov 21 13:17:50 2005 @@ -706,35 +706,34 @@ struct pfn_info *page; u32 _d, _nd, x, y; int i; - int result = GNTST_okay; grant_entry_t *sha; + gnttab_transfer_t gop; for ( i = 0; i < count; i++ ) { - gnttab_transfer_t *gop = &uop[i]; - - page = &frame_table[gop->mfn]; - - if ( unlikely(IS_XEN_HEAP_FRAME(page))) + /* Read from caller address space. */ + if ( unlikely(__copy_from_user(&gop, &uop[i], sizeof(gop))) ) + { + /* Caller error: bail immediately. */ + DPRINTK("gnttab_transfer: error reading req %d/%d\n", i, count); + return -EFAULT; + } + + /* Check the passed page frame for basic validity. */ + page = &frame_table[gop.mfn]; + if ( unlikely(!pfn_valid(gop.mfn) || IS_XEN_HEAP_FRAME(page)) ) { - printk("gnttab_transfer: xen heap frame mfn=%lx\n", - (unsigned long) gop->mfn); - gop->status = GNTST_bad_virt_addr; - continue; - } - - if ( unlikely(!pfn_valid(page_to_pfn(page))) ) - { - printk("gnttab_transfer: invalid pfn for mfn=%lx\n", - (unsigned long) gop->mfn); - gop->status = GNTST_bad_virt_addr; - continue; - } - - if ( unlikely((e = find_domain_by_id(gop->domid)) == NULL) ) - { - printk("gnttab_transfer: can't find domain %d\n", gop->domid); - gop->status = GNTST_bad_domain; + /* Caller error: bail immediately. */ + DPRINTK("gnttab_transfer: out-of-range or xen frame %lx\n", + (unsigned long)gop.mfn); + return -EINVAL; + } + + /* Find the target domain. */ + if ( unlikely((e = find_domain_by_id(gop.domid)) == NULL) ) + { + DPRINTK("gnttab_transfer: can't find domain %d\n", gop.domid); + (void)__put_user(GNTST_bad_domain, &uop[i].status); continue; } @@ -752,15 +751,16 @@ do { x = y; if (unlikely((x & (PGC_count_mask|PGC_allocated)) != - (1 | PGC_allocated)) || unlikely(_nd != _d)) { - printk("gnttab_transfer: Bad page values %p: ed=%p(%u), sd=%p," + (1 | PGC_allocated)) || unlikely(_nd != _d)) { + /* Caller error: bail immediately. */ + DPRINTK("gnttab_transfer: Bad page %p: ed=%p(%u), sd=%p," " caf=%08x, taf=%" PRtype_info "\n", (void *) page_to_pfn(page), d, d->domain_id, unpickle_domptr(_nd), x, page->u.inuse.type_info); spin_unlock(&d->page_alloc_lock); put_domain(e); - return 0; + return -EINVAL; } __asm__ __volatile__( LOCK_PREFIX "cmpxchg8b %2" @@ -788,16 +788,16 @@ */ if ( unlikely(test_bit(_DOMF_dying, &e->domain_flags)) || unlikely(e->tot_pages >= e->max_pages) || - unlikely(!gnttab_prepare_for_transfer(e, d, gop->ref)) ) + unlikely(!gnttab_prepare_for_transfer(e, d, gop.ref)) ) { DPRINTK("gnttab_transfer: Transferee has no reservation headroom " "(%d,%d) or provided a bad grant ref (%08x) or " "is dying (%lx)\n", - e->tot_pages, e->max_pages, gop->ref, e->domain_flags); + e->tot_pages, e->max_pages, gop.ref, e->domain_flags); spin_unlock(&e->page_alloc_lock); put_domain(e); - gop->status = result = GNTST_general_error; - break; + (void)__put_user(GNTST_general_error, &uop[i].status); + continue; } /* Okay, add the page to 'e'. */ @@ -805,23 +805,23 @@ get_knownalive_domain(e); list_add_tail(&page->list, &e->page_list); page_set_owner(page, e); - + spin_unlock(&e->page_alloc_lock); TRACE_1D(TRC_MEM_PAGE_GRANT_TRANSFER, e->domain_id); - + /* Tell the guest about its new page frame. */ - sha = &e->grant_table->shared[gop->ref]; - sha->frame = gop->mfn; + sha = &e->grant_table->shared[gop.ref]; + sha->frame = gop.mfn; wmb(); sha->flags |= GTF_transfer_completed; - + put_domain(e); - - gop->status = GNTST_okay; - } - - return result; + + (void)__put_user(GNTST_okay, &uop[i].status); + } + + return 0; } long _______________________________________________ Xen-changelog mailing list Xen-changelog@xxxxxxxxxxxxxxxxxxx http://lists.xensource.com/xen-changelog
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |