All of lore.kernel.org
 help / color / mirror / Atom feed
From: Konrad Rzeszutek Wilk <konrad@darnok.org>
To: Andres Lagar-Cavilla <andres@lagarcavilla.org>
Cc: olaf@aepfle.de, xen-devel@lists.xensource.com,
	andres@gridcentric.ca, jonathan.ludlam@eu.citrix.com,
	jbeulich@suse.com, mike.mcclurg@citrix.com
Subject: Re: [PATCH 2 of 2] xenpaging: handle GNTST_eagain in kernel drivers
Date: Fri, 2 Dec 2011 12:50:03 -0400	[thread overview]
Message-ID: <20111202165003.GA7807@andromeda.dapyr.net> (raw)
In-Reply-To: <8940966a49442039be1480307f59656b.squirrel@webmail.lagarcavilla.org>

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

  reply	other threads:[~2011-12-02 16:50 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
2011-12-17  3:00         ` Adin Scannell
2011-12-02  9:22 ` [PATCH 0 of 2] Paging support updates for XCP dom0 Jan Beulich

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20111202165003.GA7807@andromeda.dapyr.net \
    --to=konrad@darnok.org \
    --cc=andres@gridcentric.ca \
    --cc=andres@lagarcavilla.org \
    --cc=jbeulich@suse.com \
    --cc=jonathan.ludlam@eu.citrix.com \
    --cc=mike.mcclurg@citrix.com \
    --cc=olaf@aepfle.de \
    --cc=xen-devel@lists.xensource.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.