[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] slow xp hibernation revisited


  • To: James Harper <james.harper@xxxxxxxxxxxxxxxx>, Tim Deegan <Tim.Deegan@xxxxxxxxxx>
  • From: Keir Fraser <keir@xxxxxxx>
  • Date: Sat, 04 Jun 2011 09:30:38 +0100
  • Cc: xen-devel@xxxxxxxxxxxxxxxxxxx, Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>, Stefano Stabellini <Stefano.Stabellini@xxxxxxxxxxxxx>
  • Delivery-date: Sat, 04 Jun 2011 01:31:23 -0700
  • Domainkey-signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=sender:user-agent:date:subject:from:to:cc:message-id:thread-topic :thread-index:in-reply-to:mime-version:content-type :content-transfer-encoding; b=dA1Bz/0dzbHhNEl8rq8kLO1ztWcm+hU5kKiOYZUe6XhNl2J01j/Lkp7X94WUCLNlMJ Iw/loLfz9ZQlRHa9XbQYaYjso1E5EVPf8QhWUX/506FoUBAWKRy/PsMfS5B4PRTKCXGY EJpg055iVH9YmiAqO8PyJ/yG1o9DetCmhp20s=
  • List-id: Xen developer discussion <xen-devel.lists.xensource.com>
  • Thread-index: AcwiBQi9TTmL+VIUTGmHL1mgnlOPmgAak8qgAADdoAAAA9fn5AABgqJAAAEDCUwAACdjUAABMxE1
  • Thread-topic: [Xen-devel] slow xp hibernation revisited

On 04/06/2011 09:05, "James Harper" <james.harper@xxxxxxxxxxxxxxxx> wrote:

>> 
>> On 04/06/2011 08:38, "James Harper" <james.harper@xxxxxxxxxxxxxxxx>
> wrote:
>> 
>>> Looking past the test_bit call, the next statement does another test
> and
>>> sets last_address_index to 0 and returns NULL. Is this just to
> ensure
>>> that the next access isn't just trivially accepted?
>> 
>> Yes, first test is on a potentially stale bucket. Second test is on a
> fresh
>> bucket.
>> 
> 
> How about the following patch? Is munmap the correct way to unmap or is
> an IOCTL required too?
> 
> The exit condition is what would happen anyway after the remap is done
> and the page is still invalid.

Looks fine to me, although maybe the function needs refactoring a little. Cc
Stefano and Ian who are more involved in qemu maintenance.

Also, looking at qemu_map_cache() now, the early exit at the top of the
function looks a bit bogus to me. It exits successfully if we hit the same
address_index as last invocation, even though we might be hitting a
different pfn within the indexed range, and a possibly invalid/unmapped pfn
at that.

 -- Keir

> diff --git a/hw/xen_machine_fv.c b/hw/xen_machine_fv.c
> index d02e23f..1ff80bb 100644
> --- a/hw/xen_machine_fv.c
> +++ b/hw/xen_machine_fv.c
> @@ -151,6 +151,24 @@ uint8_t *qemu_map_cache(target_phys_addr_t
> phys_addr, uint8_t lock)
>          pentry->next = entry;
>          qemu_remap_bucket(entry, address_index);
>      } else if (!entry->lock) {
> +        if (entry->vaddr_base && entry->paddr_index == address_index &&
> !test_bit(address_offset>>XC_PAGE_SHIFT, entry->valid_mapping))
> +        {
> +            /* The page was invalid previously. Test if it is valid now
> and only remap if so */
> +            xen_pfn_t pfn;
> +            int err;
> +            void *tmp_vaddr;
> +
> +            pfn = phys_addr >> XC_PAGE_SHIFT;
> +            tmp_vaddr = xc_map_foreign_bulk(xc_handle, domid,
> PROT_READ|PROT_WRITE, &pfn, &err, 1);
> +            if (tmp_vaddr)
> +                munmap(tmp_vaddr, PAGE_SIZE);
> +
> +            if (!tmp_vaddr || err)
> +            {
> +                last_address_index = ~0UL;
> +                return NULL;
> +            }
> +        }
>          if (!entry->vaddr_base || entry->paddr_index != address_index
> || !test_bit(address_offset>>XC_PAGE_SHIFT, entry->valid_mapping))
>              qemu_remap_bucket(entry, address_index);
>      }
> 



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.