WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

Re: [Xen-devel] slow xp hibernation revisited

To: James Harper <james.harper@xxxxxxxxxxxxxxxx>, Tim Deegan <Tim.Deegan@xxxxxxxxxx>
Subject: Re: [Xen-devel] slow xp hibernation revisited
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
Dkim-signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:sender:user-agent:date:subject:from:to:cc :message-id:thread-topic:thread-index:in-reply-to:mime-version :content-type:content-transfer-encoding; bh=0OQox1L+NE9KIhKZmLlh2WfdqQfwulZNx2Zw7FSKTHc=; b=pn7OJLQMSrn/JNfnkQ4+Og/PsWE0PN/2XT1DnNeYLjLGkJsfIhK7WVUd/WxjWR824C a7gRvA/PiK29AgyIQ9jT9+yCuE9MnHT+1dQ+SKM1+RYdZ0nOchHvwbuGibyYaaZriwq1 CJ8jBLMDctjD5j21VppWDB9qb9sjUS6MKZLQQ=
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=
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <AEC6C66638C05B468B556EA548C1A77D01D57761@trantor>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
Thread-index: AcwiBQi9TTmL+VIUTGmHL1mgnlOPmgAak8qgAADdoAAAA9fn5AABgqJAAAEDCUwAACdjUAABMxE1
Thread-topic: [Xen-devel] slow xp hibernation revisited
User-agent: Microsoft-Entourage/12.29.0.110113
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