Re: slow xp hibernation revisited

[Keir Fraser <keir@xen.org>]

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

>> 
>> On 04/06/2011 08:38, "James Harper" <james.harper@bendigoit.com.au>
> 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?

By the way, depending on how this patch performs for you, another
alternative I thought of would be to fail this function if the address
passed in is the same as the address in a io-request we are currently
processing from Xen. After all, if Xen punted the memory access to qemu,
obviously there is no RAM there!

Could be an uglier patch than what you have below however, and maybe below
patch is good enough.

 -- Keir

> The exit condition is what would happen anyway after the remap is done
> and the page is still invalid.
> 
> 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);
>      }
>